Re: The INN/mmap bug

From: Daniel Phillips (news-innominate.list.linux.kernel@innominate.de)
Date: Mon Sep 18 2000 - 08:45:01 EST


Alexander Viro wrote:
>
> On Sun, 17 Sep 2000, Linus Torvalds wrote:
>
> > On Sun, 17 Sep 2000, Alexander Viro wrote:
> > >
> > > Looks sane. But I really wonder if we could just do it in
> > > create_page_buffers() if page is up-to-date. OTOH it would require attempt
> > > to map them all. Comments?
> >
> > That would certainly simplify a lot.
> >
> > And as we've seen, simplifying this area would not necessarily be a bad
> > thing ;)
> >
> > Right now I'll just do the minimal fix, though, I think.
>
> Fine with me. I'll do the full analysis tonight, anyway, and try to write
> down the rules for that stuff. One thing that makes me seriously uneasy
> is the fact that VM knows about ->buffers - I would be much happier if all
> this stuff would be contained in fs/buffer.c. IOW, I'm not sure that
> block_flushpage()/try_to_free_buffers() is a happy camper.

As you know, I've begun to analyse it;

  Canonic buffer states and transitions
  http://marc.theaimsgroup.com/?l=linux-fsdevel&m=96893011609105&w=2

Since that post I've found one additional useful buffer state (*)
bringing the total to 5:

  0: undef - Neither data on disk or in cache known to be valid
  1: inval - Data on disk known to be invalid (*)
  2: known - Data on disk known to be valid
  3: dirty - Data in cache known to be valid
  4: clean - Data in cache known to match data on disk

This is still a lot less that the current 16 states. I'm still
waiting for comments on this analysis one way or the other. I think
it strikes at the heart of the problem. I already used this way of
thinking to find and fix a similar bug in my tailmerging code where
__block_prepare_write was wrongly clearing part of a dirty buffer just
because ext2_get_block reported it had created a new block. The quick
fix was to suppress the buffer_new bit coming back from ext2_get_block
if the buffer is dirty. A better fix would be to eliminate the
buffer_new bit entirely, but of course this can wait.

My contribution to this will be to update the buffer states post and
if nobody thinks I'm completely off-base, do a similar one for page
states. It seems to me that you and Linus have already found exactly
the crevice that this bug lives in.

> I'm not proposing it for immediate inclusion, but I don't feel good about
> all this code - current rules are too complex and rely on details of
> VM/buffer interaction too much for my taste. It may be correct, but it's
> not obviously correct...

Amen.

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



This archive was generated by hypermail 2b29 : Sat Sep 23 2000 - 21:00:17 EST