Re: [PATCH] btrfs: fix write_dev_supers

[Chris Mason <chris.mason@oracle.com>]

On Wed, Jun 10, 2009 at 04:32:31PM +0900, Hisashi Hifumi wrote:
> 
> At 20:25 09/06/09, Chris Mason wrote:
> >On Tue, Jun 09, 2009 at 10:46:55AM +0900, Hisashi Hifumi wrote:
> >> Hi.
> >> 
> >> I got following BUG trace.
> >> This is violation of BUG_ON(!buffer_locked(bh)) check on submit_bh() function.
> >> In write_dev_supers(), if wait parameter is set and buffer_uptodate() check
> >> is negative,  submit_bh() is executed and hit above BUG_ON.
> >> So I fixed this issue.
> >
> >Thanks for finding this bug and sending the patch.
> >
> >This function is very confusing.  If wait parameter is set, it
> >isn't supposed to do any IO at all.  The caller first does
> >write_dev_supers with wait == 0, and that sends all the supers down on
> >all the devices.
> >
> >Then it calls again with wait == 1, which is supposed to make sure all
> >the supers actually got to disk.
> >
> >We should change the wait == 0 behavior to leave a reference held on all
> >the buffers, and wait == 1 to drop that reference.  That way the buffer
> >won't disappear while we are waiting, and we can return an error if the
> >buffer wasn't up to date when wait == 1.
> >
> 
> Like this?
> 
> I changed wait == 0 case to get extra ref and on wait == 1 case if buffer is 
> uptodate, bh releases ref otherwise buffer takes lock to proceed to submit_bh.

That's very close to what I had in mind, thank you.  In reviewing this I
realized that write_dev_supers had other bugs, including a race with
device add/removal.  So, I took your patch and edited it slightly.  You
could you please check the change I put into newformat2 branch?

In this version, wait == 1 only waits for IO and does not try to start
it, I think it makes it more clear overall.

git://git.kernel.org/pub/scm/linux/kernel/git/mason/btrfs-unstable.git newformat2

Thanks!

-chris