[systemd-devel] [PATCH 10/11] Avoid doubling the efforts for /var/log/journal

[Goffredo Baroncelli]

Hi Fink

On 06/13/2014 04:41 PM, Werner Fink wrote:
> That is: set NOATIME, NOCOW, and NOCOMP for the journal directory
> 
> ---
>  src/journal/journald-server.c |   29 +++++++++++++++++++++++++++--
>  1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git src/journal/journald-server.c src/journal/journald-server.c
> index eda5dcf..37d6dc3 100644
> --- src/journal/journald-server.c
> +++ src/journal/journald-server.c
> @@ -21,6 +21,7 @@
>  
>  #include <sys/signalfd.h>
>  #include <sys/ioctl.h>
> +#include <linux/fs.h>
>  #include <linux/sockios.h>
>  #include <sys/statvfs.h>
>  #include <sys/mman.h>
> @@ -920,7 +921,7 @@ finish:
>  
>  
>  static int system_journal_open(Server *s) {
> -        int r;
> +        int r, fd;
>          char *fn;
>          sd_id128_t machine;
>          char ids[33];
> @@ -947,7 +948,31 @@ static int system_journal_open(Server *s) {
>                          (void) mkdir("/var/log/journal/", 0755);
>  
>                  fn = strappenda("/var/log/journal/", ids);
> -                (void) mkdir(fn, 0755);
> +                (void)mkdir(fn, 0755);
> +
> +                /*
> +                 * On journaling and/or compressing file systems avoid doubling the
> +                 * efforts for the system, that is set NOCOW and NOCOMP inode flags.
> +                 * Check for every single flag as otherwise some of the file systems
> +                 * may return EOPNOTSUPP on one unkown flag (like BtrFS does).
> +                 */
> +                if ((fd = open(fn, O_DIRECTORY)) >= 0) {
> +                        long flags;
> +                        if (ioctl(fd, FS_IOC_GETFLAGS, &flags) == 0) {
> +                                int old = flags;
> +                                if (!(flags&FS_NOATIME_FL) && ioctl(fd, FS_IOC_SETFLAGS, flags|FS_NOATIME_FL) == 0)
> +                                        flags |= FS_NOATIME_FL;
> +                                if (!(flags&FS_NOCOW_FL) && ioctl(fd, FS_IOC_SETFLAGS, flags|FS_NOCOW_FL) == 0)
> +                                        flags |= FS_NOCOW_FL;

If I read correctly, you want set UN-conditionally the NOCOW behavior. Please, please, please DON'T DO that.

The NOCOW behavior is not without disadvantage: yes it increase the performance but
the file also lost the btrfs checksum protection; when BTRFS manage the disks in RAID mode and a corruption happens, it uses the checksum to select the correct mirror during the reading. If you set UN-conditionally the NOCOW behavior you lost this capability even if the user _want it_ (and if they spend moneys in two or more disks, it is likely they _want it_).

Moreover the NOCOW flags has some "strange" behavior when a NOCOW file is snapshotted (it lost the NOCOW property); this may lead to irregular performance.

If you want it, it must be configurable at least with a sane default (which IMHO should be "do nothing", following the "least surprise" rule).

If you are looking to something like that, I suggest also to defrag the journal file before the open (but still as configurable option, and considering the "least surprise" rule).

BR
G.Baroncelli

> +                                if (!(flags&FS_NOCOMP_FL) && s->compress) {
> +                                        flags &= ~FS_COMPR_FL;
> +                                        flags |= FS_NOCOMP_FL;
> +                                }
> +                                if (old != flags)
> +                                        ioctl(fd, FS_IOC_SETFLAGS, flags);
> +                        }
> +                        close(fd);
> +                }
>  
>                  fn = strappenda(fn, "/system.journal");
>                  r = journal_file_open_reliably(fn, O_RDWR|O_CREAT, 0640, s->compress, s->seal, &s->system_metrics, s->mmap, NULL, &s->system_journal);
> 


-- 
gpg @keyserver.linux.it: Goffredo Baroncelli (kreijackATinwind.it>
Key fingerprint BBF5 1610 0B64 DAC6 5F7D  17B2 0EDA 9B37 8B82 E0B5