Re: userret: assert td_lk_slocks == 0

2019-08-12 Thread Andriy Gapon
On 12/08/2019 13:49, Mateusz Guzik wrote:
> On 8/12/19, Andriy Gapon  wrote:
>>
>> I am trying to debug a leak of a shared vnode lock and I noticed that
>> there is no check for td_lk_slocks in userret.  There are checks for
>> td_rw_rlocks and td_sx_slocks.  I wonder if there is any valid scenario
>> where a thread is allowed to retain a shared lock manager lock across
>> system calls.
>>
> 
> These counters are not for debugging purposes. They are part of poor
> man's starvation prevention for writers.

Yes, I realize that.

> If the target lock is taken for reading and someone wants to take it for
> writing, a bit will be set to denote this fact and prevent more readers
> from showing up. However, this can lead to deadlocks so if someone
> already has a read lock on something, they can bypass the bit and
> grab the extra read lock anyway.
> 
> No locks are allowed to leak back to userspace and witness should
> already handles checking this for readers as well.

Yes.
But since we have those asserts for td_rw_rlocks and td_sx_slocks,
wouldn't it make sense to add one for td_lk_slocks?
If it's considered superfluous for FreeBSD, then at least I'll add it in
the work's fork.


-- 
Andriy Gapon
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: userret: assert td_lk_slocks == 0

2019-08-12 Thread Konstantin Belousov
On Mon, Aug 12, 2019 at 12:14:25PM +0300, Andriy Gapon wrote:
> 
> I am trying to debug a leak of a shared vnode lock and I noticed that
> there is no check for td_lk_slocks in userret.  There are checks for
> td_rw_rlocks and td_sx_slocks.  I wonder if there is any valid scenario
> where a thread is allowed to retain a shared lock manager lock across
> system calls.

There is a situation where thread returns while keeping the lockmgr lock
busied.  This is used by buffer cache to keep everybody hands away of
async buffers until io is finished.  But the ownership of the lock is
erased, and the thread's slocks count is decremented.

I think it should be correct to add the assert you proposed.
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


Re: userret: assert td_lk_slocks == 0

2019-08-12 Thread Mateusz Guzik
On 8/12/19, Andriy Gapon  wrote:
>
> I am trying to debug a leak of a shared vnode lock and I noticed that
> there is no check for td_lk_slocks in userret.  There are checks for
> td_rw_rlocks and td_sx_slocks.  I wonder if there is any valid scenario
> where a thread is allowed to retain a shared lock manager lock across
> system calls.
>

These counters are not for debugging purposes. They are part of poor
man's starvation prevention for writers.

If the target lock is taken for reading and someone wants to take it for
writing, a bit will be set to denote this fact and prevent more readers
from showing up. However, this can lead to deadlocks so if someone
already has a read lock on something, they can bypass the bit and
grab the extra read lock anyway.

No locks are allowed to leak back to userspace and witness should
already handles checking this for readers as well.

-- 
Mateusz Guzik 
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"


userret: assert td_lk_slocks == 0

2019-08-12 Thread Andriy Gapon


I am trying to debug a leak of a shared vnode lock and I noticed that
there is no check for td_lk_slocks in userret.  There are checks for
td_rw_rlocks and td_sx_slocks.  I wonder if there is any valid scenario
where a thread is allowed to retain a shared lock manager lock across
system calls.

Thanks!
-- 
Andriy Gapon
___
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"