Messages in this thread | | | Subject | Re: [PATCH] Change ll_rw_block() calls in JBD | From | "Stephen C. Tweedie" <> | Date | Fri, 19 May 2006 16:06:48 +0100 |
| |
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
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |