xfs: fix buffer flushing during unmount

The code to flush buffers in the umount code is a bit iffy: we first flush all delwri buffers out, but then might be able to queue up a new one when logging the sb counts. On a normal shutdown that one would get flushed out when doing the synchronous superblock write in xfs_unmountfs_writesb, but we skip that one if the filesystem has been shut down.

Fix this by moving the delwri list flushing until just before unmounting the log, and while we're at it also remove the superflous delwri list and buffer lru flusing for the rt and log device that can never have cached or delwri buffers.

Signed-off-by Christoph Hellwig <[email protected]>

Reported-by Amit Sahrawat <[email protected]>

Tested-by Amit Sahrawat <[email protected]>

Quoted 6 lines · Christoph Hellwig

The code to flush buffers in the umount code is a bit iffy: we first flush all delwri buffers out, but then might be able to queue up a new one when logging the sb counts. On a normal shutdown that one would get flushed out when doing the synchronous superblock write in xfs_unmountfs_writesb, but we skip that one if the filesystem has been shut down.

Quoted 4 lines

Fix this by moving the delwri list flushing until just before unmounting the log, and while we're at it also remove the superflous delwri list and buffer lru flusing for the rt and log device that can never have cached or delwri buffers.

What prevents blocks on a realtime device from being cached?

I haven't looked exhaustively, but it looks like the summary and bitmap data are operated on without caching, but the realtime data itself is treated no different from "normal" data blocks.

Quoted 7 lines · Alex Elder

Quoted 6 lines · Christoph Hellwig

The code to flush buffers in the umount code is a bit iffy: we first flush all delwri buffers out, but then might be able to queue up a new one when logging the sb counts. On a normal shutdown that one would get flushed out when doing the synchronous superblock write in xfs_unmountfs_writesb, but we skip that one if the filesystem has been shut down.

Quoted 6 lines

Quoted 4 lines

Fix this by moving the delwri list flushing until just before unmounting the log, and while we're at it also remove the superflous delwri list and buffer lru flusing for the rt and log device that can never have cached or delwri buffers.

What prevents blocks on a realtime device from being cached?

Note that cached here means using buffers that end up in the rbtree and/or on the lru. We simply never do that for the rt and log device, as that would mean complicating the code for no reason - the only time we ever use buffer on the rt device is for the size check during mount and the _uncached variant where we free the buffer after the refcount hits zero is just fine for that, and xfs_zero_remaining_bytes, which actually operates on a regular file and thus must not use the rbtree or the lru (and should eventually stop using buffer routines at all). All actual metadata which the buffer cache is used for resides on the main device. Log buffers stay alive from mount to unmount so we're fine with that interface as well.

Quoted 5 lines

I haven't looked exhaustively, but it looks like the summary and bitmap data are operated on without caching, but the realtime data itself is treated no different from "normal" data blocks.

The buffer cache is never used for actual file data.

The code to flush buffers in the umount code is a bit iffy: we first flush all delwri buffers out, but then might be able to queue up a new one when logging the sb counts.  On a normal shutdown that one would get flushed out when doing the synchronous superblock write in xfs_unmountfs_writesb, but we skip that one if the filesystem has been shut down.

Fix this by moving the delwri list flushing until just before unmounting the log, and while we're at it also remove the superflous delwri list and buffer lru flusing for the rt and log device that can never have cached or delwri buffers.

Signed-off-by Christoph Hellwig <[email protected]>

Reported-by Amit Sahrawat <[email protected]>

Tested-by Amit Sahrawat <[email protected]>

Index: xfs/fs/xfs/xfs_buf.h
===================================================================

fs/xfs/xfs_buf.h        2011-09-13 10:55:52.461091480 -0400 renamed from fs/xfs/xfs_buf.h   2011-09-13 10:55:20.744090516 -0400 +0 -1

@@ -318,7 +318,6 @@ extern struct list_head *xfs_get_buftarg
  #define xfs_getsize_buftarg(buftarg)   block_size((buftarg)->bt_bdev)
  #define xfs_readonly_buftarg(buftarg)  bdev_read_only((buftarg)->bt_bdev)
 
-#define xfs_binval(buftarg)            xfs_flush_buftarg(buftarg, 1)
  #define XFS_bflush(buftarg)            xfs_flush_buftarg(buftarg, 1)
 
  #endif /* __XFS_BUF_H__ */

Index: xfs/fs/xfs/xfs_mount.c
===================================================================

fs/xfs/xfs_mount.c      2011-09-13 10:56:19.108088343 -0400 renamed from fs/xfs/xfs_mount.c 2011-09-13 10:55:20.748087866 -0400 +0 -3

@@ -44,9 +44,6 @@
  #include "xfs_trace.h"
 
 
-STATIC void    xfs_unmountfs_wait(xfs_mount_t *);
-
-
  #ifdef HAVE_PERCPU_SB
  STATIC void    xfs_icsb_balance_counter(xfs_mount_t *, xfs_sb_field_t,
                                                int);

fs/xfs/xfs_mount.c      2011-09-13 10:56:19.108088343 -0400 renamed from fs/xfs/xfs_mount.c 2011-09-13 10:55:20.748087866 -0400 +0 -0

        /*
         * Unreserve any blocks we have so that when we unmount we don't account
         * the reserved free space as used. This is really only necessary for

@@ -1526,7 +1518,16 @@ xfs_unmountfs(

               xfs_warn(mp, "Unable to update superblock counters. " "Freespace may not be correct on next mount."); xfs_unmountfs_writesb(mp);
-       xfs_unmountfs_wait(mp);                 /* wait for async bufs */

       xfs_log_unmount_write(mp);
       xfs_log_unmount(mp);
       xfs_uuid_unmount(mp);

fs/xfs/xfs_mount.c      2011-09-13 10:56:19.108088343 -0400 renamed from fs/xfs/xfs_mount.c 2011-09-13 10:55:20.748087866 -0400 +0 -0

@@ -1537,16 +1538,6 @@ xfs_unmountfs(
        xfs_free_perag(mp);
  }
 

 int
 xfs_fs_writable(xfs_mount_t *mp)
 {

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs