On 6/19/24 00:35, Niklas Cassel wrote:
> While the assignment of ap->print_id could have been moved to
> ata_host_alloc(), let's simply move it to ata_port_alloc().
>
> If you allocate a port, you want to give it a unique name that can be used
> for printing.
>
> By moving the ap->print_id assignment to ata_port_alloc(), means that we
> can also remove the ap->print_id assignment from ata_sas_port_alloc().
>
> This will allow a LLD to use the ata_port_*() print functions before
> ata_host_register() has been called.
By the way, shouldn't we replace the ata_print_id atomic with an IDA ?
static DEFINE_IDA(ata_ida);
And then use:
ap->print_id = ida_alloc(&ata_ida, GFP_KERNEL);
ida_free(&ata_ida, ap->print_id);
That would avoid the print IDs to constantly increase when doing e.g.
rmmod ahci+modprobe ahci.
>
> Signed-off-by: Niklas Cassel <cassel@xxxxxxxxxx>
> ---
> drivers/ata/libata-core.c | 6 ++----
> drivers/ata/libata-sata.c | 1 -
> 2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/ata/libata-core.c b/drivers/ata/libata-core.c
> index 0dc6e18bf492..fb4835c2ba2d 100644
> --- a/drivers/ata/libata-core.c
> +++ b/drivers/ata/libata-core.c
> @@ -5463,7 +5463,7 @@ struct ata_port *ata_port_alloc(struct ata_host *host)
>
> ap->pflags |= ATA_PFLAG_INITIALIZING | ATA_PFLAG_FROZEN;
> ap->lock = &host->lock;
> - ap->print_id = -1;
> + ap->print_id = atomic_inc_return(&ata_print_id);
> ap->local_port_no = -1;
> ap->host = host;
> ap->dev = host->dev;
> @@ -5903,10 +5903,8 @@ int ata_host_register(struct ata_host *host, const struct scsi_host_template *sh
> WARN_ON(host->ports[i]);
>
> /* give ports names and add SCSI hosts */
> - for (i = 0; i < host->n_ports; i++) {
> - host->ports[i]->print_id = atomic_inc_return(&ata_print_id);
> + for (i = 0; i < host->n_ports; i++)
> host->ports[i]->local_port_no = i + 1;
> - }
>
> /* Create associated sysfs transport objects */
> for (i = 0; i < host->n_ports; i++) {
> diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
> index c564eac9d430..def0026188f7 100644
> --- a/drivers/ata/libata-sata.c
> +++ b/drivers/ata/libata-sata.c
> @@ -1234,7 +1234,6 @@ struct ata_port *ata_sas_port_alloc(struct ata_host *host,
> ap->flags |= port_info->flags;
> ap->ops = port_info->port_ops;
> ap->cbl = ATA_CBL_SATA;
> - ap->print_id = atomic_inc_return(&ata_print_id);
>
> return ap;
> }
--
Damien Le Moal
Western Digital Research
