> Here's a proposed patch. The real problem in all of this was that
> we had 2 places allocating a scsi target (or should I say, that
> invoked the scan function that makes the target, but the parents
> are different). This patch adds a transport alloc_target function,
> which allows the transport to adjusts the parent, and then calls
> back into the midlayer to complete the allocation (small recursive
> call). It has one additional optimization in that, if the transport
> is doling out targets, it fails target allocations on things that
> don't exist. This will make scanning slightly faster. I also like
> this patch as it leaves all "scan" logic in the midlayer.
I don't like the recursion part of this at all. I'd suggest to make
scsi_alloc_target a tiny wrapper around the transport ->alloc_target +
a default action, the shared body that's in scsi_alloc_target will
become __scsi_alloc_target.
> + spin_lock_irqsave(shost->host_lock, flags);
> + list_for_each_entry_safe(rport, next_rport,
> + &fc_host_rports(shost), peers) {
you don't actually need list_for_each_entry_safe here as you're dropping
out of the loop once the right rport has been found.
> + if ((rport->channel == channel) &&
> + (rport->scsi_target_id == id)) {
> + spin_unlock_irqrestore(shost->host_lock, flags);
> + starget = scsi_alloc_target(&rport->dev, channel, id);
> + if (starget)
> + return 0;
> + return 1;
this is always under the scan_mutex, right? else we'd get a race after
the sin_unlock but before registering the new target.
-
: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
