Re: [PATCH v3 02/10] cxl/pci: Cleanup 'sanitize' to always poll

[Dan Williams <dan.j.williams@intel.com>]

Davidlohr Bueso wrote:
> On Fri, 06 Oct 2023, Dan Williams wrote:
> 
> >In preparation for fixing the init/teardown of the 'sanitize' workqueue
> >and sysfs notification mechanism, arrange for cxl_mbox_sanitize_work()
> >to be the single location where the sysfs attribute is notified. With
> >that change there is no distinction between polled mode and interrupt
> >mode. All the interrupt does is accelerate the polling interval.
> >
> >The change to check for "mds->security.sanitize_node" under the lock is
> >there to ensure that the interrupt, the work routine and the
> >setup/teardown code can all have a consistent view of the registered
> >notifier and the workqueue state. I.e. the expectation is that the
> >interrupt is live past the point that the sanitize sysfs attribute is
> >published, and it may race teardown, so it must be consulted under a
> >lock. Given that new locking requirement, cxl_pci_mbox_irq() is moved
> >from hard to thread irq context.
> >
> >Lastly, some opportunistic replacements of
> >"queue_delayed_work(system_wq, ...)", which is just open coded
> >schedule_delayed_work(), are included.
> 
> So the current order of this patch will cause bisection issues -
> the next patch which moves the irq handler to a threaded context
> should come before this one. Otherwise:


This patch does switch the order:

@@ -432,33 +429,26 @@ static int cxl_pci_setup_mailbox(struct cxl_memdev_state *mds)
        dev_dbg(dev, "Mailbox payload sized %zu", mds->payload_size);
 
        rcuwait_init(&mds->mbox_wait);
[..]
-               if (cxl_request_irq(cxlds, irq, cxl_pci_mbox_irq, NULL))
-                       goto mbox_poll;
[..]
+       if (cxl_request_irq(cxlds, irq, NULL, cxl_pci_mbox_irq))
                return 0;

...the next patch only deletes the option to pass non-null in the third
argument.

@@ -440,7 +440,7 @@ static int cxl_pci_setup_mailbox(struct cxl_memdev_state *mds)
        if (irq < 0)
                return 0;
 
-       if (cxl_request_irq(cxlds, irq, NULL, cxl_pci_mbox_irq))
+       if (cxl_request_irq(cxlds, irq, cxl_pci_mbox_irq))
                return 0;
 
        dev_dbg(cxlds->dev, "Mailbox interrupts enabled\n");
@@ -638,7 +638,7 @@ static int cxl_event_req_irq(struct cxl_dev_state *cxlds, u8 setting)
        if (irq < 0)
                return irq;
 
-       return cxl_request_irq(cxlds, irq, NULL, cxl_event_thread);
+       return cxl_request_irq(cxlds, irq, cxl_event_thread);
 }
 
 static int cxl_event_get_int_policy(struct cxl_memdev_state *mds,


...did I miss anything?