[PATCH v5 04/21] gpu: host1x: Remove cancelled waiters immediately
Mikko Perttunen
cyndis at kapsi.fi
Wed Jan 13 18:16:02 UTC 2021
On 1/13/21 6:29 PM, Dmitry Osipenko wrote:
> 13.01.2021 01:20, Mikko Perttunen пишет:
>> On 1/13/21 12:07 AM, Dmitry Osipenko wrote:
>>> 11.01.2021 16:00, Mikko Perttunen пишет:
>>>> -void host1x_intr_put_ref(struct host1x *host, unsigned int id, void
>>>> *ref)
>>>> +void host1x_intr_put_ref(struct host1x *host, unsigned int id, void
>>>> *ref,
>>>> + bool flush)
>>>> {
>>>> struct host1x_waitlist *waiter = ref;
>>>> struct host1x_syncpt *syncpt;
>>>> - while (atomic_cmpxchg(&waiter->state, WLS_PENDING,
>>>> WLS_CANCELLED) ==
>>>> - WLS_REMOVED)
>>>> - schedule();
>>>> + atomic_cmpxchg(&waiter->state, WLS_PENDING, WLS_CANCELLED);
>>>> syncpt = host->syncpt + id;
>>>> - (void)process_wait_list(host, syncpt,
>>>> - host1x_syncpt_load(host->syncpt + id));
>>>> +
>>>> + spin_lock(&syncpt->intr.lock);
>>>> + if (atomic_cmpxchg(&waiter->state, WLS_CANCELLED, WLS_HANDLED) ==
>>>> + WLS_CANCELLED) {
>>>> + list_del(&waiter->list);
>>>> + kref_put(&waiter->refcount, waiter_release);
>>>> + }
>>>> + spin_unlock(&syncpt->intr.lock);
>>>> +
>>>> + if (flush) {
>>>> + /* Wait until any concurrently executing handler has
>>>> finished. */
>>>> + while (atomic_read(&waiter->state) != WLS_HANDLED)
>>>> + cpu_relax();
>>>> + }
>>>
>>> A busy-loop shouldn't be used in kernel unless there is a very good
>>> reason. The wait_event() should be used instead.
>>>
>>> But please don't hurry to update this patch, we may need or want to
>>> retire the host1x-waiter and then these all waiter-related patches won't
>>> be needed.
>>>
>>
>> Yes, we should improve the intr code to remove all this complexity. But
>> let's merge this first to get a functional baseline and do larger design
>> changes in follow-up patches.
>>
>> It is cumbersome for me to develop further series (of which I have
>> several under work and planning) with this baseline series not being
>> merged. The uncertainty on the approval of the UAPI design also makes it
>> hard to know whether it makes sense for me to work on top of this code
>> or not, so I'd like to focus on what's needed to get this merged instead
>> of further redesign of the driver at this time.
>
> Is this patch (and some others) necessary for the new UAPI? If not,
> could we please narrow down the patches to the minimum that is needed
> for trying out the new UAPI?
>
Yes, it is necessary. I tried to revert it and half the tests in the
test suite start failing.
I think patches 01, 03, 14 and 17 are not strictly required, though
reverting 03 will cause one of the syncpoint tests to fail.
Mikko
More information about the dri-devel
mailing list