Re: [systemd-devel] [PATCH 10/11] Avoid doubling the efforts for /var/log/journal
On Fri, 13.06.14 16:41, Werner Fink (wer...@suse.de) wrote:
> 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 (!(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);
> +}
Humm, no. We won't tape over problems. If the defragging thing is a
performance issue we need to figure out why that happens, and as it
stands now it simply seems to be a weakness in the current btrfs kernel
code.
But we will not work around these problems from userspace just like
that.
I mean, I am fine with giving btrfs special support and stuff, but I
want input from the right kernel guys about about this. If they
recommend this, then sure, we can do something like this, but we don't
blindly tape over this.
Sorry,
Lennart
--
Lennart Poettering, Red Hat
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 10/11] Avoid doubling the efforts for /var/log/journal
On Wed, Jun 18, 2014 at 08:10:03PM +0200, Goffredo Baroncelli wrote: > Hi Fink > > On 06/13/2014 04:41 PM, Werner Fink wrote: > > That is: set NOATIME, NOCOW, and NOCOMP for the journal directory > > 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). Yes, this is too much of a hack. Various defragging approaches seem better. Zbyszek ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 10/11] Avoid doubling the efforts for /var/log/journal
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
> #include
> +#include
> #include
> #include
> #include
> @@ -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
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 10/11] Avoid doubling the efforts for /var/log/journal
On Mon, Jun 16, 2014 at 11:10:15AM -0400, Cristian Rodríguez wrote:
> El 13/06/14 10:41, Werner Fink escribió:
> > 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
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -920,7 +921,7 @@ finish:
> >
> >
> > static int system_journal_open(Server *s) {
> > -int r;
> > +int r, fd;
>
> _cleanup_close_ ...
>
> > +/*
> > + * 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) {
>
> O_CLOEXEC...
>
> > +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 (!(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);
> > +}
>
> I agree that this should be done, however I remain unconvinced this is
> the right place to do it..
IMHO this is the correct place as it helps to speed up systemd-journal
on BtrFS. This was the reason for this patch and is tested even if the
patch does not use _cleanup_close_ and O_CLOEXEC ;)
Werner
--
"Having a smoking section in a restaurant is like having
a peeing section in a swimming pool." -- Edward Burr
pgp3Lcj6mvhs4.pgp
Description: PGP signature
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 10/11] Avoid doubling the efforts for /var/log/journal
El 13/06/14 10:41, Werner Fink escribió:
> 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
> #include
> +#include
> #include
> #include
> #include
> @@ -920,7 +921,7 @@ finish:
>
>
> static int system_journal_open(Server *s) {
> -int r;
> +int r, fd;
_cleanup_close_ ...
> +/*
> + * 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) {
O_CLOEXEC...
> +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 (!(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);
> +}
I agree that this should be done, however I remain unconvinced this is
the right place to do it..
--
Cristian
"I don't know the key to success, but the key to failure is trying to
please everybody."
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel
