diff mbox series

scsi: Delete scsi_{get,free}_host_dev()

Message ID 1624640314-93055-1-git-send-email-john.garry@huawei.com
State Superseded
Headers show
Series scsi: Delete scsi_{get,free}_host_dev() | expand

Commit Message

John Garry June 25, 2021, 4:58 p.m. UTC
Functions scsi_{get,free}_host_dev() no longer have any in-tree users, so
delete them.

Signed-off-by: John Garry <john.garry@huawei.com>

---
An alt agenda of this patch is to get clarification on whether this API
should be used for Hannes' reserved commands series.

Originally the recommendation was to use it, but now it seems to be to
not use it:
https://lore.kernel.org/linux-scsi/55918d68-7385-0153-0bd9-d822d3ce4c21@suse.de/

-- 
2.26.2

Comments

Bart Van Assche June 25, 2021, 5:58 p.m. UTC | #1
On 6/25/21 9:58 AM, John Garry wrote:
> Functions scsi_{get,free}_host_dev() no longer have any in-tree users, so

> delete them.


It may be a good idea to add a reference to commit 0653c358d2dc ("scsi:
Drop gdth driver") since the gdth driver was the only driver that ever
used these functions. Anyway:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Hannes Reinecke June 26, 2021, 11:55 a.m. UTC | #2
On 6/25/21 6:58 PM, John Garry wrote:
> Functions scsi_{get,free}_host_dev() no longer have any in-tree users, so

> delete them.

> 

> Signed-off-by: John Garry <john.garry@huawei.com>

> ---

> An alt agenda of this patch is to get clarification on whether this API

> should be used for Hannes' reserved commands series.

> 

> Originally the recommendation was to use it, but now it seems to be to

> not use it:

> https://lore.kernel.org/linux-scsi/55918d68-7385-0153-0bd9-d822d3ce4c21@suse.de/

> 

Please don't.

Before we're doing something like this I would like to have 
clarification from Christoph which way he prefers for reserved commands. 
Personally I _do_ like the host dev approach for reserved commands as it 
allows us to re-use existing infrastructure.

Christoph?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                Kernel Storage Architect
hare@suse.de                              +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
John Garry June 29, 2021, 8:15 a.m. UTC | #3
On 26/06/2021 12:55, Hannes Reinecke wrote:
> On 6/25/21 6:58 PM, John Garry wrote:

>> Functions scsi_{get,free}_host_dev() no longer have any in-tree users, so

>> delete them.

>>

>> Signed-off-by: John Garry <john.garry@huawei.com>

>> ---

>> An alt agenda of this patch is to get clarification on whether this API

>> should be used for Hannes' reserved commands series.

>>

>> Originally the recommendation was to use it, but now it seems to be to

>> not use it:

>> https://lore.kernel.org/linux-scsi/55918d68-7385-0153-0bd9-d822d3ce4c21@suse.de/ 

>>

>>

> Please don't.


ok, we can take that as a nacked-by for now.

> 

> Before we're doing something like this I would like to have 

> clarification from Christoph which way he prefers for reserved commands. 

> Personally I _do_ like the host dev approach for reserved commands as it 

> allows us to re-use existing infrastructure.


So when you deleted the gdth driver, the request there was to delete 
this API. And in the v8 series, again, the request was not to use it, 
and use the dedicated request queue instead - I do know that this 
conflicts with the much earlier suggestion not to do that, but that was 
when gdth existed.

As for reuse of existing infrastructure, using this API seems to add 
more code than it reuses.

So don't we just need to add a dedicated request_queue for host? I don't 
know the use in having the scsi_device. Having said all that, we don't 
seem to have a host request_queue sysfs entry with either solution, 
which would be good.

> 

> Christoph?

> 


Thanks,
John
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
index b059bf2b61d4..8e52ce545af3 100644
--- a/drivers/scsi/scsi_scan.c
+++ b/drivers/scsi/scsi_scan.c
@@ -1896,60 +1896,3 @@  void scsi_forget_host(struct Scsi_Host *shost)
 	spin_unlock_irqrestore(shost->host_lock, flags);
 }
 
-/**
- * scsi_get_host_dev - Create a scsi_device that points to the host adapter itself
- * @shost: Host that needs a scsi_device
- *
- * Lock status: None assumed.
- *
- * Returns:     The scsi_device or NULL
- *
- * Notes:
- *	Attach a single scsi_device to the Scsi_Host - this should
- *	be made to look like a "pseudo-device" that points to the
- *	HA itself.
- *
- *	Note - this device is not accessible from any high-level
- *	drivers (including generics), which is probably not
- *	optimal.  We can add hooks later to attach.
- */
-struct scsi_device *scsi_get_host_dev(struct Scsi_Host *shost)
-{
-	struct scsi_device *sdev = NULL;
-	struct scsi_target *starget;
-
-	mutex_lock(&shost->scan_mutex);
-	if (!scsi_host_scan_allowed(shost))
-		goto out;
-	starget = scsi_alloc_target(&shost->shost_gendev, 0, shost->this_id);
-	if (!starget)
-		goto out;
-
-	sdev = scsi_alloc_sdev(starget, 0, NULL);
-	if (sdev)
-		sdev->borken = 0;
-	else
-		scsi_target_reap(starget);
-	put_device(&starget->dev);
- out:
-	mutex_unlock(&shost->scan_mutex);
-	return sdev;
-}
-EXPORT_SYMBOL(scsi_get_host_dev);
-
-/**
- * scsi_free_host_dev - Free a scsi_device that points to the host adapter itself
- * @sdev: Host device to be freed
- *
- * Lock status: None assumed.
- *
- * Returns:     Nothing
- */
-void scsi_free_host_dev(struct scsi_device *sdev)
-{
-	BUG_ON(sdev->id != sdev->host->this_id);
-
-	__scsi_remove_device(sdev);
-}
-EXPORT_SYMBOL(scsi_free_host_dev);
-
diff --git a/include/scsi/scsi_host.h b/include/scsi/scsi_host.h
index 75363707b73f..bc9c45ced145 100644
--- a/include/scsi/scsi_host.h
+++ b/include/scsi/scsi_host.h
@@ -797,16 +797,6 @@  void scsi_host_busy_iter(struct Scsi_Host *,
 
 struct class_container;
 
-/*
- * These two functions are used to allocate and free a pseudo device
- * which will connect to the host adapter itself rather than any
- * physical device.  You must deallocate when you are done with the
- * thing.  This physical pseudo-device isn't real and won't be available
- * from any high-level drivers.
- */
-extern void scsi_free_host_dev(struct scsi_device *);
-extern struct scsi_device *scsi_get_host_dev(struct Scsi_Host *);
-
 /*
  * DIF defines the exchange of protection information between
  * initiator and SBC block device.