Re: [PATCH] sym53c8xx PPR negotiation fix

Mike Anderson
2003-11-06T09:30:14Z

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