From b21a18f730e4b659be951a041eff682a60c0f4a0 Mon Sep 17 00:00:00 2001 From: Julia Lawall Date: Mon, 30 Sep 2024 13:21:08 +0200 Subject: [PATCH 1/5] PCI: hotplug: Reorganize kerneldoc parameter names Reorganize kerneldoc parameter names to match the parameter order in the function header. Problems identified using Coccinelle. Link: https://lore.kernel.org/r/20240930112121.95324-23-Julia.Lawall@inria.fr Signed-off-by: Julia Lawall Signed-off-by: Bjorn Helgaas --- drivers/pci/hotplug/pci_hotplug_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c index 058d5937d8a95..db09d4992e6e2 100644 --- a/drivers/pci/hotplug/pci_hotplug_core.c +++ b/drivers/pci/hotplug/pci_hotplug_core.c @@ -388,8 +388,8 @@ static struct hotplug_slot *get_slot_from_name(const char *name) /** * __pci_hp_register - register a hotplug_slot with the PCI hotplug subsystem - * @bus: bus this slot is on * @slot: pointer to the &struct hotplug_slot to register + * @bus: bus this slot is on * @devnr: device number * @name: name registered with kobject core * @owner: caller module owner From 43ee11adcb940204948ac0ca3a05d6178f0e8b08 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Mon, 30 Sep 2024 15:44:36 +0300 Subject: [PATCH 2/5] PCI: hotplug: Remove "Returns" kerneldoc from void functions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pci_hp_deregister() was converted to void by the commit 51bbf9bee34f ("PCI: hotplug: Demidlayer registration with the core") but its kerneldoc still describes the return value. pci_hp_del() and pci_hp_destroy() have been void since they were introduced in that same commit. Remove the return value description from the kerneldoc of those functions. Link: https://lore.kernel.org/r/20240930124436.17908-1-ilpo.jarvinen@linux.intel.com Signed-off-by: Ilpo Järvinen Signed-off-by: Bjorn Helgaas --- drivers/pci/hotplug/pci_hotplug_core.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/drivers/pci/hotplug/pci_hotplug_core.c b/drivers/pci/hotplug/pci_hotplug_core.c index db09d4992e6e2..36236ac88fd56 100644 --- a/drivers/pci/hotplug/pci_hotplug_core.c +++ b/drivers/pci/hotplug/pci_hotplug_core.c @@ -498,8 +498,6 @@ EXPORT_SYMBOL_GPL(pci_hp_add); * * The @slot must have been registered with the pci hotplug subsystem * previously with a call to pci_hp_register(). - * - * Returns 0 if successful, anything else for an error. */ void pci_hp_deregister(struct hotplug_slot *slot) { @@ -513,8 +511,6 @@ EXPORT_SYMBOL_GPL(pci_hp_deregister); * @slot: pointer to the &struct hotplug_slot to unpublish * * Remove a hotplug slot's sysfs interface. - * - * Returns 0 on success or a negative int on error. */ void pci_hp_del(struct hotplug_slot *slot) { @@ -545,8 +541,6 @@ EXPORT_SYMBOL_GPL(pci_hp_del); * the driver may no longer invoke hotplug_slot_name() to get the slot's * unique name. The driver no longer needs to handle a ->reset_slot callback * from this point on. - * - * Returns 0 on success or a negative int on error. */ void pci_hp_destroy(struct hotplug_slot *slot) { From 2985b1844f3f3447f2d938eff1ef6762592065a5 Mon Sep 17 00:00:00 2001 From: Todd Kjos Date: Tue, 1 Oct 2024 23:11:47 +0000 Subject: [PATCH 3/5] PCI: Fix reset_method_store() memory leak In reset_method_store(), a string is allocated via kstrndup() and assigned to the local "options". options is then used in with strsep() to find spaces: while ((name = strsep(&options, " ")) != NULL) { If there are no remaining spaces, then options is set to NULL by strsep(), so the subsequent kfree(options) doesn't free the memory allocated via kstrndup(). Fix by using a separate tmp_options to iterate with strsep() so options is preserved. Link: https://lore.kernel.org/r/20241001231147.3583649-1-tkjos@google.com Fixes: d88f521da3ef ("PCI: Allow userspace to query and set device reset mechanism") Signed-off-by: Todd Kjos Signed-off-by: Bjorn Helgaas --- drivers/pci/pci.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index 7d85c04fbba2a..0e6562ff3dcf8 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -5244,7 +5244,7 @@ static ssize_t reset_method_store(struct device *dev, const char *buf, size_t count) { struct pci_dev *pdev = to_pci_dev(dev); - char *options, *name; + char *options, *tmp_options, *name; int m, n; u8 reset_methods[PCI_NUM_RESET_METHODS] = { 0 }; @@ -5264,7 +5264,8 @@ static ssize_t reset_method_store(struct device *dev, return -ENOMEM; n = 0; - while ((name = strsep(&options, " ")) != NULL) { + tmp_options = options; + while ((name = strsep(&tmp_options, " ")) != NULL) { if (sysfs_streq(name, "")) continue; From 3ca258994b32c486b0ed59307ec31ee413524598 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= Date: Fri, 4 Oct 2024 18:22:40 +0300 Subject: [PATCH 4/5] PCI: Simplify pci_create_slot() logic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Simplify pci_create_slot() by splitting an "if" statement into two parts. In order to not duplicate error handling, add a new label to handle kobj put. Link: https://lore.kernel.org/r/20241004152240.7926-1-ilpo.jarvinen@linux.intel.com Signed-off-by: Ilpo Järvinen Signed-off-by: Bjorn Helgaas --- drivers/pci/slot.c | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/drivers/pci/slot.c b/drivers/pci/slot.c index 0f87cade10f74..9ac5a4f267949 100644 --- a/drivers/pci/slot.c +++ b/drivers/pci/slot.c @@ -244,12 +244,13 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr, slot = get_slot(parent, slot_nr); if (slot) { if (hotplug) { - if ((err = slot->hotplug ? -EBUSY : 0) - || (err = rename_slot(slot, name))) { - kobject_put(&slot->kobj); - slot = NULL; - goto err; + if (slot->hotplug) { + err = -EBUSY; + goto put_slot; } + err = rename_slot(slot, name); + if (err) + goto put_slot; } goto out; } @@ -278,10 +279,8 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr, err = kobject_init_and_add(&slot->kobj, &pci_slot_ktype, NULL, "%s", slot_name); - if (err) { - kobject_put(&slot->kobj); - goto err; - } + if (err) + goto put_slot; down_read(&pci_bus_sem); list_for_each_entry(dev, &parent->devices, bus_list) @@ -296,6 +295,9 @@ struct pci_slot *pci_create_slot(struct pci_bus *parent, int slot_nr, kfree(slot_name); mutex_unlock(&pci_slot_mutex); return slot; + +put_slot: + kobject_put(&slot->kobj); err: slot = ERR_PTR(err); goto out; From d9db393f2b9ed674f965dd823c1692b41bad7bfb Mon Sep 17 00:00:00 2001 From: Luo Yifan Date: Tue, 12 Nov 2024 17:09:24 +0800 Subject: [PATCH 5/5] tools: PCI: Fix incorrect printf format specifiers Fix several incorrect printf format specifiers that misused signed and unsigned versions. Link: https://lore.kernel.org/r/20241112090924.287056-1-luoyifan@cmss.chinamobile.com Signed-off-by: Luo Yifan Signed-off-by: Bjorn Helgaas Reviewed-by: Manivannan Sadhasivam --- tools/pci/pcitest.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tools/pci/pcitest.c b/tools/pci/pcitest.c index 470258009ddc2..7b530d838d408 100644 --- a/tools/pci/pcitest.c +++ b/tools/pci/pcitest.c @@ -95,7 +95,7 @@ static int run_test(struct pci_test *test) if (test->msinum > 0 && test->msinum <= 32) { ret = ioctl(fd, PCITEST_MSI, test->msinum); - fprintf(stdout, "MSI%d:\t\t", test->msinum); + fprintf(stdout, "MSI%u:\t\t", test->msinum); if (ret < 0) fprintf(stdout, "TEST FAILED\n"); else @@ -104,7 +104,7 @@ static int run_test(struct pci_test *test) if (test->msixnum > 0 && test->msixnum <= 2048) { ret = ioctl(fd, PCITEST_MSIX, test->msixnum); - fprintf(stdout, "MSI-X%d:\t\t", test->msixnum); + fprintf(stdout, "MSI-X%u:\t\t", test->msixnum); if (ret < 0) fprintf(stdout, "TEST FAILED\n"); else @@ -116,7 +116,7 @@ static int run_test(struct pci_test *test) if (test->use_dma) param.flags = PCITEST_FLAGS_USE_DMA; ret = ioctl(fd, PCITEST_WRITE, ¶m); - fprintf(stdout, "WRITE (%7ld bytes):\t\t", test->size); + fprintf(stdout, "WRITE (%7lu bytes):\t\t", test->size); if (ret < 0) fprintf(stdout, "TEST FAILED\n"); else @@ -128,7 +128,7 @@ static int run_test(struct pci_test *test) if (test->use_dma) param.flags = PCITEST_FLAGS_USE_DMA; ret = ioctl(fd, PCITEST_READ, ¶m); - fprintf(stdout, "READ (%7ld bytes):\t\t", test->size); + fprintf(stdout, "READ (%7lu bytes):\t\t", test->size); if (ret < 0) fprintf(stdout, "TEST FAILED\n"); else @@ -140,7 +140,7 @@ static int run_test(struct pci_test *test) if (test->use_dma) param.flags = PCITEST_FLAGS_USE_DMA; ret = ioctl(fd, PCITEST_COPY, ¶m); - fprintf(stdout, "COPY (%7ld bytes):\t\t", test->size); + fprintf(stdout, "COPY (%7lu bytes):\t\t", test->size); if (ret < 0) fprintf(stdout, "TEST FAILED\n"); else