2001-02-02 18:09:45

by Daniel Phillips

[permalink] [raw]
Subject: SMP Race in brelse

There is a rare SMP race in brelse:

1138 void __brelse(struct buffer_head * buf)
1139 {
1140 if (atomic_read(&buf->b_count)) {
1141 atomic_dec(&buf->b_count);
1142 return;
1143 }
1144 printk("VFS: brelse: Trying to free free buffer\n");
1145 }

cpu1 cpu2

Starting with buf->b_count = 1, if we have:

if (atomic_read(&buf->b_count))
if (atomic_read(&buf->b_count))
atomic_dec(&buf->b_count);
atomic_dec(&buf->b_count);

buf->b_count is now 0, but it should be -1, we fail to to report
an erroneous extra brelse.

--
Daniel


2001-02-02 18:24:10

by Tigran Aivazian

[permalink] [raw]
Subject: Re: SMP Race in brelse

Hi Daniel,

That is very well known (I posted about it many years ago :) but, as Ingo
(or someone else? maybe sct or Alan? actually, I think it was Andrea)
explained it is not a bug -- for that if() is only for purpose of catching
bad callers (which, in perfect world, shouldn't exist). The whole brelse()
could just contain a single atomic_dec() and that is all.

Regards,
Tigran

PS. Having thought about it -- it was neither sct, nor Alan, nor even
Andrea -- it was Linus who explained it :)


On Fri, 2 Feb 2001, Daniel Phillips wrote:

> There is a rare SMP race in brelse:
>
> 1138 void __brelse(struct buffer_head * buf)
> 1139 {
> 1140 if (atomic_read(&buf->b_count)) {
> 1141 atomic_dec(&buf->b_count);
> 1142 return;
> 1143 }
> 1144 printk("VFS: brelse: Trying to free free buffer\n");
> 1145 }
>
> cpu1 cpu2
>
> Starting with buf->b_count = 1, if we have:
>
> if (atomic_read(&buf->b_count))
> if (atomic_read(&buf->b_count))
> atomic_dec(&buf->b_count);
> atomic_dec(&buf->b_count);
>
> buf->b_count is now 0, but it should be -1, we fail to to report
> an erroneous extra brelse.
>
>