[systemd-devel] [PATCH 1/2] Revert patch 11689d2a which force the NOCOW attribute

[Lennart Poettering]

On Sat, 21.03.15 12:56, Goffredo Baroncelli (kreijack at libero.it) wrote:

> From: Goffredo Baroncelli <kreijack at inwind.it>
> 
> Revert patch 11689d2a, which force the NOCOW attribute for the journal files.
> This patch was introduced to allievate the perfomances problem that
> journald shows on the BTRFS filesystem.
> 

>  #include "btrfs-util.h"
>  #include "journal-def.h"
> @@ -142,17 +141,8 @@ void journal_file_close(JournalFile *f) {
>          if (f->mmap && f->fd >= 0)
>                  mmap_cache_close_fd(f->mmap, f->fd);
>  
> -        if (f->fd >= 0 && f->defrag_on_close) {
> -
> -                /* Be friendly to btrfs: turn COW back on again now,
> -                 * and defragment the file. We won't write to the file
> -                 * ever again, hence remove all fragmentation, and
> -                 * reenable all the good bits COW usually provides
> -                 * (such as data checksumming). */
> -
> -                (void) chattr_fd(f->fd, false, FS_NOCOW_FL);
> -                (void) btrfs_defrag_fd(f->fd);
> -        }
> +        if (f->fd >= 0 && f->defrag_on_close)
> +                btrfs_defrag_fd(f->fd);

I think this part should probably stay... I mean, it's actually
without effect since btrfs doesn't allow unsetting the COW flag right
now if the file is non-empty. But I still think we should do this, in
case it learns it one day. And after rotating the file there's really
no need to disable COW anymore...

>  
>          /* btrfs doesn't cope well with our write pattern and
>           * fragments heavily. Let's defrag all files we rotate */
> -
> -        (void) chattr_path(p, false, FS_NOCOW_FL);
>          (void) btrfs_defrag(p);

This case is similar.

>  
>          log_warning("File %s corrupted or uncleanly shut down, renaming and replacing.", fname);
> diff --git a/src/journal/journalctl.c b/src/journal/journalctl.c
> index 55c7786..abb9d0c 100644
> --- a/src/journal/journalctl.c
> +++ b/src/journal/journalctl.c
> @@ -1290,7 +1290,7 @@ static int setup_keys(void) {
>          size_t mpk_size, seed_size, state_size, i;
>          uint8_t *mpk, *seed, *state;
>          ssize_t l;
> -        int fd = -1, r;
> +        int fd = -1, r, attr = 0;
>          sd_id128_t machine, boot;
>          char *p = NULL, *k = NULL;
>          struct FSSHeader h;
> @@ -1385,9 +1385,13 @@ static int setup_keys(void) {
>  
>          /* Enable secure remove, exclusion from dump, synchronous
>           * writing and in-place updating */
> -        r = chattr_fd(fd, true, FS_SECRM_FL|FS_NODUMP_FL|FS_SYNC_FL|FS_NOCOW_FL);
> -        if (r < 0)
> -                log_warning_errno(errno, "Failed to set file attributes: %m");
> +        if (ioctl(fd, FS_IOC_GETFLAGS, &attr) < 0)
> +                log_warning_errno(errno, "FS_IOC_GETFLAGS failed: %m");
> +
> +        attr |= FS_SECRM_FL|FS_NODUMP_FL|FS_SYNC_FL|FS_NOCOW_FL;
> +
> +        if (ioctl(fd, FS_IOC_SETFLAGS, &attr) < 0)
> +                log_warning_errno(errno, "FS_IOC_SETFLAGS failed: %m");

This is unrelated, and should not be reverted at all.

Lennart

-- 
Lennart Poettering, Red Hat