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

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

[Changyul Lee]

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
-- 
2.43.0

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

[Laurence Oberman]

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>