Message-ID: <41D52176.80703@trash.net>
Date: Fri, 31 Dec 2004 10:52:54 +0100
From: Patrick McHardy <kaber@trash.net>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.6) Gecko/20040413 Debian/1.6-5
MIME-Version: 1.0
To: hadi@cyberus.ca
CC: Thomas Graf <tgraf@suug.ch>, "David S. Miller" <davem@davemloft.net>,
        netdev@oss.sgi.com
Subject: Re: [PATCH 2/9] PKT_SCHED: tc filter extension API
References: <20041230122652.GM32419@postel.suug.ch>	 <20041230123023.GO32419@postel.suug.ch>  <41D4A4D2.4000109@trash.net> <1104469362.1049.224.camel@jzny.localdomain>
In-Reply-To: <1104469362.1049.224.camel@jzny.localdomain>
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Transfer-Encoding: 7bit
Sender: netdev-bounce@oss.sgi.com
Errors-to: netdev-bounce@oss.sgi.com
Precedence: bulk

jamal wrote:
> On Thu, 2004-12-30 at 20:01, Patrick McHardy wrote:
> 
>>This isn't right (its also wrong in the current code). If the
>>CPU reorders stores and another CPU looks at dst->action at
>>the wrong time it might see an inconsistent structure. 
> 
> 
> I think an xchg around the else should fix this.

Yes.

>>I also wonder if anyone
>>actually knows why we need the xchg (here and in all the other
>>places), it looks totally useless.
> 
> All these were put in by Alexey and the LinuxWay(tm) took effect.
> an xchg puts almost a lock and ensures an atomic swap. I dont see any
> harm in leaving it as is - just needs fixing the else

No real harm, but it still should be removed IMO, or used _instead_
of tcf_tree_lock in this place. I've asked myself multiple times what
it is meant for and I've seen others do the same, this alone justifies
removing it. Another reason is what you call LinuxWay(tm), strange
things spread on their own and at some time you have to touch lots of
files to get rid them. So its best to do it as early as possible.

Regards
Patrick