[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f0e47ac3-d736-458d-8ffc-f7648174aa93@suse.de>
Date: Tue, 18 Mar 2025 12:19:18 +0100
From: Hannes Reinecke <hare@...e.de>
To: Daniel Wagner <wagi@...nel.org>, James Smart <james.smart@...adcom.com>,
Christoph Hellwig <hch@....de>, Sagi Grimberg <sagi@...mberg.me>,
Chaitanya Kulkarni <kch@...dia.com>
Cc: Keith Busch <kbusch@...nel.org>, linux-nvme@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 16/18] nvmet-fc: take tgtport refs for portentry
On 3/18/25 11:40, Daniel Wagner wrote:
> Ensure that the tgtport is not going away as long portentry has a
> pointer on it.
>
> Signed-off-by: Daniel Wagner <wagi@...nel.org>
> ---
> drivers/nvme/target/fc.c | 45 +++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 39 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/nvme/target/fc.c b/drivers/nvme/target/fc.c
> index d10ddcb57c1b09d871152f0d9a48f93ec6dc8685..649afce908bbade0a843efc4b8b76105c164b035 100644
> --- a/drivers/nvme/target/fc.c
> +++ b/drivers/nvme/target/fc.c
> @@ -1265,6 +1265,7 @@ nvmet_fc_portentry_bind(struct nvmet_fc_tgtport *tgtport,
> {
> lockdep_assert_held(&nvmet_fc_tgtlock);
>
> + nvmet_fc_tgtport_get(tgtport);
> pe->tgtport = tgtport;
> tgtport->pe = pe;
>
> @@ -1284,8 +1285,10 @@ nvmet_fc_portentry_unbind(struct nvmet_fc_port_entry *pe)
> unsigned long flags;
>
> spin_lock_irqsave(&nvmet_fc_tgtlock, flags);
> - if (pe->tgtport)
> + if (pe->tgtport) {
> + nvmet_fc_tgtport_put(pe->tgtport);
> pe->tgtport->pe = NULL;
> + }
> list_del(&pe->pe_list);
> spin_unlock_irqrestore(&nvmet_fc_tgtlock, flags);
> }
> @@ -1303,8 +1306,10 @@ nvmet_fc_portentry_unbind_tgt(struct nvmet_fc_tgtport *tgtport)
>
> spin_lock_irqsave(&nvmet_fc_tgtlock, flags);
> pe = tgtport->pe;
> - if (pe)
> + if (pe) {
> + nvmet_fc_tgtport_put(pe->tgtport);
> pe->tgtport = NULL;
> + }
> tgtport->pe = NULL;
> spin_unlock_irqrestore(&nvmet_fc_tgtlock, flags);
> }
> @@ -1327,6 +1332,9 @@ nvmet_fc_portentry_rebind_tgt(struct nvmet_fc_tgtport *tgtport)
> list_for_each_entry(pe, &nvmet_fc_portentry_list, pe_list) {
> if (tgtport->fc_target_port.node_name == pe->node_name &&
> tgtport->fc_target_port.port_name == pe->port_name) {
> + if (!nvmet_fc_tgtport_get(tgtport))
> + continue;
> +
> WARN_ON(pe->tgtport);
> tgtport->pe = pe;
> pe->tgtport = tgtport;
> @@ -1664,7 +1672,6 @@ nvmet_fc_unregister_targetport(struct nvmet_fc_target_port *target_port)
> }
> EXPORT_SYMBOL_GPL(nvmet_fc_unregister_targetport);
>
> -
> /* ********************** FC-NVME LS RCV Handling ************************* */
>
>
Empty line ...
> @@ -2901,12 +2908,17 @@ nvmet_fc_add_port(struct nvmet_port *port)
> list_for_each_entry(tgtport, &nvmet_fc_target_list, tgt_list) {
> if ((tgtport->fc_target_port.node_name == traddr.nn) &&
> (tgtport->fc_target_port.port_name == traddr.pn)) {
> + if (!nvmet_fc_tgtport_get(tgtport))
> + continue;
> +
> /* a FC port can only be 1 nvmet port id */
> if (!tgtport->pe) {
> nvmet_fc_portentry_bind(tgtport, pe, port);
> ret = 0;
> } else
> ret = -EALREADY;
> +
> + nvmet_fc_tgtport_put(tgtport);
> break;
> }
> }
> @@ -2922,11 +2934,21 @@ static void
> nvmet_fc_remove_port(struct nvmet_port *port)
> {
> struct nvmet_fc_port_entry *pe = port->priv;
> + struct nvmet_fc_tgtport *tgtport = NULL;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&nvmet_fc_tgtlock, flags);
> + if (pe->tgtport && nvmet_fc_tgtport_get(pe->tgtport))
> + tgtport = pe->tgtport;
> + spin_unlock_irqrestore(&nvmet_fc_tgtlock, flags);
>
> nvmet_fc_portentry_unbind(pe);
>
> - /* terminate any outstanding associations */
> - __nvmet_fc_free_assocs(pe->tgtport);
> + if (tgtport) {
> + /* terminate any outstanding associations */
> + __nvmet_fc_free_assocs(tgtport);
> + nvmet_fc_tgtport_put(tgtport);
> + }
>
> kfree(pe);
> }
> @@ -2935,10 +2957,21 @@ static void
> nvmet_fc_discovery_chg(struct nvmet_port *port)
> {
> struct nvmet_fc_port_entry *pe = port->priv;
> - struct nvmet_fc_tgtport *tgtport = pe->tgtport;
> + struct nvmet_fc_tgtport *tgtport = NULL;
> + unsigned long flags;
> +
> + spin_lock_irqsave(&nvmet_fc_tgtlock, flags);
> + if (pe->tgtport && nvmet_fc_tgtport_get(pe->tgtport))
> + tgtport = pe->tgtport;
> + spin_unlock_irqrestore(&nvmet_fc_tgtlock, flags);
> +
> + if (!tgtport)
> + return;
>
> if (tgtport && tgtport->ops->discovery_event)
> tgtport->ops->discovery_event(&tgtport->fc_target_port);
> +
> + nvmet_fc_tgtport_put(tgtport);
> }
>
> static ssize_t
>
Otherwise:
Reviewed-by: Hannes Reinecke <hare@...e.de>
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@...e.de +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
Powered by blists - more mailing lists