Re: [replace-alexv-buffer.c-patch] Re: [PATCH] Several bad bugs in

Alexander Viro (viro@math.psu.edu)
Thu, 22 Apr 1999 18:59:25 -0400 (EDT)


On Thu, 22 Apr 1999, Andrea Arcangeli wrote:

> >> Starting from the second run of the loop bh is == bhnext. But bhnext is
> >> been moved from the dirty list to the clean list while we was sleeping in
> >> wait_on_buffer(). So without my patch we could continue browsing the clean
> >> list instead of continue to browse the dirty list.
> >
> >Andrea, look what will happen if lru_list[nlist]->b_list!=nlist. You will
> >not get to the end of the inner loop - it will just spin. I'll try to look
>
> But lru_list[nlist]->b_list is always == nlist, otherwise the buffer
> pointed by lru_list[nlist] wouldn't be there ;).

What the... Wait a bit. How and when can ->b_list become *not*
corrsponding to the list where the buffer sits, anyway?
AFAICS assumptions being:
a) we have unbound buffer heads (no ->b_data, no nothing) on a
single-linked list.
b) the rest is organized into cyclic lists, with heads in free_list[] and
lru_list[].
c) bh belongs to the list referenced from free_list[] <=> it is
unhashed <=> ->b_dev==B_FREE. Choice of list is controlled by ->b_size.
All such bh's should have ->b_count == 0.
d) the rest lives on lru_list[]-referenced lists. All of them are hashed
and all buffers for given device should have the same size. I.e. nothing
should call bread() before the set_blocksize() completes and nothing
should keep any pointers to bh across the set_blocksize(). Since
set_blocksize() is used only in mount/umount those assumptions seem to be
safe (and the patch I've posted fixes the last violation of the second
requirement (in fat_read_super())). For buffers of that class we have
->b_list containg the number of list.
e) Any bh on the clean list has !buffer_locked(bh) && !buffer_dirty(bh)

Ooops! I can see why md.c and raid[15].c may want to set BH_Lock
bit in irregular way, but WTF is so special about ide-tape.c that it
wants to do it?

Another question - wouldn't it be nice if we'ld add moving to the
LOCKED list in the end of lock_buffer()? That (modulo raid, md and
ide-tape) would give us
f) Every bh with buffer_locked(bh) belongs to locked list.
AFAICS that would seriously simplify fs/buffer.c.
Comments?

Disclaimer: I'm really going down now, so I could miss something very
obvious.

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