Re: [patch] fix the 2nd buffer race properly

[Nick Piggin]

Andrew Morton wrote:
> Nick Piggin <nickpiggin@xxxxxxxxxxxx> wrote:

> > > That's all a bit too complex.   How's about this instead?
> > > 
> >
> > Well you really don't need to hold the page locked for that long.
>
>
> Is a rare case, so there's no perfomance issue here.

Well it is for buffered writes, right? And in general it will be OK,
but if you run into queue/memory congestion when submitting the IO,
it will be locked for a lot longer than required.

> I do prefer the idea of simply keeping other threads of control out of the
> page until this thread has finished playing with its buffers.

That's exactly what my patch does too!

> (The buffer-ring walk we have in there is racy against page reclaim, too. 
> If only the first buffer is dirty, we inspect the other buffers after
> PageWriteback has potentially cleared.)

Well we do have a reference on the buffers, so in this particular case
perhaps not. But we have no mutual exclusion on the page or buffers
so I agree it could be racy against a lot of things.

> > block_read_full_page, nobh_prepare_write both use the same sort of
> > array of buffer heads logic - I think it makes sense not to touch
> > any buffers after submitting them all for IO...?
>
>
> Well.  Most code in there uses the ->b_this_page walk.

block_read_full_page does the walk in order to gather up the buffers.
They then get submitted for IO via the buffer head array walk.

I prefer my patch. I don't think it is particularly complex.

--
SUSE Labs, Novell Inc.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/