Re: [patch] pagecache-2.3.9-H3, bmap & ext2fs cleanup patch

Albert D. Cahalan (acahalan@cs.uml.edu)
Sat, 26 Jun 1999 20:19:10 -0400 (EDT)


Ingo Molnar writes:
> On Sat, 26 Jun 1999, Linus Torvalds wrote:

> nicely in other cases. (!mapped, uptodate, !dirty) does _not_ express
> holes in a straightforward way IMO. It just happens to accidentally

If Linux will use 3 bits to represent a hole (and I hope not),
please hide it in a macro. The kernel, all 1.5 million lines of
it, is a bit hard to follow when you can't hack Linux full-time.

> cleared when first mapped. They happen to be in the '!mapped' branch after
> the block is allocated only because the lowlevel fs sets it explicitly:
>
> phys = ext2_block_map(inode, iblock);
> if (phys) {
> bh_result->b_dev = inode->i_dev;
> bh_result->b_blocknr = phys;
> bh_result->b_state |= (1UL << BH_Mapped);
> }
>
> In the write case it's everything but not logical.

Code that is not logical tends to exclude the part-time hackers.
Linux development works best when any intelligent person can
fix a bug. Documentation and clean code make this possible.

>> So obviously I'll drop the BH_Hole bit - it isn't buying us anything at
>> all, and it _is_ setting us up for coherency problems.

Coherency problems are bugs which can be tested for with debug code.
#define CHECK_BH_BITS 1

> no, i simply have some optimizations planned which will be much uglier
> with 'if (!mapped && uptodate && !dirty)' than with a 'if (hole)'. And not
> because i cannot write macros to wrap the first one, but because the first
> one is embedded into the current code in sometimes subtle ways. I
> definitely feel uneasy about building false and misleading logic into the
> generic block handling code.

Ugh. Readable code is good.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/