[lkml]   [2009]   [Mar]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH] issue storage device flush via sync_blockdev() (was Re: Linux 2.6.29)
    Ric Wheeler wrote:
    > Eric Sandeen wrote:
    >> Jeff Garzik wrote:
    >>> On Wed, Mar 25, 2009 at 01:40:37PM -0700, Linus Torvalds wrote:
    >>>> On Wed, 25 Mar 2009, Jeff Garzik wrote:
    >>>>> It is clearly possible to implement an fsync(2) that causes FLUSH
    >>>>> CACHE to be
    >>>>> issued, without adding full barrier support to a filesystem. It is
    >>>>> likely
    >>>>> doable to avoid touching per-filesystem code at all, if we issue
    >>>>> the flush
    >>>>> from a generic fsync(2) code path in the kernel.
    >>>> We could easily do that. It would even work for most cases. The
    >>>> problematic ones are where filesystems do their own disk management,
    >>>> but I guess those people can do their own fsync() management too.
    >>>> Somebody send me the patch, we can try it out.
    >>> This is a simple step that would cover a lot of cases... sync(2)
    >>> calls sync_blockdev(), and many filesystems do as well via the generic
    >>> filesystem helper file_fsync (fs/sync.c).
    >>> XFS code calls sync_blockdev() a "big hammer", so I hope my patch
    >>> follows with known practice.
    >>> Looking over every use of sync_blockdev(), its most frequent use is
    >>> through fsync(2), for the selected filesystems that use the generic
    >>> file_fsync helper.
    >>> Most callers of sync_blockdev() in the kernel do so infrequently,
    >>> when removing and invalidating volumes (MD) or storing the superblock
    >>> prior to release (put_super) in some filesystems.
    >>> Compile-tested only, of course :) But it should be work :)
    >>> My main concern is some hidden area that calls sync_blockdev() with
    >>> a high-enough frequency that the performance hit is bad.
    >>> Signed-off-by: Jeff Garzik <>
    >>> diff --git a/fs/buffer.c b/fs/buffer.c
    >>> index 891e1c7..7b9f74a 100644
    >>> --- a/fs/buffer.c
    >>> +++ b/fs/buffer.c
    >>> @@ -173,9 +173,14 @@ int sync_blockdev(struct block_device *bdev)
    >>> {
    >>> int ret = 0;
    >>> - if (bdev)
    >>> - ret = filemap_write_and_wait(bdev->bd_inode->i_mapping);
    >>> - return ret;
    >>> + if (!bdev)
    >>> + return 0;
    >>> +
    >>> + ret = filemap_write_and_wait(bdev->bd_inode->i_mapping);
    >>> + if (ret)
    >>> + return ret;
    >>> +
    >>> + return blkdev_issue_flush(bdev, NULL);
    >>> }
    >>> EXPORT_SYMBOL(sync_blockdev);
    >> What about when you're running over a big raid device with
    >> battery-backed cache, and you trust the cache as much as much as the
    >> disks. Wouldn't this unconditional cache flush be painful there on any
    >> of the callers even if they're rare? (fs unmounts, freezes, unmounts,
    >> etc? Or a fat filesystem on that device doing an fsync?)
    >> xfs, reiserfs, ext4 all avoid the blkdev flush on fsync if barriers are
    >> not enabled, I think for that reason...
    >> (I'm assuming these raid devices still honor a cache flush request even
    >> if they're battery-backed? I dunno.)
    >> -Eric
    > I think that Jeff's patch misses the whole need to protect transactions,
    > including meta data, in a precise way. Useful for thing like unmount,
    > not to give us strong protection for transactions or for fsync().

    What do you think sync_blockdev() does? What is its purpose?

    (1) guarantee all user data is flushed out before a major event
    (unmount, journal close, unplug, poweroff, explosion, ...)

    (2) As a sledgehammer hack for simple or legacy filesystems that do not
    wish or need the complexity of transactional protection.
    sync_blockdev() is intentionally used in lieu of complexity for the
    following filesystems: HFS, HFS+, ADFS, AFFS, FAT, bfs, UFS, NTFS, qnx4.

    My patch adds needed guarantees, only for the above filesystems, where
    none were present before.

    > This patch will be adding overhead here - you will still need flushing
    > at the transaction commit layer of the specific file systems to get any
    > reliable transactions.

    sync_blockdev() is used as fsync(2) only in simple or legacy filesystems
    that do not want a transaction commit layer!

    Read the patch :)


     \ /
      Last update: 2009-03-26 02:31    [W:0.033 / U:4.696 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site