Re: git: 02ea6033020e - main - LinuxKPI: Allow spin_lock_irqsave to be called within a critical section
- Reply: Vladimir Kondratyev : "Re: git: 02ea6033020e - main - LinuxKPI: Allow spin_lock_irqsave to be called within a critical section"
- In reply to: Vladimir Kondratyev : "Re: git: 02ea6033020e - main - LinuxKPI: Allow spin_lock_irqsave to be called within a critical section"
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Sat, 22 Jan 2022 09:49:09 UTC
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