Date: Tue, 29 Nov 2005 11:41:06 +0530
From: Dipankar Sarma <dipankar@in.ibm.com>
To: Andi Kleen <ak@suse.de>
Cc: akpm@osdl.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] Allow lockless traversal of notifier lists
Message-ID: <20051129061106.GD4359@in.ibm.com>
Reply-To: dipankar@in.ibm.com
References: <20051128133757.GQ20775@brahms.suse.de> <20051128160129.GA8478@in.ibm.com> <20051128160547.GA20775@brahms.suse.de> <20051128161747.GA4359@in.ibm.com> <20051128162709.GC20775@brahms.suse.de> <20051128174203.GB4359@in.ibm.com> <20051129000158.GE7209@brahms.suse.de>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20051129000158.GE7209@brahms.suse.de>
User-Agent: Mutt/1.5.10i
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 1341
Lines: 43

On Tue, Nov 29, 2005 at 01:01:58AM +0100, Andi Kleen wrote:
> On Mon, Nov 28, 2005 at 11:12:03PM +0530, Dipankar Sarma wrote:
> 
> Ok third version, hopefully Dipankar proof now. 
> 

Not quite. I spoke without looking at the code of the whole
notifier_call_chain() function.

> + * 	against parallel traversals.
>   *
>   *	Returns zero on success, or %-ENOENT on failure.
>   */
> @@ -171,10 +177,12 @@
>  int notifier_call_chain(struct notifier_block **n, unsigned long val, void *v)
>  {
>  	int ret=NOTIFY_DONE;
> -	struct notifier_block *nb = *n;
> -
> +	struct notifier_block *nb;
> +	smp_read_barrier_depends();
> +	nb = *n;
>  	while(nb)
>  	{
> +		smp_read_barrier_depends();
>  		ret=nb->notifier_call(nb,val,v);
>  		if(ret&NOTIFY_STOP_MASK)
>  		{

Looking at the full code, it seems to me that we dereference
the first notifier block only inside the while(nb) loop.
That means the smp_read_barrier_depends() in the while(nb)
loop should be sufficient - IOW, the previous version of
the patch with one smp_read_barrier_depends() was good. 
Sorry about the confusion.

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