lkml.org 
[lkml]   [1999]   [Mar]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [patch] fix for buffer hash leakage
On Fri, 19 Mar 1999, Chuck Lever wrote:

>[ -3 lines, +2 lines, for those of you keeping track ]
>
>void __bforget(struct buffer_head * buf)
>{
> clear_bit(BH_Protected, &buf->b_state);
^^^^^^^^^ this is not needed anymore
> remove_from_queues(buf);
> put_last_free(buf);

Agreed, my detection of the bug was ok, but _your_ one is the _right_ fix.
I simply forgot to put the buffer in the freelist due the lack of sleep
;). With my wrong fix the forgotten buffer had no way to be reused fast so
it wouldn't change so much the bad behavour... I understood this when I
was in the bad (not fun..) but it was really too much late to rediff a new
one... ;))

>ps: andrea how do these throughput numbers sound to you? :)
>
> run 1 run 2 run 3 run 4 run 5
> 4227.1 4247.4 4237.2 4247.4 4240.4
>
>you see, even inter-run variation is small!

Cool! Just for the record the number before the fix was:

4xxx 1xxx 1xxx 1xxx 1xxx ....
Here the real fix that can be applyed with `patch`:

Index: fs/buffer.c
===================================================================
RCS file: /var/cvs/linux/fs/buffer.c,v
retrieving revision 1.1.1.6
diff -u -r1.1.1.6 buffer.c
--- buffer.c 1999/03/09 00:53:50 1.1.1.6
+++ linux/fs/buffer.c 1999/03/20 10:41:34
@@ -76,6 +76,7 @@
static int nr_buffers_type[NR_LIST] = {0,};
static int nr_buffer_heads = 0;
static int nr_unused_buffer_heads = 0;
+static int nr_hashed_buffers = 0;

/* This is used by some architectures to estimate available memory. */
int buffermem = 0;
@@ -434,6 +435,7 @@
*pprev = next;
bh->b_pprev = NULL;
}
+ nr_hashed_buffers--;
}

static inline void remove_from_lru_list(struct buffer_head * bh)
@@ -568,6 +570,7 @@
*bhp = bh;
bh->b_pprev = bhp;
}
+ nr_hashed_buffers++;
}
}

@@ -845,11 +850,8 @@
*/
void __bforget(struct buffer_head * buf)
{
- mark_buffer_clean(buf);
- clear_bit(BH_Protected, &buf->b_state);
- remove_from_hash_queue(buf);
- buf->b_dev = NODEV;
- refile_buffer(buf);
+ remove_from_queues(buf);
+ put_last_free(buf);
if (!--buf->b_count)
return;
printk("VFS: forgot an in-use buffer! (count=%d)\n",
@@ -1010,9 +1011,9 @@

/* This is critical. We can't swap out pages to get
* more buffer heads, because the swap-out may need
- * more buffer-heads itself. Thus SLAB_ATOMIC.
+ * more buffer-heads itself. Thus SLAB_BUFFER.
*/
- if((bh = kmem_cache_alloc(bh_cachep, SLAB_ATOMIC)) != NULL) {
+ if((bh = kmem_cache_alloc(bh_cachep, SLAB_BUFFER)) != NULL) {
memset(bh, 0, sizeof(*bh));
nr_buffer_heads++;
return bh;
@@ -1491,6 +1496,7 @@
printk("Buffer memory: %6dkB\n",buffermem>>10);
printk("Buffer heads: %6d\n",nr_buffer_heads);
printk("Buffer blocks: %6d\n",nr_buffers);
+ printk("Buffer hashed: %6d\n",nr_hashed_buffers);

for(nlist = 0; nlist < NR_LIST; nlist++) {
found = locked = dirty = used = lastused = protected = 0;


This patch include also the monitoring change (but note that this time the
hashed buffers must decrease, but with the difference that now we'll be
able to istant reuse them...).

It also changed a SLAB_ATOMIC to SLAB_BUFFER since this last it's just I/O
safe but will allow shrink_mmap to run (and I am using it from ages
without problems).

Chuck, now that this critical bug is been fixed, could you also try your
nice bench with ftp://e-mind.com/pub/linux/arca-tree/2.2.3_arca12.gz ? ;)

Thanks. ;))

Andrea Arcangeli


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

\
 
 \ /
  Last update: 2005-03-22 13:50    [from the cache]
©2003-2014 Jasper Spaans. Advertise on this site