Re: git: 02ea6033020e - main - LinuxKPI: Allow spin_lock_irqsave to be called within a critical section

From: Vladimir Kondratyev <vladimir_at_kondratyev.su>
Date: Sat, 22 Jan 2022 11:04:20 UTC
On 22.01.2022 12:49, Hans Petter Selasky wrote:
> On 1/19/22 23:02, Vladimir Kondratyev wrote:
>> On 19.01.2022 12:50, Hans Petter Selasky wrote:
>>> On 1/18/22 22:35, Vladimir Kondratyev wrote:
>>>>
>>>> Any ideas how to avoid it in a generic way?
>>>
>>> Hi,
>>>
>>> On a non-SMP system this will lead to deadlock.
>>>
>>> Is it possible you can pre-lock this spin lock earlier, so that it is already 
>>> locked, so instead of
>>>
>>> while(trylock());
>>>
>>> You have:
>>>
>>> assert (trylock() XXX)
>>>
>>
>> Unfortunately, vulnerable functions are called in too many code paths to patch 
>> them all.
>>
>>> Or else,
>>>
>>> convert this particular lock to a native FreeBSD spinlock mutex.
>>>
>> It can be done for wake_up() but not for dma_fence_signal() which suffers from 
>> this problem too. Some code that uses that lock expect it to be spinlock_t
>>
>> I think we can just drop critical section in seqlock-related part of dma-buf 
>> code and replace it with rwlock as kib@ and mjg@ suggested. Leave seqlock for 
>> actual locking to preserve semantics as much as possible and add rwlock to 
>> implement reader's blocking. Following snippets show code conversion required 
>> for this change:
>>
>>
>> Lock seqlock as reader:
>>
>> retry:
>>      seq = read_seqcount_begin(&obj->seq);
>>
>> ... reader payload ...
>>
>>      if (read_seqcount_retry(&obj->seq, seq)) {
>> #ifdef __FreeBSD__
>>          /* Wait for rwlock to be released by writer */
>>          rw_rlock(&obj->rwlock);
>>          rw_runlock(&obj->rwlock);
>> #endif
>>          goto retry;
>>      }
>>
>>
>> Lock seqlock as writer:
>>
>> #ifdef __linux__
>>      preempt_disable();
>> #elif defined (__FreeBSD__)
>>      rw_wlock(&obj->rwlock);
>> #endif
>>      write_seqcount_begin(&obj->seq);
>>
>> ... writer payload ...
>>
>>      write_seqcount_end(&obj->seq);
>> #ifdef __linux__
>>      preempt_enable();
>> #elif defined (__FreeBSD__)
>>      rw_wunlock(&obj->rwlock);
>> #endif
>>
> 
> Hi Vladimir,
> 
> Waiting for a differential revision.
> 
> --HPS

See https://github.com/freebsd/drm-kmod/pull/138


-- 
WBR
Vladimir Kondratyev