On Tue, May 04, 2010 at 03:34:59PM +0530, Aneesh Kumar K. V wrote: > > bh flags are not set here. This check should be based on map.m_flags. Good catch, thanks. > Only thing i am worried about is we were modifying bh_flags in all > possible confusing ways. We may want to make sure we get the update > correct. I am still going through the patch after applying it to the > tree. So may take more time to look at the full changeset. I'm concerned about that as well; in fact I'm not sure what we had before was completely correct, and I *still* have trouble reasoning about how all the flags work and why we do some of the things that we do based on how the flags are set --- and that scares me. XFS doesn't jump through *nearly* as many hoops as we do --- and given XFS's reputation for complexity, that's saying a lot! --- and I suspect some of the things we do are mandated by the fs/buffer.c and fs/direct_io.c, the latter of which does some very strange and unnatural things with buffer_heads --- and some of the things we do are based on our own, ext4-specific logic and how we route state through the many layers of callback functions. I think I've figured some of this out, but it's gotten very crufty over the course of ext4's development. One of the reasons I had for doing this cleanup, in addition to reducing stack usage (which I have measured as around 120 bytes or so on an 32-bit system, and I'm sure it's got to be more on a 64-bit system) was to make explicit how we were modifying the bh_flags. At least now we can grep for EXT4_MAP_* and see on the places where we were mucking with things. - Ted -- To unsubscribe from this list: send the line "unsubscribe linux-ext4" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html