> > 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/