Re: [PATCH] invalidate buffers on blkdev_put

[Anton Altaparmakov]

At 05:21 25/09/01, Linus Torvalds wrote:
>On Mon, 24 Sep 2001, Alexander Viro wrote:
> > OK, not exactly nay, but...  What you are trying to do is a workaround
> > for problem that can be solved in somewhat saner way.  Namely, we can
> > make getblk() return buffres backed by pages from device page cache.
>
>I now have the patches for this, but I have to fix up fs/block_dev.c to
>also honour the block size thing because otherwise the two are still not
>in sync.
>
>I'll send out a test-patch later this evening, I hope.
>
> > It's solvable, but not obvious.  It _does_ solve coherency problems between
> > device page cache and buffer cache (thus killing update_buffers() and its
> > ilk), but the last issue (new access path to page-private buffer_heads)
> > may be rather nasty.
>
>It's certainly solvable, but it is also certainly very fraught with tons
>of small details. I'll be very happy if people end up looking through the
>patches _very_ critically (and don't even bother testing them if you don't
>have a machine where you can lose a filesystem or two).
>
>Hopefully in another hour or two (but the first version is going to have
>some ugly stuff in it still).

Looking at the patch, you introduce a static inline blksize_bits. Wouldn't 
it be a lot more efficient to change the function to say:

static inline unsigned int blksize_bits(unsigned int size)
{
         return ffs(size) - 1;
}

and optionally, throw in a power of two assertion a-la:

static inline unsigned int blksize_bits(unsigned int size)
{
         if (!(size & size - 1))
                 return ffs(size) - 1;
         BUG();
}

Or am I barking mad and block sizes which are not a power of two are valid? (-;

Your version is not too happy with such beasts either but it does round 
down rather than do god knows what in arch specific ffs() implementation... 
Haven't looked at non-ia32 code but at least ia32's implementation fails 
miserably for non-powers of two by it's design. But it should be a lot 
faster than doing the while loop considering ffs() just uses a single CPU 
instruction instead of the loop (on ia32 anyway).

Best regards,

         Anton