[PATCH v5 04/21] gpu: host1x: Remove cancelled waiters immediately

[Mikko Perttunen]

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