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

From: Laurence Oberman

Date: Mon Nov 09 2026 - 13:04:45 EST


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@xxxxxxxxx>
> ---
> 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@xxxxxxxxxx>