Messages in this thread |  | | | Date | Tue, 31 Oct 2006 20:40:21 +1100 | | From | Nick Piggin <> | | Subject | Re: [PATCH] splice : two smp_mb() can be omitted |
| |
Eric Dumazet wrote: > This patch deletes two calls to smp_mb() that were done after > mutex_unlock() that contains an implicit memory barrier.
Uh, there is nothing that says mutex_unlock or any unlock functions contain an implicit smp_mb(). What is given is that the lock and unlock obey aquire and release memory ordering, respectively.
a = x; xxx_unlock b = y;
In this situation, the load of y can be executed before that of x. And some architectures will even do so (i386 can, because the unlock is an unprefixed store; ia64 can, because it uses a release barrier in the unlock).
Whenever you rely on orderings of things *outside* locks (even partially outside), you do need to be very careful about barriers and can't rely on locks to do the right thing for you.
> > The first one in splice_to_pipe(), where 'do_wakeup' is set to true only > if pipe->inode is set (and in this case the > if (pipe->inode) > mutex_unlock(&pipe->inode->i_mutex); > is done too) > > The second one in link_pipe(), following inode_double_unlock() that > contains calls to mutex_unlock() too.
It *may* be the case that these can be removed, but not by virtue of the fact that the smp_mb is redundant.
> > Signed-off-by: Eric Dumazet <dada1@cosmosbay.com> > > > ------------------------------------------------------------------------ > > --- linux/fs/splice.c 2006-10-31 07:49:52.000000000 +0100 > +++ linux-ed/fs/splice.c 2006-10-31 08:04:58.000000000 +0100 > @@ -248,7 +248,6 @@ > mutex_unlock(&pipe->inode->i_mutex); > > if (do_wakeup) { > - smp_mb(); > if (waitqueue_active(&pipe->wait)) > wake_up_interruptible(&pipe->wait); > kill_fasync(&pipe->fasync_readers, SIGIO, POLL_IN); > @@ -1518,7 +1517,6 @@ > * If we put data in the output pipe, wakeup any potential readers. > */ > if (ret > 0) { > - smp_mb(); > if (waitqueue_active(&opipe->wait)) > wake_up_interruptible(&opipe->wait); > kill_fasync(&opipe->fasync_readers, SIGIO, POLL_IN);
-- SUSE Labs, Novell Inc. Send instant messages to your online friends http://au.messenger.yahoo.com
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
|  |