Re: Bad SMP race in getblk()

From: Tigran Aivazian (tigran@veritas.com)
Date: Wed Aug 02 2000 - 14:22:02 EST


On Wed, 2 Aug 2000, Andrea Arcangeli wrote:
> +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;
> +}

But on UP the above alias = __get_hash_table() is not really needed. I
know that page cache code contains similar "insert unique" tricks which
hurt UP performance but why not think of a better way to approach it,
while we are at it?

At the very least put ifdef CONFIG_SMP around it and admit that it's an
ugly thing to do.

Mark spotted this issue (and a few others) but I have not seen any
suggestions on how to fix them nicely (and the conditional compilation is
not a nice thing!) without doing:

cd fs ; rm buffer.c ; think ; vi buffer.c

I cc'd the original author of the patch - maybe he has good ideas on how
to improve his own patch?

Regards,
Tigran

-
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 Aug 07 2000 - 21:00:09 EST