linux-kernel - Re: [PATCH v3 2/5] locking/rwsem: Limit # of null owner retries for handoff writer

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <7dc61070-4696-4668-eaef-efae8d7fa271@redhat.com>
Date:   Tue, 25 Oct 2022 15:00:29 -0400
From:   Waiman Long <longman@...hat.com>
To:     Hillf Danton <hdanton@...a.com>,
        Peter Zijlstra <peterz@...radead.org>
Cc:     Ingo Molnar <mingo@...hat.com>, Will Deacon <will@...nel.org>,
        Boqun Feng <boqun.feng@...il.com>,
        linux-kernel@...r.kernel.org, john.p.donnelly@...cle.com,
        Mukesh Ojha <quic_mojha@...cinc.com>,
        Ting11 Wang 王婷 <wangting11@...omi.com>
Subject: Re: [PATCH v3 2/5] locking/rwsem: Limit # of null owner retries for
 handoff writer

On 10/25/22 10:58, Hillf Danton wrote:
>> @@ -1179,15 +1171,12 @@ rwsem_down_write_slowpath(struct rw_semaphore *sem, int state)
>>   		if (waiter.handoff_set) {
>>   			enum owner_state owner_state;
>>   
>> -			preempt_disable();
>>   			owner_state = rwsem_spin_on_owner(sem);
>> -			preempt_enable();
>> -
>>   			if (owner_state == OWNER_NULL)
>>   				goto trylock_again;
>>   		}
> 	__up_write()
> 	{
> 	rwsem_clear_owner(sem);
> 	/*
> 	 If lockup can happen when a bound kworker gets preempted here by
> 	 a FIFO acquirer for write, this is a case of preemption deeper
> 	 than thought IMO
> 	*/
> 	tmp = atomic_long_fetch_add_release(-RWSEM_WRITER_LOCKED, &sem->count);
> 	if (unlikely(tmp & RWSEM_FLAG_WAITERS))
> 		rwsem_wake(sem);
>
A preempt_disable()/preempt_enable() pair has been added by commit 
48dfb5d2560 ("locking/rwsem: Disable preemption while trying for rwsem 
lock") to __up_write(). So that should not be a problem. However, that 
does make this change, if implemented, has dependency on the coexistence 
of the previous mentioned commit to be functionally complete.

Cheers,
Longman