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

[Andrew Morton <akpm@osdl.org>]

Nick Piggin <nickpiggin@yahoo.com.au> wrote:
>
> > There are two situations:
>  > 
>  > a) Thread 2 comes in and tries to write a buffer which thread1 didn't write:
>  > 
>  >    Yes, thread 1 will get confused and will try to write thread 2's buffer.
>  > 
>  > b) Thread 2 comes in and tries to write a buffer which thread 1 is
>  >    writing.  (Say, the buffer was redirtied by
>  >    munmap->__set_page_dirty_buffers, which doesn't lock the page or the
>  >    buffers)
>  > 
> 
>  I don't think b) happens, because thread 1 has to have finished all
>  its writes before the page will end writeback and thread 2 can go
>  anywhere.

hm, spose so.

>  >    Thread 2 will fail the test_set_buffer_locked() and will redirty the page.
>  > 
>  > 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.

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.

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

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