From: Bjorn Helgaas <helgaas@kernel.org>
To: Niklas Schnelle <schnelle@linux.ibm.com>
Cc: linux-s390@vger.kernel.org,
Matthew Rosato <mjrosato@linux.ibm.com>,
Pierre Morel <pmorel@linux.ibm.com>,
Jan Kiszka <jan.kiszka@siemens.com>,
linux-kernel@vger.kernel.org,
virtualization@lists.linux-foundation.org,
linux-pci@vger.kernel.org, Bjorn Helgaas <bhelgaas@google.com>
Subject: Re: [PATCH RESEND v5 1/4] PCI: Clean up pci_scan_slot()
Date: Fri, 13 May 2022 09:07:23 -0500 [thread overview]
Message-ID: <20220513140723.GA947754@bhelgaas> (raw)
In-Reply-To: <f34130034637278c9999cf6d430be558cb0c2318.camel@linux.ibm.com>
On Thu, May 12, 2022 at 04:56:42PM +0200, Niklas Schnelle wrote:
> On Thu, 2022-05-05 at 10:38 +0200, Niklas Schnelle wrote:
> > While determining the next PCI function is factored out of
> > pci_scan_slot() into next_fn() the former still handles the first
> > function as a special case. This duplicates the code from the scan loop.
> >
> > Furthermore the non ARI branch of next_fn() is generally hard to
> > understand and especially the check for multifunction devices is hidden
> > in the handling of NULL devices for non-contiguous multifunction. It
> > also signals that no further functions need to be scanned by returning
> > 0 via wraparound and this is a valid function number.
> >
> > Improve upon this by transforming the conditions in next_fn() to be
> > easier to understand.
> >
> > By changing next_fn() to return -ENODEV instead of 0 when there is no
> > next function we can then handle the initial function inside the loop
> > and deduplicate the shared handling. This also makes it more explicit
> > that only function 0 must exist.
> >
> > No functional change is intended.
> >
> > Cc: Jan Kiszka <jan.kiszka@siemens.com>
> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> > ---
>
> Friendly ping :-)
Thanks and sorry for the delay. I'm off today for my daughter's
wedding reception but will get back to it next week. Just to expose
some of my thought process (and not to request more work from you!)
I've been wondering whether b1bd58e448f2 ("PCI: Consolidate
"next-function" functions") is really causing us more trouble than
it's worth. In some ways that makes the single next-function harder
to read. But I guess the hypervisor special case is not exactly a
"next-function" thing -- it's a "keep scanning even if there's no fn
0" thing.
Bjorn
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
parent reply other threads:[~2022-05-13 14:07 UTC|newest]
Thread overview: expand[flat|nested] mbox.gz Atom feed
[parent not found: <f34130034637278c9999cf6d430be558cb0c2318.camel@linux.ibm.com>]
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=20220513140723.GA947754@bhelgaas \
--to=helgaas@kernel.org \
--cc=bhelgaas@google.com \
--cc=jan.kiszka@siemens.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=linux-s390@vger.kernel.org \
--cc=mjrosato@linux.ibm.com \
--cc=pmorel@linux.ibm.com \
--cc=schnelle@linux.ibm.com \
--cc=virtualization@lists.linux-foundation.org \
/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 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).