Date: Tue, 24 Aug 2010 10:02:34 -0400 From: Stephan Uphoff <ups@freebsd.org> To: Max Laier <max@laiers.net> Cc: freebsd-arch@freebsd.org Subject: Re: rmlock(9) two additions Message-ID: <4C73D0FA.5030102@freebsd.org> In-Reply-To: <201008240045.15998.max@laiers.net> References: <201008160515.21412.max@love2party.net> <4C7042BA.8000402@freebsd.org> <201008222105.47276.max@love2party.net> <201008240045.15998.max@laiers.net>
index | next in thread | previous in thread | raw e-mail
Max Laier wrote:
> On Sunday 22 August 2010 21:05:47 Max Laier wrote:
>
>> On Saturday 21 August 2010 23:18:50 Stephan Uphoff wrote:
>>
>>> Max Laier wrote:
>>> ...
>>> The rm_try_rlock() will never succeed when the lock was last locked as
>>> writer.
>>> Maybe add:
>>>
>>> void
>>> _rm_wunlock(struct rmlock *rm)
>>> {
>>> + rm->rm_noreadtoken = 0;
>>>
>>> mtx_unlock(&rm->rm_lock);
>>>
>>> }
>>>
>>> But then
>>>
>>> _rm_wlock(struct rmlock *rm)
>>>
>>> always needs to use IPIs - even when the lock was used last as a write
>>> lock.
>>>
>> I don't think this is a big problem - I can't see many use cases for
>> rmlocks where you'd routinely see repeated wlocks without rlocks between
>> them. However, I think there should be a release memory barrier
>> before/while clearing rm_noreadtoken, otherwise readers may not see the
>> data writes that are supposed to be protected by the lock?!?
>>
>>> ...
>>>
>> I believe either solution will work. #1 is a bit more in the spirit of the
>> rmlock - i.e. make the read case cheap and the write case expensive. I'm
>> just not sure about the lock semantics.
>>
>> I guess a
>>
>> atomic_store_rel_int(&rm->rm_noreadtoken, 0);
>>
>> should work.
>>
>
> thinking about this for a while makes me wonder: Are readers really guaranteed
> to see all the updates of a writer - even in the current version?
>
> Example:
>
> writer thread:
> rm_wlock(); // lock mtx, IPI, wait for reader drain
> modify_data();
> rm_wunlock(); // unlock mtx (this does a atomic_*_rel)
>
> reader thread #1:
> // failed to get the lock, spinning/waiting on mtx
> mtx_lock(); // this does a atomic_*_acq -> this CPU sees the new data
> rm->rm_noreadtoken = 0; // now new readers can enter quickly
> ...
>
> reader thread 2# (on a different CPU than reader #1):
> // enters rm_rlock() "after" rm_noreadtoken was reset -> no memory barrier
> // does this thread see the modifications?
>
> I realize this is a somewhat pathological case, but would it be possible in
> theory? Or is the compiler_memory_barrier() actually enough?
>
> Otherwise, I think we need an IPI on rm_wunlock() that does a atomic_*_acq on
> every CPU.
>
> Thoughts?
> Max
>
>
Yes - this is a problem that needs to be addressed.
Fortunately most platforms won't need to be as strict and I suggest per
platform parameters.
An alternative that was in my original design was to use a bitmap for
the rm_noreadtoken.
Each CPU would then have an associated bit that will only be cleared by
that cpu.
This would also allow targeted IPIs to only the token holders.
Stephan
home |
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C73D0FA.5030102>