Re: [PATCHv2 1/1] SCSI: update hosts module to use idr index management

[Hannes Reinecke]

On 10/06/2015 07:17 PM, James Bottomley wrote:
> On Tue, 2015-10-06 at 10:14 -0700, Lee Duncan wrote:
> > On 10/06/2015 02:40 AM, Christoph Hellwig wrote:
> > > >   struct Scsi_Host *scsi_host_lookup(unsigned short hostnum)
> > > >   {
> > > > -	struct device *cdev;
> > > > -	struct Scsi_Host *shost = NULL;
> > > > -
> > > > -	cdev = class_find_device(&shost_class, NULL, &hostnum,
> > > > -				 __scsi_host_match);
> > > > -	if (cdev) {
> > > > -		shost = scsi_host_get(class_to_shost(cdev));
> > > > -		put_device(cdev);
> > > > -	}
> > > > +	struct Scsi_Host *shost;
> > > > +
> > > > +	spin_lock(&host_index_lock);
> > > > +	shost = idr_find(&host_index_idr, hostnum);
> > > > +	spin_unlock(&host_index_lock);
> > > > +
> > > >   	return shost;
> > >
> > > How does this actually grab a reference to the host?
> >
> > Good catch -- I should have noticed that.
> >
> > I will resubmit the patch.
>
> I'll wait to see what you produce, but I don't think, using a separate
> idr array, you can close the race window between lookup and get.  One of
> the nice things about using the cdev iterator is that the get is part of
> the lookup process.
Hmm. Should be possible if you free up the IDR in scsi_remove_host(), 
just after setting the state to SHOST_DEL.

Cheers,

Hannes

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/