Messages in this thread | | | Date | Mon, 5 Apr 1999 23:03:43 -0400 (EDT) | From | Chuck Lever <> | Subject | Re: [patch] some buffer.c glitches |
| |
i don't have the experience you have with some of this, so this is all a "humble opinion."
On Mon, 5 Apr 1999, Andrea Arcangeli wrote: > 2. insert_into_queues() can't be recalled with a B_FREE buffer becasue > insert_into_queues() is reacalled only by the buffer allocation code (that > removed the buffer from the freelist a bit before), or from file_buffer, > that is recalled only by refile_buffer, and refile_buffer will panic if > the buffer is the buffer passed is a B_FREE one.
this is probably left over from 2.0, but it might be a good idea to leave it in for protection against programming errors later on. based on some of the things i've been experimenting with, a branch like this is arguably not too expensive. i think we should change the error case to panic, or at least print a message.
or not. :)
> 4. both put_last_free and put_last_lru don't need to check if the buffer > passed as parameter is null, this is enforced by the calling function. > put_last_lru() is called only from here:
see 2.
> 5. if we want to touch a buffer we should do that in find_buffer() or when > we alloc a buffer removing it from the freelist (if we didn't found it in > the fuzzy hash).
agreed.
> I am also thinking about set_blocksize() and invalidate_buffers(). As > first I think that both these two functions should move all invalidated > buffers to the free_list (as bforget now does).
yes, this will help these buffers get re-used more quickly. i can imagine this being useful during a software installation from CD-ROM, for instance.
> I also don't understand why to walk every lru_list for 2 times (that's > really an overkill). The point is that if a wrong-sized buffer can be > moved to the end of the lru_list, if we are unlucky and the I/O is slow > enough, a double pass won't save us from missing an interesting bh (and > missing it in the invalidate case means corruption on the read level). To > make sure to invalidate all buffers that belongs to the kdev, we can't > rely on walking the list two times, but we must make sure that noplace in > the kernel can call put_last_lru() or adding wrong-sized buffers, while we > are invalidating/resizing buffers (no problems instead if the buffers that > we are going to invalidate will be freed while we are sleeping wakling the > list).
i'm thinking that would be hard to implement efficiently. set_blocksize() and invalidate_buffers() would have to block getblk() to do this correctly. wouldn't that be expensive to add to mainline code?
> I also think that we should put put_last_lru() in find_buffer() and not in > getblk().
same as touch_buffer() comment above -- agreed. naked find_buffer() is used in lots of places from ext2 and the raid code, isn't it?
- Chuck Lever -- corporate: <chuckl@netscape.com> personal: <chucklever@netscape.net> or <cel@monkey.org>
The Linux Scalability project: http://www.citi.umich.edu/projects/citi-netscape/
- 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/
| |