Re: [PATCH v2 3/8] ata,scsi: remove ata_sas_port_destroy()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 21/07/2023 14:33, Niklas Cassel wrote:
On Thu, Jul 20, 2023 at 09:57:29AM +0100, John Garry wrote:
On 20/07/2023 01:42, Niklas Cassel wrote:
From: Hannes Reinecke <hare@xxxxxxx>

Is now a wrapper around kfree(), so call it directly.

Signed-off-by: Hannes Reinecke <hare@xxxxxxx>
Signed-off-by: Niklas Cassel <niklas.cassel@xxxxxxx>
---
   drivers/ata/libata-sata.c          | 18 ------------------
   drivers/scsi/libsas/sas_ata.c      |  2 +-
   drivers/scsi/libsas/sas_discover.c |  2 +-
   include/linux/libata.h             |  1 -
   4 files changed, 2 insertions(+), 21 deletions(-)

diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
index d3b595294eee..b5de0f40ea25 100644
--- a/drivers/ata/libata-sata.c
+++ b/drivers/ata/libata-sata.c
@@ -1177,10 +1177,6 @@ EXPORT_SYMBOL_GPL(ata_sas_sync_probe);
   int ata_sas_port_init(struct ata_port *ap)


Hi Niklas,

This is a bit of a daft function now, considering it only does
atomic_inc_return(&ata_print_id). Do we really need to export a symbol for
that?

$ git grep ata_print_id
drivers/ata/libata-core.c:atomic_t ata_print_id = ATOMIC_INIT(0);
drivers/ata/libata-core.c:              host->ports[i]->print_id = atomic_inc_return(&ata_print_id);
drivers/ata/libata-sata.c:      ap->print_id = atomic_inc_return(&ata_print_id);
drivers/ata/libata.h:extern atomic_t ata_print_id;

It seems to be defined and used only in libata, while I agree that the function
is a bit silly, with my limited knowledge of how the linker works, moving it to
libsas seems a bit dangerous...

You can build libata as a module and libsas as built-in, and vice versa...

Also, since there are no direct users in libsas, I'd rather keep it in libata.

I wasn't really suggesting to move it to libsas - indeed, it is libata functionality.

Could you just put the ap->print_id = atomic_inc_return(&ata_print_id) call in ata_sas_port_alloc() (and remove ata_sas_port_init())? ata_sas_port_alloc() is only called from libsas, and ata_sas_port_init() is called straight after ata_sas_port_alloc() there. And ata_sas_port_alloc() is already doing ap init also (so setting ap->print_id would not be out of place there).

Thanks,
John



[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux