All of lore.kernel.org
 help / color / mirror / Atom feed
From: Anisa Su <anisa.su887@gmail.com>
To: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
Cc: Anisa Su <anisa.su887@gmail.com>,
	Davidlohr Bueso <dave@stgolabs.net>,
	Jonathan Cameron <jic23@kernel.org>,
	Dave Jiang <dave.jiang@intel.com>,
	Alison Schofield <alison.schofield@intel.com>,
	Vishal Verma <vishal.l.verma@intel.com>,
	Dan Williams <djbw@kernel.org>, Ira Weiny <iweiny@kernel.org>,
	Li Ming <ming.li@zohomail.com>,
	linux-cxl@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] cxl/events: Return IRQ_NONE when no event is pending
Date: Thu, 10 Sep 2026 03:43:57 +0900	[thread overview]
Message-ID: <aqGo7Rtb00mgS1lH@cxlqual> (raw)
In-Reply-To: <aqGPliCzr8lS64K7@acer-nitro-anv15-41>

On Wed, Sep 09, 2026 at 10:25:50PM +0530, Shaikh Kamaluddin wrote:
> On Tue, Sep 08, 2026 at 11:22:09AM -0700, Anisa Su wrote:
> > On Sun, Sep 06, 2026 at 09:27:05PM +0530, Shaikh Kamaluddin wrote:
> > > CXL event interrupts may share an MSI/MSI-X vector with other event
> > > logs or device features. Consequently, cxl_event_thread() is registered
> > > with IRQF_SHARED and must determine whether an interrupt belongs to the
> > > event-log facility.
> > > 
> > > The handler masks the Device Event Status register to the event logs
> > > supported by the driver. However, when no supported status bit is set,
> > > it exits the processing loop and still returns IRQ_HANDLED.
> > > 
> > > Track whether at least one supported event status bit was observed.
> > > Return IRQ_NONE when there was no event to service, while continuing to
> > > return IRQ_HANDLED after processing one or more event logs.
> > > 
> > > Fixes: a49aa8141b65 ("cxl/mem: Wire up event interrupts")
> > > 
> > > Signed-off-by: Shaikh Kamaluddin <shaikhkamal2012@gmail.com>
> > Hello,
> > 
> > This looks similar to a patch I sent last week:
> > [PATCH v2 4/4] cxl/events: Return IRQ_NONE when no events were processed
> > https://lore.kernel.org/linux-cxl/8e22c1c2-8098-492a-8162-3dd27507c853@amd.com/T/#ma4cf2b2dddaeebc06cb73f81647817bdbb51963c
> > 
> > It was dropped because I received the feedback that in general, the driver does
> > not need to handle device-side errors/spec-violating behaviors. I believe it
> > would apply here as well. But if I misunderstood the intent of this patch,
> > let me know.
> >
> 
> Hi Anisa,
> 
> Thanks for pointing this out. I came across your v2 patch 4 only after I
> had sent my patch.
> 
> Your patch 4 is based on the preceding robustness changes in the series.
> By that point, cxl_event_thread() has been changed from the original
> do-while loop to a while (mask) loop, and
> cxl_mem_get_event_records() reports both an error return and the set of
> drained logs. The handler also uses the drained and stuck masks to
> control further retries.
> 
> My patch leaves the existing event-record retrieval and do-while loop
> unchanged. It only tracks whether a supported event-status bit was
> observed and uses that information to select IRQ_HANDLED or IRQ_NONE.
> It therefore does not change the handling of mailbox errors, undrained
> logs, or retry behavior.
> 
> The handled tracking in both cases addresses the same zero-status
> shared-vector case. My motivation was its effect on generic IRQ
> spurious-interrupt detection. Jonathan agreed that this is a cleanup
> rather than an interrupt-loss fix, so the Fixes tag should be dropped.
> 
> Since your patch was posted first, would you like to repost the IRQ
> return change as a standalone cleanup? If so, I am happy to step back.
> If you no longer plan to pursue it, I can continue with a v2
> incorporating the review feedback.
> 
> Please let me know which you prefer.
> 
Hello Shaikh,

Thank you for your clarification. Please feel free to continue with v2.
Can you add me to the CC list for v2? I am working on some prepatory
patches for DCD an there would be a minor conflict with this patch,
so I will need to rebase on your changes.

Also Ben Cheatham gave the feedback to mention that "MSI will eventually
be disabled" as a consequence of this patch in the commit message on my patch,
which I think may be valuable here as well:
https://lore.kernel.org/linux-cxl/20260901002912.958-1-anisa.su@samsung.com/T/#m866a4fdb159553039d8d47b6ee7f2312caa9ed73

Thanks,
Anisa

> Thanks,
> Shaikh
> 
> 
> 
> 
> > Thanks,
> > Anisa
> > > ---
> > >  drivers/cxl/pci.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > > index c7c91e8dc51d..8b560cae91f2 100644
> > > --- a/drivers/cxl/pci.c
> > > +++ b/drivers/cxl/pci.c
> > > @@ -515,6 +515,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
> > >  	struct cxl_dev_id *dev_id = id;
> > >  	struct cxl_dev_state *cxlds = dev_id->cxlds;
> > >  	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
> > > +	bool handled = false;
> > >  	u32 status;
> > >  
> > >  	do {
> > > @@ -527,11 +528,13 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
> > >  		status &= CXLDEV_EVENT_STATUS_ALL;
> > >  		if (!status)
> > >  			break;
> > > +
> > > +		handled = true;
> > >  		cxl_mem_get_event_records(mds, status);
> > >  		cond_resched();
> > >  	} while (status);
> > >  
> > > -	return IRQ_HANDLED;
> > > +	return handled ? IRQ_HANDLED : IRQ_NONE;
> > >  }
> > >  
> > >  static int cxl_event_req_irq(struct cxl_dev_state *cxlds, u8 setting)
> > > 
> > > base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
> > > prerequisite-patch-id: 92e40cd60a697020faac475dcc77ba63b33434ea
> > > prerequisite-patch-id: 10027ad5d9aed85806047f807b3a76273b6c4a77
> > > -- 
> > > 2.43.0
> > > 

  reply	other threads:[~2026-09-09 18:42 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-06 15:57 [PATCH] cxl/events: Return IRQ_NONE when no event is pending Shaikh Kamaluddin
2026-09-06 16:09 ` sashiko-bot
2026-09-07 18:44   ` Jonathan Cameron
2026-09-07 18:48 ` Jonathan Cameron
2026-09-09  2:08   ` Li Ming
2026-09-09 19:11     ` Jonathan Cameron
2026-09-09 16:13   ` Shaikh Kamaluddin
2026-09-08 18:22 ` Anisa Su
2026-09-09 16:55   ` Shaikh Kamaluddin
2026-09-09 18:43     ` Anisa Su [this message]
2026-09-10 16:22       ` Shaikh Kamaluddin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aqGo7Rtb00mgS1lH@cxlqual \
    --to=anisa.su887@gmail.com \
    --cc=alison.schofield@intel.com \
    --cc=dave.jiang@intel.com \
    --cc=dave@stgolabs.net \
    --cc=djbw@kernel.org \
    --cc=iweiny@kernel.org \
    --cc=jic23@kernel.org \
    --cc=linux-cxl@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=ming.li@zohomail.com \
    --cc=shaikhkamal2012@gmail.com \
    --cc=vishal.l.verma@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.