From: Heiko Carstens <hca@linux.ibm.com>
To: Niklas Schnelle <schnelle@linux.ibm.com>
Cc: Alexander Gordeev <agordeev@linux.ibm.com>,
Sven Schnelle <svens@linux.ibm.com>,
Vasily Gorbik <gor@linux.ibm.com>,
Christian Borntraeger <borntraeger@linux.ibm.com>,
Gerd Bayer <gbayer@linux.ibm.com>,
linux-s390@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 2/3] s390/pci: Rework__zpci_event_availability() to remove conditional locking
Date: Wed, 5 Aug 2026 14:43:31 +0200 [thread overview]
Message-ID: <20260805124331.76476C3a-hca@linux.ibm.com> (raw)
In-Reply-To: <0b65164f7442fec896c76e0a39a8d9a43f98b721.camel@linux.ibm.com>
On Wed, Aug 05, 2026 at 01:32:10PM +0200, Niklas Schnelle wrote:
> On Mon, 2026-08-03 at 16:29 +0200, Heiko Carstens wrote:
> > Clang's compiler based static context analysis does not work with locks
> > that are conditionally taken like in __zpci_event_availability():
> >
> > arch/s390/pci/pci_event.c:402:10: warning: mutex 'get_zdev_by_fid(ccdf->fid).state_lock'
> > is not held on every path through here [-Wthread-safety-analysis]
> >
> > Given that code which takes locks conditionally can be considered
> > suboptimal rework __zpci_event_availability() to get rid of this.
> >
> > Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
> > ---
> > arch/s390/pci/pci_event.c | 108 +++++++++++++++++++-------------------
> > 1 file changed, 53 insertions(+), 55 deletions(-)
...
> Personally I think I'd put the 0x0306 and each of the two switches in
> helper functions to improve readability as this is getting awfully
> long. Maybe something like zpci_event_avail_any_device() (0x0306),
> zpci_event_avail_new_device() and zpci_event_avail_existing_device().
>
> If you prefer and since I'm doing a follow up for the missing locking
> in zpci_reserved_devices() I can also do that in a separate patch. Also
> just to clarify Sashiko is right in that the missing locking is a pre-
> existing issue as the state_lock was already not taken in that case,
> it's just more obvious now.
>
> Either way, functionality looks good to me so feel free to add:
>
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
Thanks, I'll address the two nits for patch 1 and 2, and will send a
new version. However I prefer if you do additional code refactoring
with addon patches, so the result looks exactly like you want it.
Otherwise we will go back and forth :)
next prev parent reply other threads:[~2026-08-05 12:43 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-03 14:29 [PATCH v2 0/3] s390/pci: Enable CONTEXT_ANALYSIS Heiko Carstens
2026-08-03 14:29 ` [PATCH v2 1/3] s390/pci: Rework __zpci_event_error() to remove conditional locking Heiko Carstens
2026-08-03 14:36 ` sashiko-bot
2026-08-03 16:40 ` Niklas Schnelle
2026-08-03 14:29 ` [PATCH v2 2/3] s390/pci: Rework__zpci_event_availability() " Heiko Carstens
2026-08-03 14:43 ` sashiko-bot
2026-08-05 11:32 ` Niklas Schnelle
2026-08-05 12:43 ` Heiko Carstens [this message]
2026-08-03 14:29 ` [PATCH v2 3/3] s390/pci: Enable CONTEXT_ANALYSIS Heiko Carstens
2026-08-03 14:37 ` sashiko-bot
2026-08-05 13:22 ` Niklas Schnelle
2026-08-03 14:55 ` [PATCH v2 0/3] " Niklas Schnelle
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=20260805124331.76476C3a-hca@linux.ibm.com \
--to=hca@linux.ibm.com \
--cc=agordeev@linux.ibm.com \
--cc=borntraeger@linux.ibm.com \
--cc=gbayer@linux.ibm.com \
--cc=gor@linux.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=schnelle@linux.ibm.com \
--cc=svens@linux.ibm.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.