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
