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

2014-06-20 Thread Lennart Poettering 
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 (!(flagsFS_NOATIME_FL)  ioctl(fd, 
 FS_IOC_SETFLAGS, flags|FS_NOATIME_FL) == 0)
 +flags |= FS_NOATIME_FL;
 +if (!(flagsFS_NOCOW_FL)  ioctl(fd, 
 FS_IOC_SETFLAGS, flags|FS_NOCOW_FL) == 0)
 +flags |= FS_NOCOW_FL;
 +if (!(flagsFS_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

2014-06-19 Thread Zbigniew Jędrzejewski-Szmek 
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

2014-06-18 Thread Dr. Werner Fink 
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 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;
 
 _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 (!(flagsFS_NOATIME_FL)  ioctl(fd, 
  FS_IOC_SETFLAGS, flags|FS_NOATIME_FL) == 0)
  +flags |= FS_NOATIME_FL;
  +if (!(flagsFS_NOCOW_FL)  ioctl(fd, 
  FS_IOC_SETFLAGS, flags|FS_NOCOW_FL) == 0)
  +flags |= FS_NOCOW_FL;
  +if (!(flagsFS_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

2014-06-18 Thread 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 (!(flagsFS_NOATIME_FL)  ioctl(fd, 
 FS_IOC_SETFLAGS, flags|FS_NOATIME_FL) == 0)
 +flags |= FS_NOATIME_FL;
 +if (!(flagsFS_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 (!(flagsFS_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

2014-06-16 Thread Cristian Rodríguez 
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 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;

_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 (!(flagsFS_NOATIME_FL)  ioctl(fd, 
 FS_IOC_SETFLAGS, flags|FS_NOATIME_FL) == 0)
 +flags |= FS_NOATIME_FL;
 +if (!(flagsFS_NOCOW_FL)  ioctl(fd, 
 FS_IOC_SETFLAGS, flags|FS_NOCOW_FL) == 0)
 +flags |= FS_NOCOW_FL;
 +if (!(flagsFS_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

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

2014-06-13 Thread Werner Fink 
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 (!(flagsFS_NOATIME_FL)  ioctl(fd, 
FS_IOC_SETFLAGS, flags|FS_NOATIME_FL) == 0)
+flags |= FS_NOATIME_FL;
+if (!(flagsFS_NOCOW_FL)  ioctl(fd, 
FS_IOC_SETFLAGS, flags|FS_NOCOW_FL) == 0)
+flags |= FS_NOCOW_FL;
+if (!(flagsFS_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);
-- 
1.7.9.2

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel