Re: [PATCH] Change ll_rw_block() calls in JBD

Stephen C. Tweedie
2006-05-19T15:07:04Z

Hi,

On Thu, 2006-05-18 at 15:45 +0200, Jan Kara wrote:

> Yes, I'm aware of this problem. Actually I wrote a patch (attached) for it
> some time ago but as I'm checking current kernels it got lost somewhere on
> the way. I'll rediff it and submit again. Thanks for spotting the
> problem.

...
>
> + was_dirty = buffer_dirty(bh);
> + if (was_dirty && test_set_buffer_locked(bh)) {

The way was_dirty is used here seems confusing and hard to read; there
are completely separate code paths for dirty and non-dirty, lockable and
already-locked buffers, with all the paths interleaved to share a few
common bits of locking. It would be far more readable with any sharable
locking code simply removed to a separate function (such as we already
have for inverted_lock(), for example), and the different dirty/clean
logic laid out separately. Otherwise the code is littered with

> + if (was_dirty)
> + unlock_buffer(bh);

and it's not obvious at any point just what locks are held.

Having said that, it looks like it should work --- it just took more
effort than it should have to check!

--Stephen