[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: spinlock requests (was RE: [Xen-devel] [Patch] don't spin with irq disabled)



On 27/03/2009 17:00, "Dan Magenheimer" <dan.magenheimer@xxxxxxxxxx> wrote:

> For rw_is_write_locked(), I had a devil of a time
> because of what appeared to be a weird code generated
> race; the obvious simple implementation failed
> periodically... apparently due to racing against
> try_readlock attempts!  (I use it in an ASSERT so it
> was a rather noticeable and spectacular failure!)
> The workaround I used below is a horrible hack
> but I haven't had problems since.

What try_readlock would that be? There is no such function. The failing
version of rw_is_write_locked() is exactly what I would implement. I don't
see how it could race other lock acquisition attempts -- if anyone could see
the lock field as >0 then read_lock attempts could spuriously fail. It
should especially definitely work if you are ASSERTing that the CPU you are
running on has the write lock. If you are ASSERTing about the lock being
held by other CPUs, perhaps there could be races in that case?

Actually I would argue that rw_is_write_locked() is hard to implement
accurately -- a reader and a writer can both update the lock field; only the
first to update wins the race; and an external observer of the lock field
cannot tell whether the reader or writer won unless it spins and waits for
the failed party to undo its update. But this can only result in false
positives (returns TRUE when the writer has actually failed to grab the
lock) I think, which will generally be benign for the kinds of assertion
statements you want to write. OTOH it makes me uncertain about providing
this function, and I wonder if you should just use rw_is_locked().

 -- Keir



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.