[Top] [All Lists]

Re: [PATCH] xfs: fix buffer flushing during unmount

To: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Subject: Re: [PATCH] xfs: fix buffer flushing during unmount
From: Alex Elder <aelder@xxxxxxx>
Date: Tue, 4 Oct 2011 22:55:42 -0500
Cc: <xfs@xxxxxxxxxxx>
In-reply-to: <20110928163437.GA1530@xxxxxxxxxxxxx>
References: <20110914140826.GA25729@xxxxxxxxxxxxx> <1316453600.2941.37.camel@doink> <20110928163437.GA1530@xxxxxxxxxxxxx>
Reply-to: <aelder@xxxxxxx>
On Wed, 2011-09-28 at 12:34 -0400, Christoph Hellwig wrote:
> On Mon, Sep 19, 2011 at 12:33:20PM -0500, Alex Elder wrote:
> > On Wed, 2011-09-14 at 10:08 -0400, Christoph Hellwig wrote:
> > > 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.
> > 
> > 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.

And now that you've said that I see that there are in fact
no calls to xfs_buf_get() that specify anything other than
the data device.

> > 
> > 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.

OK, I get it.  Your original patch looks good.

Reviewed-by: Alex Elder <aelder@xxxxxxx>

<Prev in Thread] Current Thread [Next in Thread>