Hi,
I think I found a bug in the rw_enter() implementation (emulation?) in
libzpool, file /usr/src/lib/libzpool/common/kernel.c:
void
rw_enter(krwlock_t *rwlp, krw_t rw)
{
ASSERT(!RW_LOCK_HELD(rwlp));
ASSERT(rwlp->rw_owner != (void *)-1UL);
ASSERT(rwlp->rw_owner != curthread);
if (rw == RW_READER)
(void) rw_rdlock(&rwlp->rw_lock);
else
(void) rw_wrlock(&rwlp->rw_lock);
rwlp->rw_owner = curthread;
}
Doesn''t RW_LOCK_HELD() check if there''s a reader or a writer
locked? If it
does, then these read-write locks would produce an assertion when multiple
readers tried to lock it.
However, RW_LOCK_HELD() is being applied to "rwlp" instead
of "&rwlp->rw_lock", which could explain why it''s not
failing.
Am I understanding this correctly?
Unfortunately POSIX threads don''t have an equivalent of rw_lock_held(),
rw_write_held(), mutex_held(), ..., so I really have to understand this in
order to somehow emulate their behavior.
Thanks!
I believe RW_LOCK_HELD checks it''s not held by the calling thread only. Note, a thread should not doubly read lock the same lock as a write lock from another thread between the 2 would deadlock. Ricardo Correia wrote On 06/01/06 22:03,:> Hi, > > I think I found a bug in the rw_enter() implementation (emulation?) in > libzpool, file /usr/src/lib/libzpool/common/kernel.c: > > void > rw_enter(krwlock_t *rwlp, krw_t rw) > { > ASSERT(!RW_LOCK_HELD(rwlp)); > ASSERT(rwlp->rw_owner != (void *)-1UL); > ASSERT(rwlp->rw_owner != curthread); > > if (rw == RW_READER) > (void) rw_rdlock(&rwlp->rw_lock); > else > (void) rw_wrlock(&rwlp->rw_lock); > > rwlp->rw_owner = curthread; > } > > Doesn''t RW_LOCK_HELD() check if there''s a reader or a writer locked? If it > does, then these read-write locks would produce an assertion when multiple > readers tried to lock it. > > However, RW_LOCK_HELD() is being applied to "rwlp" instead > of "&rwlp->rw_lock", which could explain why it''s not failing. > > Am I understanding this correctly? > > Unfortunately POSIX threads don''t have an equivalent of rw_lock_held(), > rw_write_held(), mutex_held(), ..., so I really have to understand this in > order to somehow emulate their behavior. > > Thanks! > _______________________________________________ > zfs-code mailing list > zfs-code at opensolaris.org > http://opensolaris.org/mailman/listinfo/zfs-code
On Friday 02 June 2006 05:12, Neil Perrin wrote:> I believe RW_LOCK_HELD checks it''s not held by the calling thread only. > Note, a thread should not doubly read lock the same lock as > a write lock from another thread between the 2 would deadlock.Ok, that makes sense. Thanks :) I''m assuming RW_WRITE_HELD() and MUTEX_HELD() also only checks if it''s held by the calling thread (I couldn''t find any documentation on this, and the implementation of the RW_xxx_HELD() macros weren''t very easy to understand :p) Oh well... now I only have to figure out a way to emulate this with POSIX threads.. :)
On Fri, Jun 02, 2006 at 05:50:21AM +0100, Ricardo Correia wrote:> On Friday 02 June 2006 05:12, Neil Perrin wrote: > > I believe RW_LOCK_HELD checks it''s not held by the calling thread only. > > Note, a thread should not doubly read lock the same lock as > > a write lock from another thread between the 2 would deadlock. > > Ok, that makes sense. Thanks :) > > I''m assuming RW_WRITE_HELD() and MUTEX_HELD() also only checks if it''s held by > the calling thread (I couldn''t find any documentation on this, and the > implementation of the RW_xxx_HELD() macros weren''t very easy to > understand :p)Our userland threads library keeps track, of which reader locks are held by a given thread. If you look at the implementation of _rw_read_held(): http://cvs.opensolaris.org/source/xref/on/usr/src/lib/libc/port/threads/rwlock.c#_rw_read_held it checks the current thread''s list of locks to verify that it is not being held.> Oh well... now I only have to figure out a way to emulate this with POSIX > threads.. :)It''s much easier with help from the implementation; you could just disable that single assert. Cheers, - jonathan -- Jonathan Adams, Solaris Kernel Development
Ok, I took a look at the libc and the kernel implementation of rwlocks.
I''m a little worried if I got this right, because most of the ZFS code
can run
both in userspace and in the kernel, which seem to behave differently in the
RW_xxx_HELD() macros, and I need to port it correctly.
This is what I understood, please correct me if I got something wrong:
1) Like you said, libc keeps track of which reader locks are held by a given
thread
2) The kernel only keeps track of either:
2.1) The thread that has the rwlock locked for writing
2.2) Or the number of readers that have the rwlock locked for reading
3) The RW_READ_HELD() macro:
3.1) In userspace, checks if the current thread holds the rwlock locked
for reading
3.2) In the kernel, only checks if the rwlock is locked for reading (by
any thread)
4) The RW_LOCK_HELD() macro:
4.1) In userspace, is equivalent to RW_READ_HELD() || RW_WRITE_HELD(), so
basically it checks if the current thread has the rwlock locked.
4.2) In the kernel, it only checks if the rwlock is locked (!!)
Is this correct?
Thank you for helping.
On another note, I already got zdb to compile in Linux. Cool, heh? ;)
On Tuesday 06 June 2006 19:17, Jonathan Adams wrote:> Our userland threads library keeps track, of which reader locks are held by
> a given thread. If you look at the implementation of _rw_read_held():
>
> http://cvs.opensolaris.org/source/xref/on/usr/src/lib/libc/port/threads/rwl
>ock.c#_rw_read_held
>
> it checks the current thread''s list of locks to verify that it is
not being
> held.
Ricardo Correia wrote:> Ok, I took a look at the libc and the kernel implementation of rwlocks. > > I''m a little worried if I got this right, because most of the ZFS code can run > both in userspace and in the kernel, which seem to behave differently in the > RW_xxx_HELD() macros, and I need to port it correctly. > > This is what I understood, please correct me if I got something wrong: > > 1) Like you said, libc keeps track of which reader locks are held by a given > thread > 2) The kernel only keeps track of either: > 2.1) The thread that has the rwlock locked for writing > 2.2) Or the number of readers that have the rwlock locked for reading > > 3) The RW_READ_HELD() macro: > 3.1) In userspace, checks if the current thread holds the rwlock locked > for reading > 3.2) In the kernel, only checks if the rwlock is locked for reading (by > any thread) > > 4) The RW_LOCK_HELD() macro: > 4.1) In userspace, is equivalent to RW_READ_HELD() || RW_WRITE_HELD(), so > basically it checks if the current thread has the rwlock locked.Yes, and that is exactly how it is defined in synch.h #define RW_LOCK_HELD(x) (RW_READ_HELD(x) || RW_WRITE_HELD(x))> 4.2) In the kernel, it only checks if the rwlock is locked (!!) > > Is this correct?Yes, I believe you have this correct. -Mark