Re: [PATCH] conditionalize some boring buffer_head checks

[Jens Axboe <axboe@suse.de>]

On Wed, Apr 14 2004, Jeff Garzik wrote:
> Jens Axboe wrote:
> >On Wed, Apr 14 2004, Jeff Garzik wrote:
> >>===== fs/buffer.c 1.237 vs edited =====
> >>--- 1.237/fs/buffer.c	Wed Apr 14 03:18:09 2004
> >>+++ edited/fs/buffer.c	Wed Apr 14 03:39:15 2004
> >>@@ -2688,6 +2688,7 @@
> >>{
> >>	struct bio *bio;
> >>
> >>+#ifdef BH_DEBUG
> >>	BUG_ON(!buffer_locked(bh));
> >>	BUG_ON(!buffer_mapped(bh));
> >>	BUG_ON(!bh->b_end_io);
> >
> >
> >The last one will be 'caught' at the other end of io completion, so I
> >guess that could be killed (even though you already lost the context of
> >the error, then). The first two are buffer state errors, I think those
> >should be kept unconditionally.
> >
> >
> >>@@ -2698,6 +2699,7 @@
> >>		buffer_error();
> >>	if (rw == READ && buffer_dirty(bh))
> >>		buffer_error();
> >>+#endif
> >
> >
> >I'm fine with killing the buffer_error(), maybe
> >
> >	if (rw == WRITE && !buffer_uptodate(bh))
> >		buffer_error();
> >
> >should be kept though.
> 
> 
> Well, all of these are buffer state (and programmer) errors...

Certainly, that is what they are meant to catch :-)

That's why I agree that some of them can be skipped, but I do think that
the ones I listed should be kept. It's an order of magnitude easier to
find and debug these errors if you are warned up front. I don't think
that saving those few cycles in an io submission path justifies that.

-- 
Jens Axboe