Re: [PATCH] scsi: ch: Remove the minor from ch_index_idr after device_destroy()

[Laurence Oberman <loberman@redhat.com>]

On Mon, 2026-11-09 at 11:06 -0500, Changyul Lee wrote:
> ch_remove() removes the minor from ch_index_idr before
> device_destroy()
> removes the "sch<N>" device. A changer probed in between can get the
> same minor, and its device_create() fails:
> 
>   sysfs: cannot create duplicate filename '/class/scsi_changer/sch0'
>   ...
>   ch 1:0:0:0: ch0: device_create failed
> 
> The new changer is then not attached.
> 
> Replace the IDR entry with NULL instead, so that ch_open() still
> fails
> with -ENXIO for the changer being removed, and remove the minor after
> device_destroy().
> 
> Fixes: da707c54c342 ("[SCSI] ch: fix device minor number management
> bug")
> Signed-off-by: Changyul Lee <lcy8047@gmail.com>
> ---
> Reproduced in QEMU (x86_64) with scsi_debug (ptype=8, add_host=2),
> which gives two changers on two hosts. Two threads, one per host,
> delete the changer through sysfs and rescan the host in a loop for 20
> seconds. Without this patch, device_create() failed 992 to 1151 times
> per run in three runs. With it, it never failed in two runs, and
> /dev/sch0 can still be opened before and after a delete and rescan.
> 
>  drivers/scsi/ch.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/ch.c b/drivers/scsi/ch.c
> index b2c9fb71c9e0..390425671a3a 100644
> --- a/drivers/scsi/ch.c
> +++ b/drivers/scsi/ch.c
> @@ -952,11 +952,15 @@ static void ch_remove(struct scsi_device *sd)
>  	scsi_changer *ch = dev_get_drvdata(dev);
>  
>  	spin_lock(&ch_index_lock);
> -	idr_remove(&ch_index_idr, ch->minor);
> +	idr_replace(&ch_index_idr, NULL, ch->minor);
>  	dev_set_drvdata(dev, NULL);
>  	spin_unlock(&ch_index_lock);
>  
>  	device_destroy(&ch_sysfs_class, MKDEV(SCSI_CHANGER_MAJOR,
> ch->minor));
> +
> +	spin_lock(&ch_index_lock);
> +	idr_remove(&ch_index_idr, ch->minor);
> +	spin_unlock(&ch_index_lock);
>  	scsi_device_put(ch->device);
>  	kref_put(&ch->ref, ch_destroy);
>  }
> 
> base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4

Looks good to me:
Reviewed-by: Laurence Oberman <loberman@redhat.com>