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

Andrew Morton
2005-04-28T01:00:51Z

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.