Re: [PATCH RESEND 2/2] scsi: libfc: drop extra rport reference in fc_rport_create()

[Hannes Reinecke]

On 1/14/20 3:43 PM, Igor Druzhinin wrote:
> The callers of this function seem to assume the reference is not taken
> in case rport already exists. This results in one extra reference taken
> on each rport re-discovery that will eventually get to inability to
> free rport structure on port removal.
> 
> Signed-off-by: Igor Druzhinin <igor.druzhinin@xxxxxxxxxx>
> ---
>  drivers/scsi/libfc/fc_rport.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c
> index da6e97d..a43f9dd 100644
> --- a/drivers/scsi/libfc/fc_rport.c
> +++ b/drivers/scsi/libfc/fc_rport.c
> @@ -133,8 +133,10 @@ struct fc_rport_priv *fc_rport_create(struct fc_lport *lport, u32 port_id)
>  	lockdep_assert_held(&lport->disc.disc_mutex);
>  
>  	rdata = fc_rport_lookup(lport, port_id);
> -	if (rdata)
> +	if (rdata) {
> +		kref_put(&rdata->kref, fc_rport_destroy);
>  		return rdata;
> +	}
>  
>  	if (lport->rport_priv_size > 0)
>  		rport_priv_size = lport->rport_priv_size;
> 
NAK.
The caller _does_ assume that a reference is taken once
fc_rport_create() returns non-NULL.
And the caller is responsible to drop the reference once 'rdatat' isn't
used anymore.
Any other usage is an error, but should be fixed in the caller, not here.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		           Kernel Storage Architect
hare@xxxxxxx			                  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 NÃrnberg
HRB 36809 (AG NÃrnberg), GF: Felix ImendÃrffer