Date: Sat, 21 Aug 2010 17:18:50 -0400 From: Stephan Uphoff <ups@freebsd.org> To: Max Laier <max@love2party.net> Cc: freebsd-arch@freebsd.org Subject: Re: rmlock(9) two additions Message-ID: <4C7042BA.8000402@freebsd.org> In-Reply-To: <201008160515.21412.max@love2party.net> References: <201008160515.21412.max@love2party.net>
next in thread | previous in thread | raw e-mail | index | archive | help
Max Laier wrote:
> Hi,
>
> I'd like to run two additions to rmlock(9) by you:
>
> 1) See the attached patch. It adds rm_try_rlock() which avoids taking the
> mutex if the lock is currently in a write episode. The only overhead to the
> hot path is an additional argument and a return value for _rm_rlock*. If you
> are worried about that, it can obviously be done in a separate code path, but
> I reckon it not worth the code crunch. Finally, there is one additional
> branch to check the "trylock" argument, but that's well past the hot path.
>
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.
Alternatively something like:
if (trylock) {
if(mtx_trylock( &rm->rm_lock) == 0)
return (0);
}
else
{
mtx_lock(&rm->rm_lock);
}
would work - but has a race. Two readers colliding just after a writer
(with the second not succeeding in trylocking the mutex) leads to not
granting the read lock (also it would be possible to do so).
Let me think about it a bit.
> 2) No code for this yet - testing the waters first. I'd like to add the
> ability to replace the mutex for writer synchronization with a general lock -
> especially to be able to use a sx(9) lock here.
>
> The reason for #2 is the following use case in a debugging facility:
>
> "reader":
> if (rm_try_rlock()) {
> grab_per_cpu_buffer();
> fill_per_cpu_buffer();
> rm_runlock();
> }
>
> "writer" - better exclusive access thread:
> rm_wlock();
> collect_buffers_and_copy_out_to_userspace();
> rm_wunlock();
>
> This is much cleaner and possibly cheaper than the various hand rolled
> versions I've come across, that try to get the same synchronization with
> atomic operations. If we could sleep with the wlock held, we can also avoid
> copying the buffer contents, or swapping buffers.
>
> Is there any concern about either of this? Any objection? Input?
>
>
Will take a look at your second patch soonish.
Just ask per IPI for a copy of per cpu buffers (but not a copy to user
space) - and delay the copy when an update is in progress?
Stephan
> Thanks,
> Max Laier
>
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C7042BA.8000402>