NAK: [SRU][F][G][PATCH v2 1/1] s390/pci: Fix zpci_alloc_domain() over allocation
Frank Heimes
frank.heimes at canonical.com
Wed May 6 06:32:26 UTC 2020
Hi Kleber,
well, I thought about that, and I can add of course the entire provenance
block of the base commit to backport, but:
- I strictly added the name(s) to the provevance block to this SRU, that
are mentioned in the backport (actually git tooling does that
automatically),
since I think that the people who signed the base commit only signed the
base commit, and not the backport (they are probably even not aware of the
backport).
(I always did it this way in the past.)
- The backport is coming from IBM, means I have to (kind of) modify the
backport with add. provenance info - which didn't seem to be right to me.
- Hence I thought that adding the provenance block from the base commit
(like you asked for):
Signed-off-by: Niklas Schnelle <schnelle at linux.ibm.com>
Reviewed-by: Pierre Morel <pmorel at linux.ibm.com>
Signed-off-by: Vasily Gorbik <gor at linux.ibm.com>
to the SRU for the backport may lead to the assumption that these people
also signed-off / reviewed the backport itself, which is obviously not the
case.
Hence I (personally) think this would be missleading information.
- And I want to strictly follow to the StablePatchFormat documentation:
https://wiki.ubuntu.com/Kernel/Dev/StablePatchFormat
In the 'Complete Example" at the end a backport of (the real world)
commit "c9fcc5d269949a0fbd46ffbea6cc83741e61c05f" is given.
If one looks-up that commit, the (upstream / base) provenance is listed
like this:
Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
Tested-by: "Paulo J. S. Silva" <pjssilva at gmail.com>
Signed-off-by: Eric Anholt <eric at anholt.net>
*Signed-off-by: Greg Kroah-Hartman <gregkh at suse.de <gregkh at suse.de>>*
but the Wiki example just lists:
*OriginalAuthor: Jesse Barnes <jbarnes at virtuousgeek.org
<jbarnes at virtuousgeek.org>> *
Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
Tested-by: "Paulo J. S. Silva" <pjssilva at gmail.com>
Signed-off-by: Eric Anholt <eric at anholt.net>
(backported from commit c9fcc5d269949a0fbd46ffbea6cc83741e61c05f)
[ bjf: context adjustments. ]
Acked-by: Brad Figg <brad.figg at canonical.com>
Acked-by: Steve Conklin <sconklin at canonical.com>
Signed-off-by: Steve Conklin <sconklin at canonical.com>
Hence even here the entire provenance is *not* fully listed, since
'Signed-off-by: Greg Kroah-Hartman <gregkh at suse.de>' is missing.
- For me the fact that the line "(backported from commit
c9fcc5d269949a0fbd46ffbea6cc83741e61c05f)" is added,
allows everybody to look up the original commit and the provenance of
that.
- And btw. this example is also the reason why I added a line about
the OriginalAuthor.
OriginalAuthor: Jesse Barnes <jbarnes at virtuousgeek.org>
Maybe I misinterpreted the wiki or it became a bit outdated in between ...
Anyway, I'm going to submit a v3 with the entire provenance, which means
also manually copying over the provenance from the base commit to the
backport, and will drop the OriginalAuthor line.
Bye, Frank
On Tue, May 5, 2020 at 12:34 PM Kleber Souza <kleber.souza at canonical.com>
wrote:
> On 05.05.20 12:22, frank.heimes at canonical.com wrote:
> > From: Niklas Schnelle <schnelle at linux.ibm.com>
> >
> > BugLink: https://bugs.launchpad.net/bugs/1874057
> >
> > Until now zpci_alloc_domain() only prevented more than
> > CONFIG_PCI_NR_FUNCTIONS from being added when using automatic domain
> > allocation. When explicit UIDs were defined UIDs above
> > CONFIG_PCI_NR_FUNCTIONS were not counted at all.
> > When more PCI functions are added this could lead to various errors
> > including under sized IRQ vectors and similar issues.
> >
> > Fix this by explicitly tracking the number of allocated domains.
> >
> > Signed-off-by: Niklas Schnelle <schnelle at linux.ibm.com>
> > OriginalAuthor: Niklas Schnelle <schnelle at linux.ibm.com>
> > (backported from commit 969ae01bab2fe938b4c8324836038b5ac1c78fac)
> > Signed-off-by: Frank Heimes <frank.heimes at canonical.com>
>
> Hi Frank,
>
> When we backport/cherry-pick patches from upstream or any other external
> tree we need to keep the whole provenance block from the original commit
> and add our tags below it. Looking at commit 969ae01bab2f, the original
> provenance block is:
>
> Signed-off-by: Niklas Schnelle <schnelle at linux.ibm.com>
> Reviewed-by: Pierre Morel <pmorel at linux.ibm.com>
> Signed-off-by: Vasily Gorbik <gor at linux.ibm.com>
>
> All of this needs to be kept, adding the '(backported from ...)' and
> your s-o-b lines below them. We don't need to add the 'OriginalAuthor:'
> tag as git will automatically create the commit with the author set
> to the name/email from the "From:" tag added as first line of the
> patch.
>
>
> Thanks,
> Kleber
>
>
> > ---
> > arch/s390/include/asm/pci.h | 1 +
> > arch/s390/pci/pci.c | 34 ++++++++++++++++++++--------------
> > 2 files changed, 21 insertions(+), 14 deletions(-)
> >
> > diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
> > index 6087a4e9b2bf..7850e8c8c79a 100644
> > --- a/arch/s390/include/asm/pci.h
> > +++ b/arch/s390/include/asm/pci.h
> > @@ -28,6 +28,7 @@ int pci_proc_domain(struct pci_bus *);
> >
> > #define ZPCI_NR_DMA_SPACES 1
> > #define ZPCI_NR_DEVICES CONFIG_PCI_NR_FUNCTIONS
> > +#define ZPCI_DOMAIN_BITMAP_SIZE (1 << 16)
> >
> > /* PCI Function Controls */
> > #define ZPCI_FC_FN_ENABLED 0x80
> > diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
> > index 6105b1b6e49b..0af46683dd66 100644
> > --- a/arch/s390/pci/pci.c
> > +++ b/arch/s390/pci/pci.c
> > @@ -39,8 +39,9 @@
> > static LIST_HEAD(zpci_list);
> > static DEFINE_SPINLOCK(zpci_list_lock);
> >
> > -static DECLARE_BITMAP(zpci_domain, ZPCI_NR_DEVICES);
> > +static DECLARE_BITMAP(zpci_domain, ZPCI_DOMAIN_BITMAP_SIZE);
> > static DEFINE_SPINLOCK(zpci_domain_lock);
> > +static unsigned int zpci_num_domains_allocated;
> >
> > #define ZPCI_IOMAP_ENTRIES \
> > min(((unsigned long) ZPCI_NR_DEVICES * PCI_BAR_COUNT / 2), \
> > @@ -651,39 +652,44 @@ struct dev_pm_ops pcibios_pm_ops = {
> >
> > static int zpci_alloc_domain(struct zpci_dev *zdev)
> > {
> > + spin_lock(&zpci_domain_lock);
> > + if (zpci_num_domains_allocated > (ZPCI_NR_DEVICES - 1)) {
> > + spin_unlock(&zpci_domain_lock);
> > + pr_err("Adding PCI function %08x failed because the
> configured limit of %d is reached\n",
> > + zdev->fid, ZPCI_NR_DEVICES);
> > + return -ENOSPC;
> > + }
> > +
> > if (zpci_unique_uid) {
> > zdev->domain = (u16) zdev->uid;
> > - if (zdev->domain >= ZPCI_NR_DEVICES)
> > - return 0;
> > -
> > - spin_lock(&zpci_domain_lock);
> > if (test_bit(zdev->domain, zpci_domain)) {
> > spin_unlock(&zpci_domain_lock);
> > + pr_err("Adding PCI function %08x failed because
> domain %04x is already assigned\n",
> > + zdev->fid, zdev->domain);
> > return -EEXIST;
> > }
> > set_bit(zdev->domain, zpci_domain);
> > + zpci_num_domains_allocated++;
> > spin_unlock(&zpci_domain_lock);
> > return 0;
> > }
> > -
> > - spin_lock(&zpci_domain_lock);
> > + /*
> > + * We can always auto allocate domains below ZPCI_NR_DEVICES.
> > + * There is either a free domain or we have reached the maximum in
> > + * which case we would have bailed earlier.
> > + */
> > zdev->domain = find_first_zero_bit(zpci_domain, ZPCI_NR_DEVICES);
> > - if (zdev->domain == ZPCI_NR_DEVICES) {
> > - spin_unlock(&zpci_domain_lock);
> > - return -ENOSPC;
> > - }
> > set_bit(zdev->domain, zpci_domain);
> > + zpci_num_domains_allocated++;
> > spin_unlock(&zpci_domain_lock);
> > return 0;
> > }
> >
> > static void zpci_free_domain(struct zpci_dev *zdev)
> > {
> > - if (zdev->domain >= ZPCI_NR_DEVICES)
> > - return;
> > -
> > spin_lock(&zpci_domain_lock);
> > clear_bit(zdev->domain, zpci_domain);
> > + zpci_num_domains_allocated--;
> > spin_unlock(&zpci_domain_lock);
> > }
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20200506/3860aa97/attachment-0001.html>
More information about the kernel-team
mailing list