Re: [PATCH] sym53c8xx PPR negotiation fix

[Mike Anderson <andmike@us.ibm.com>]

Christoph Hellwig [hch@infradead.org] wrote:
> > @@ -282,24 +301,9 @@ out:
> >   **/
> >  void scsi_free_sdev(struct scsi_device *sdev)
> >  {
> > -	unsigned long flags;
> > -
> > -	spin_lock_irqsave(sdev->host->host_lock, flags);
> > -	list_del(&sdev->siblings);
> > -	list_del(&sdev->same_target_siblings);
> > -	spin_unlock_irqrestore(sdev->host->host_lock, flags);
> > -
> > -	if (sdev->request_queue)
> > -		scsi_free_queue(sdev->request_queue);
> > -
> > -	spin_lock_irqsave(sdev->host->host_lock, flags);
> > -	list_del(&sdev->starved_entry);
> > -	if (sdev->single_lun && --sdev->sdev_target->starget_refcnt == 0)
> > -		kfree(sdev->sdev_target);
> > -	spin_unlock_irqrestore(sdev->host->host_lock, flags);
> > -
> > -	kfree(sdev->inquiry);
> > -	kfree(sdev);
> > +	if (sdev->host->hostt->slave_destroy)
> > +		sdev->host->hostt->slave_destroy(sdev);
> > +	put_device(&sdev->sdev_gendev);
> 
> The name of this function is rather misleading now, I'd rather kill
> it and inline it into the caller - it's just three lines anyway.
> 
> Otherwise the patch looks ok to me, but I need to look at a tree
> with it applied before coming to final conclusions..

I agree on the misleading name. I debated about inlining it when I was
doing the last updates. I will update my patch and wait for anymore
feedback.

-andmike
--
Michael Anderson
andmike@us.ibm.com