Messages in this thread | | | Date | Sat, 26 Jun 1999 11:51:38 -0700 (PDT) | From | Linus Torvalds <> | Subject | Re: [patch] pagecache-2.3.9-H3, bmap & ext2fs cleanup patch |
| |
On Sat, 26 Jun 1999, Ingo Molnar wrote: > > in my patch even holes are BH_Mapped, thats the difference.
And I think that's wrong.
Holes do not have a disk mapping. We should not claim that they do.
The bits should make sense, and they should not contain redundant information.
For example, if done right (I think my tree does it right), then you can have an assert in ll_rw_block:
if (!buffer_mapping(bh)) BUG(); if (buffer_new(bh)) BUG();
and neither of them should EVER fire - because it's obviously a bug to try to do IO with a buffer that hasn't got any mapping associated with it. Similarly it's a bug to do anything but a write on a new buffer, and by the time we do that we should have marked it dirty adn updated it, which means that it's not new any more.
Contrast that strict logic with your rather convoluted version that is redundant..
> So i think there is _no_ real relation between BH_Mapped and the caching > state bits, apart from the fact that currently we first establish the > mapping and then start off with !BH_Uptodate !Dirty.
You can not EVER have a dirty non-mapped buffer. That would be an error.
You can have a up-to-date non-mapped buffer: it's just a hole, and is zero-filled.
> _currently_ i agree that we might encode BH_Hole as an otherwise invalid > combination of state-bits, but i think this is dangerous. Eg. if we want > to do the 'delayed write' stuff [to allocate on-disk blocks only when a > block is being written out], then (Uptodate && !Mapped) will be a valid > combination: it means that the mapping towards the disk (filesystem) is > not yet established, but in-memory contents are valid and (obviously) more > recent than on-disk contents.
(Uptodate && !Mapped) is valid already - it's _used_ in pre-2.3.9-4 (except "Mapped" was called "Allocated", and I agree that "Mapped" is a better name for it).
It _should_ be valid.
I think your problem is that you use "uptodate()" to check whether something is mapped. Your patches had a number of these instances, and it's wrong. I explicitly removed all of them from pre-2.3.9-4, and you re-introduced the ugly behaviour, probably because you were working off an older source base..
Anyway, I'll reboot into my current kernel, verify that it works (and that none of my asserts trigger), and make a pre-5 so that we can synch up.
Linus
- 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/
| |