steve,
would a symptom of this problem be random lockups under heavy ide load?
yesterday, i was burning some cds from my (ide) hard-drives and it just
randomly locked up two or three times. there were no messages in the
logs. just to be sure that it wasn't x locking up, i tried to access
the machine from another machine on my network and couldn't. even pings
weren't returned.
i'm running 2.3.99-pre6 on a dual p3-600 (smp-enabled kernel), asus
p2b-d mobo, 512m ram (actually, 480m because of glx).
thanks,
pete
On Sat, 06 May 2000, Steve Dodd wrote:
> Hi Linus,
>
> I believe the attached patch fixes a potential race in buffer.c:getblk() on
> SMP machines. Most callers currently take the big kernel lock first, but
> some don't - block_write, for example. I also nuked the comment at the top
> of get_hash_table, because it really doesn't seem to make sense..
> --- buffer.c.orig Sat May 6 12:33:38 2000
> +++ buffer.c Sat May 6 16:14:02 2000
> @@ -28,6 +28,8 @@
>
> /* async buffer flushing, 1999 Andrea Arcangeli <andrea@suse.de> */
>
> +/* [6-May-2000, Steve Dodd] fix SMP race in getblk() */
> +
> #include <linux/config.h>
> #include <linux/sched.h>
> #include <linux/fs.h>
> @@ -495,17 +497,6 @@
> __remove_from_lru_list(bh, bh->b_list);
> }
>
> -static void insert_into_queues(struct buffer_head *bh)
> -{
> - struct buffer_head **head = &hash(bh->b_dev, bh->b_blocknr);
> -
> - spin_lock(&lru_list_lock);
> - write_lock(&hash_table_lock);
> - __hash_link(bh, head);
> - __insert_into_lru_list(bh, bh->b_list);
> - write_unlock(&hash_table_lock);
> - spin_unlock(&lru_list_lock);
> -}
>
> /* This function must only run if there are no other
> * references _anywhere_ to this buffer head.
> @@ -530,19 +521,11 @@
> spin_unlock(&head->lock);
> }
>
> -/*
> - * Why like this, I hear you say... The reason is race-conditions.
> - * As we don't lock buffers (unless we are reading them, that is),
> - * something might happen to it while we sleep (ie a read-error
> - * will force it bad). This shouldn't really happen currently, but
> - * the code is ready.
> - */
> -struct buffer_head * get_hash_table(kdev_t dev, int block, int size)
> +static struct buffer_head * __get_hash_table(kdev_t dev, int block, int size,
> + struct buffer_head **head)
> {
> - struct buffer_head **head = &hash(dev, block);
> struct buffer_head *bh;
>
> - read_lock(&hash_table_lock);
> for(bh = *head; bh; bh = bh->b_next)
> if (bh->b_blocknr == block &&
> bh->b_size == size &&
> @@ -550,11 +533,44 @@
> break;
> if (bh)
> atomic_inc(&bh->b_count);
> +
> + return bh;
> +}
> +
> +struct buffer_head * get_hash_table(kdev_t dev, int block, int size)
> +{
> + struct buffer_head **head = &hash(dev, block);
> + struct buffer_head *bh;
> +
> + read_lock(&hash_table_lock);
> + bh = __get_hash_table(dev, block, size, head);
> read_unlock(&hash_table_lock);
>
> return bh;
> }
>
> +static int insert_into_queues_unique(struct buffer_head *bh)
> +{
> + struct buffer_head **head = &hash(bh->b_dev, bh->b_blocknr);
> + struct buffer_head *alias;
> + int err = 0;
> +
> + spin_lock(&lru_list_lock);
> + write_lock(&hash_table_lock);
> +
> + alias = __get_hash_table(bh->b_dev, bh->b_blocknr, bh->b_size, head);
> + if (!alias) {
> + __hash_link(bh, head);
> + __insert_into_lru_list(bh, bh->b_list);
> + } else
> + err = 1;
> +
> + write_unlock(&hash_table_lock);
> + spin_unlock(&lru_list_lock);
> +
> + return err;
> +}
> +
> unsigned int get_hardblocksize(kdev_t dev)
> {
> /*
> @@ -831,9 +847,8 @@
> spin_unlock(&free_list[isize].lock);
>
> /*
> - * OK, FINALLY we know that this buffer is the only one of
> - * its kind, we hold a reference (b_count>0), it is unlocked,
> - * and it is clean.
> + * OK, we hold a reference (b_count>0) to the buffer,
> + * it is unlocked, and it is clean.
> */
> if (bh) {
> init_buffer(bh, end_buffer_io_sync, NULL);
> @@ -841,8 +856,16 @@
> bh->b_blocknr = block;
> bh->b_state = 1 << BH_Mapped;
>
> - /* Insert the buffer into the regular lists */
> - insert_into_queues(bh);
> + /* Insert the buffer into the regular lists; check nobody
> + else added it first */
> +
> + if (!insert_into_queues_unique(bh))
> + goto out;
> +
> + /* someone added it after we last checked the hash table */
> + put_last_free(bh);
> + goto repeat;
> +
> out:
> touch_buffer(bh);
> return bh;
-- Pete Toscano h:sigsegv@psinet.com w:ptoscano@netsol.com GPG fingerprint: AE5C 18E4 D069 76D3 9B9C D226 D86A 522F 446C 767A
- 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/
This archive was generated by hypermail 2b29 : Mon May 15 2000 - 21:00:11 EST