Re: [PATCH v2 4/7] AMSO1100 Memory Management.

[Nick Piggin <nickpiggin@yahoo.com.au>]

Tom Tucker wrote:
> On Thu, 2006-06-08 at 01:17 -0700, Andrew Morton wrote:
> 
>>On Wed, 07 Jun 2006 15:06:55 -0500
>>Steve Wise <swise@opengridcomputing.com> wrote:
>>
>>
>>>+void c2_free(struct c2_alloc *alloc, u32 obj)
>>>+{
>>>+	spin_lock(&alloc->lock);
>>>+	clear_bit(obj, alloc->table);
>>>+	spin_unlock(&alloc->lock);
>>>+}
>>
>>The spinlock is unneeded here.
> 
> 
> Good point.

Really?

clear_bit does not give you any memory ordering, so you can have
the situation where another CPU sees the bit cleared, but this
CPU still has stores pending to whatever it is being freed. Or
any number of other nasty memory ordering badness.

I'd just use the spinlocks, and prepend the clear_bit with a
double underscore (so you get the non-atomic version), if that
is appropriate.

The spinlocks nicely handle all the memory ordering issues, and
serve to document concurrency issues. If you need every last bit
of performance and scalability, that's OK, but you need comments
and I suspect you'd need more memory barriers.

-- 
SUSE Labs, Novell Inc.
Send instant messages to your online friends http://au.messenger.yahoo.com