Messages in this thread | | | Date | Sun, 27 Jun 1999 12:45:36 +0200 (CEST) | From | Ingo Molnar <> | Subject | Re: [patch] pagecache-2.3.9-H3, bmap & ext2fs cleanup patch |
| |
On Sat, 26 Jun 1999, Linus Torvalds wrote:
> > if (!buffer_mapped(bh)) { > > err = inode->i_op->get_block(inode, block, bh, 1); > > if (err) > > goto out; > > } > > > > if (!buffer_uptodate(bh) && (start_offset || > > (end_bytes && (i == end_block)))) { > > > > if (buffer_new(bh)) { > > memset(bh->b_data, 0, bh->b_size); > > } else { > > lock_kernel(); > > ll_rw_block(READ, 1, &bh);
> > this will obviously fail if holes are a little bit different and are not > > uptodate anymore. > > "obviously fail" HOW?
i'll have to change the above to do something like:
was_hole = 0; if (!buffer_mapped(bh)) { if (buffer_uptodate(bh) && !buffer_dirty(bh)) was_hole = 1; err = inode->i_op->get_block(inode, block, bh, 1); if (err) goto out; }
if (!buffer_uptodate(bh) && (start_offset || (end_bytes && (i == end_block)))) { if (!buffer_new(bh)) { ll_rw_block(READ); } }
if ((start_offset || (end_bytes && (i == end_block))) ) && (buffer_new(bh) || was_hole)) clear_untouched_area();
memcpy();
look how much this has changed relative to the original version, because due to the (Uptodate && !Mapped == Hole) rule i cannot naturally fall into the !Uptodate path where i'd otherwise do the clear_rest(). ( The point is that (Uptodate && !Mapped) makes sense _now_, but it doesnt make sense if eventually holes are _not_ kept uptodate anymore. They are not kept uptodate in this case because i'm trying to do an optimization of 'late clearing' of holes, ie. 'late uptodate' (in fact never) holes.)
with an independent buffer_hole() bit i could do a straightforward:
was_hole = 0; if (buffer_hole(bh)) was_hole = 1; if (!buffer_mapped(bh) || was_hole) { err = inode->i_op->get_block(inode, block, bh, 1); if (err) goto out; }
if (!buffer_uptodate(bh) && (start_offset || (end_bytes && (i == end_block)))) { if (buffer_new(bh) || was_hole) clear_untouched_area(); else ll_rw_block(READ); }
memcpy();
i think this one is much cleaner - and just a natural extension of the old version. These problems arise because currently a mapping concept (holes) are tied to the Uptodate bit - but once holes want to be !Uptodate, the code breaks in nasty ways.
it might have one more condition in the get_block() path, but i think thats clean as well, holes _are_ special. [It's not a problem in the above example anyway because we already calculate that condition.]
I totally agree that putting buffer_hole() into the _current_ code just increases complexity unnecesserily. But robustness wrt. changing the behavior of holes might not be a too important reason - and as i agreed too, pushing the fs-hole concept into the VM [we need that for the above thing to make sense in the first place] is not necesserily a good move. Maybe i was really just trying to overdesign holes :)
-- mingo
- 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/
| |