On 5/28/20 9:24 AM, Daniel Wagner wrote:
On Wed, May 27, 2020 at 04:13:57PM +0200, Hannes Reinecke wrote:
--- a/drivers/scsi/scsi_sysfs.c
+++ b/drivers/scsi/scsi_sysfs.c
@@ -1512,15 +1512,19 @@ void scsi_remove_target(struct device *dev)
{
struct Scsi_Host *shost = dev_to_shost(dev->parent);
struct scsi_target *starget;
+ unsigned long tid = 0;
unsigned long flags;
-restart:
spin_lock_irqsave(shost->host_lock, flags);
- list_for_each_entry(starget, &shost->__targets, siblings) {
+ starget = xa_find(&shost->__targets, &tid, ULONG_MAX, XA_PRESENT);
+ while (starget) {
if (starget->state == STARGET_DEL ||
starget->state == STARGET_REMOVE ||
- starget->state == STARGET_CREATED_REMOVE)
+ starget->state == STARGET_CREATED_REMOVE) {
+ starget = xa_find_after(&shost->__targets, &tid,
+ ULONG_MAX, XA_PRESENT);
continue;
+ }
if (starget->dev.parent == dev || &starget->dev == dev) {
kref_get(&starget->reap_ref);
if (starget->state == STARGET_CREATED)
@@ -1530,7 +1534,10 @@ void scsi_remove_target(struct device *dev)
spin_unlock_irqrestore(shost->host_lock, flags);
__scsi_remove_target(starget);
scsi_target_reap(starget);
- goto restart;
+ spin_lock_irqsave(shost->host_lock, flags);
+ starget = xa_find_after(&shost->__targets, &tid,
+ ULONG_MAX, XA_PRESENT);
+ continue;
}
Is there a special reason why don't use xa_for_each to iterate through all
entries?
This entire function is completely daft.
It says 'scsi_remove_target()', but for some weird reason fails to pass
in the target it want to delete, so it has to go round in circles trying
to figure out which target to delete.
There probably had been an obscure reason for this, but with the current
code it's just pointless.
So that's the next thing to fix (after all of this): use a struct
scsi_target as argument for this function, then this entire loop can go.
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer