archive mirror
 help / color / mirror / Atom feed
From: Lukas Wunner <>
To: Stuart Hayes <>
Cc: Bjorn Helgaas <>,
	Austin Bolen <>,
	Keith Busch <>,
	Alexandru Gagniuc <>,
	"Rafael J . Wysocki" <>,
	Mika Westerberg <>,
	Andy Shevchenko <>,
	"Gustavo A . R . Silva" <>,
	Sinan Kaya <>,
	Oza Pawandeep <>,,,, Enzo Matsumiya <>
Subject: Re: [PATCH v3] PCI: pciehp: Make sure pciehp_isr clears interrupt events
Date: Wed, 19 Feb 2020 16:57:24 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Sun, Feb 09, 2020 at 09:25:12PM +0100, Lukas Wunner wrote:
> Below is another attempt.  I'll have to take a look at this with a
> fresh pair of eyeballs though to verify I haven't overlooked anything
> else and also to determine if this is actually simpler than Stuart's
> approach.  Again, the advantage here is that processing of the events
> by the IRQ thread is sped up by not delaying it until the Slot Status
> register has settled.

After some deliberation I've come full circle and think that Stuart's
approach is actually better than mine:

I thought that my approach would speed up processing of events by
waking the IRQ thread immediately after the first loop iteration.
But I've realized that right at the beginning of the IRQ thread,
synchronize_hardirq() is called, so the IRQ thread will wait for
the hardirq handler to finish before actually processing the events.

The rationale for the call to synchronize_hardirq() is that the
IRQ thread was woken, but now sees that the hardirq handler is
running (again) to collect more events.  In that situation it makes
sense to wait for them to be collected before starting to process

Is the synchronize_hardirq() absolutely necessary?  Not really,
but I still think that it makes sense.  In reality, the latency
for additional loop iterations is likely small, so it's probably
not worth to optimize for immediate processing after the first
loop iteration.

Stuart's approach is also less intrusive and doesn't change the
logic as much as my approach does.  His patch therefore lends
itself better for backporting to stable.

So I've just respun Stuart's v3 patch, taking into account the
review comments I had sent for it.  I've taken the liberty to make
some editorial changes to the commit message and code comment.
Stuart & Bjorn, if you don't like these, please feel free to roll
back my changes to them as you see fit.

I realize now that I forgot to add the following tags,
Bjorn, could you add them if/when applying?

Fixes: 7b4ce26bcf69 ("PCI: pciehp: Convert to threaded IRQ")
Cc: # v4.19+



  parent reply	other threads:[~2020-02-19 15:57 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-07 19:54 [PATCH v3] PCI: pciehp: Make sure pciehp_isr clears interrupt events Stuart Hayes
2020-02-09 15:03 ` Lukas Wunner
2020-02-09 18:07   ` Lukas Wunner
2020-02-09 20:25     ` Lukas Wunner
2020-02-10 21:40       ` Stuart Hayes
2020-02-13 20:19         ` Stuart Hayes
2020-02-19 15:57       ` Lukas Wunner [this message]
2020-02-19 14:31 ` [PATCH v4] PCI: pciehp: Fix MSI interrupt race Lukas Wunner
2020-03-06 19:59   ` Stuart Hayes
2020-03-20 15:16   ` Joerg Roedel
2020-03-28 21:27   ` Bjorn Helgaas

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:

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

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).