| Message ID | 20240626180031.4050226-18-cassel@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | ata,libsas: Assign the unique id used for printing earlier | expand |
On Wed, Jun 26, 2024 at 08:00:33PM +0200, Niklas Cassel wrote: > .probe() (ahci_init_one()) calls sysfs_add_file_to_group(), however, > if probe() fails after this call, we currently never call > sysfs_remove_file_from_group(). > > (The sysfs_remove_file_from_group() call in .remove() (ahci_remove_one()) > does not help, as .remove() is not called on .probe() error.) > > Thus, if probe() fails after the sysfs_add_file_to_group() call, we get: Nit: s/we get/the next time we insmod the module we will get/
On 6/27/24 03:00, Niklas Cassel wrote: > .probe() (ahci_init_one()) calls sysfs_add_file_to_group(), however, > if probe() fails after this call, we currently never call > sysfs_remove_file_from_group(). > > (The sysfs_remove_file_from_group() call in .remove() (ahci_remove_one()) > does not help, as .remove() is not called on .probe() error.) > > Thus, if probe() fails after the sysfs_add_file_to_group() call, we get: > > sysfs: cannot create duplicate filename '/devices/pci0000:00/0000:00:04.0/remapped_nvme' > CPU: 11 PID: 954 Comm: modprobe Not tainted 6.10.0-rc5 #43 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-2.fc40 04/01/2014 > Call Trace: > <TASK> > dump_stack_lvl+0x5d/0x80 > sysfs_warn_dup.cold+0x17/0x23 > sysfs_add_file_mode_ns+0x11a/0x130 > sysfs_add_file_to_group+0x7e/0xc0 > ahci_init_one+0x31f/0xd40 [ahci] > > Fixes: 894fba7f434a ("ata: ahci: Add sysfs attribute to show remapped NVMe device count") > Cc: stable@vger.kernel.org > Signed-off-by: Niklas Cassel <cassel@kernel.org> Looks good. And I think same as patch 1 and 2: let's send this out as a 6.10 fix. Reviewed-by: Damien Le Moal <dlemoal@kernel.org>
On 6/26/24 20:00, Niklas Cassel wrote: > .probe() (ahci_init_one()) calls sysfs_add_file_to_group(), however, > if probe() fails after this call, we currently never call > sysfs_remove_file_from_group(). > > (The sysfs_remove_file_from_group() call in .remove() (ahci_remove_one()) > does not help, as .remove() is not called on .probe() error.) > > Thus, if probe() fails after the sysfs_add_file_to_group() call, we get: > > sysfs: cannot create duplicate filename '/devices/pci0000:00/0000:00:04.0/remapped_nvme' > CPU: 11 PID: 954 Comm: modprobe Not tainted 6.10.0-rc5 #43 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-2.fc40 04/01/2014 > Call Trace: > <TASK> > dump_stack_lvl+0x5d/0x80 > sysfs_warn_dup.cold+0x17/0x23 > sysfs_add_file_mode_ns+0x11a/0x130 > sysfs_add_file_to_group+0x7e/0xc0 > ahci_init_one+0x31f/0xd40 [ahci] > > Fixes: 894fba7f434a ("ata: ahci: Add sysfs attribute to show remapped NVMe device count") > Cc: stable@vger.kernel.org > Signed-off-by: Niklas Cassel <cassel@kernel.org> > --- > drivers/ata/ahci.c | 17 ++++++++++++----- > 1 file changed, 12 insertions(+), 5 deletions(-) > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c index 5eb38fbbbecd..fc6fd583faf8 100644 --- a/drivers/ata/ahci.c +++ b/drivers/ata/ahci.c @@ -1975,8 +1975,10 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) n_ports = max(ahci_nr_ports(hpriv->cap), fls(hpriv->port_map)); host = ata_host_alloc_pinfo(&pdev->dev, ppi, n_ports); - if (!host) - return -ENOMEM; + if (!host) { + rc = -ENOMEM; + goto err_rm_sysfs_file; + } host->private_data = hpriv; if (ahci_init_msi(pdev, n_ports, hpriv) < 0) { @@ -2031,11 +2033,11 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) /* initialize adapter */ rc = ahci_configure_dma_masks(pdev, hpriv); if (rc) - return rc; + goto err_rm_sysfs_file; rc = ahci_pci_reset_controller(host); if (rc) - return rc; + goto err_rm_sysfs_file; ahci_pci_init_controller(host); ahci_pci_print_info(host); @@ -2044,10 +2046,15 @@ static int ahci_init_one(struct pci_dev *pdev, const struct pci_device_id *ent) rc = ahci_host_activate(host, &ahci_sht); if (rc) - return rc; + goto err_rm_sysfs_file; pm_runtime_put_noidle(&pdev->dev); return 0; + +err_rm_sysfs_file: + sysfs_remove_file_from_group(&pdev->dev.kobj, + &dev_attr_remapped_nvme.attr, NULL); + return rc; } static void ahci_shutdown_one(struct pci_dev *pdev)
.probe() (ahci_init_one()) calls sysfs_add_file_to_group(), however, if probe() fails after this call, we currently never call sysfs_remove_file_from_group(). (The sysfs_remove_file_from_group() call in .remove() (ahci_remove_one()) does not help, as .remove() is not called on .probe() error.) Thus, if probe() fails after the sysfs_add_file_to_group() call, we get: sysfs: cannot create duplicate filename '/devices/pci0000:00/0000:00:04.0/remapped_nvme' CPU: 11 PID: 954 Comm: modprobe Not tainted 6.10.0-rc5 #43 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-2.fc40 04/01/2014 Call Trace: <TASK> dump_stack_lvl+0x5d/0x80 sysfs_warn_dup.cold+0x17/0x23 sysfs_add_file_mode_ns+0x11a/0x130 sysfs_add_file_to_group+0x7e/0xc0 ahci_init_one+0x31f/0xd40 [ahci] Fixes: 894fba7f434a ("ata: ahci: Add sysfs attribute to show remapped NVMe device count") Cc: stable@vger.kernel.org Signed-off-by: Niklas Cassel <cassel@kernel.org> --- drivers/ata/ahci.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)