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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux