diff mbox series

[v4,3/6] scsi: merge scsi_internal_device_block() and device_block()

Message ID 20230612150309.18103-4-mwilck@suse.com (mailing list archive)
State Superseded
Headers show
Series scsi: fixes for targets with many LUNs, and scsi_target_block rework | expand

Commit Message

Martin Wilck June 12, 2023, 3:03 p.m. UTC 
From: Martin Wilck <mwilck@suse.com>

scsi_internal_device_block() is only called from device_block().
Merge the two functions, and call the result scsi_device_block(),
as the name device_block() is confusingly generic.

Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
---
 drivers/scsi/scsi_lib.c | 27 ++++++++-------------------
 1 file changed, 8 insertions(+), 19 deletions(-)

Comments

Bart Van Assche June 12, 2023, 3:35 p.m. UTC | #1 
On 6/12/23 08:03, mwilck@suse.com wrote:
> -static int scsi_internal_device_block(struct scsi_device *sdev)
> +static void scsi_device_block(struct scsi_device *sdev, void *data)
>   {
>   	int err;
>   
> @@ -2805,7 +2804,8 @@ static int scsi_internal_device_block(struct scsi_device *sdev)
>   		scsi_stop_queue(sdev, false);
>   	mutex_unlock(&sdev->state_mutex);
>   
> -	return err;
> +	WARN_ONCE(err, "__scsi_internal_device_block_nowait(%s) failed: err = %d\n",
> +		  dev_name(&sdev->sdev_gendev), err);
>   }

Hmm ... shouldn't the WARN_ONCE() statement refer to scsi_device_block() 
instead of __scsi_internal_device_block_nowait()?

Thanks,

Bart.
Martin Wilck June 12, 2023, 4:06 p.m. UTC | #2 
On Mon, 2023-06-12 at 08:35 -0700, Bart Van Assche wrote:
> On 6/12/23 08:03, mwilck@suse.com wrote:
> > -static int scsi_internal_device_block(struct scsi_device *sdev)
> > +static void scsi_device_block(struct scsi_device *sdev, void
> > *data)
> >   {
> >         int err;
> >   
> > @@ -2805,7 +2804,8 @@ static int scsi_internal_device_block(struct
> > scsi_device *sdev)
> >                 scsi_stop_queue(sdev, false);
> >         mutex_unlock(&sdev->state_mutex);
> >   
> > -       return err;
> > +       WARN_ONCE(err, "__scsi_internal_device_block_nowait(%s)
> > failed: err = %d\n",
> > +                 dev_name(&sdev->sdev_gendev), err);
> >   }
> 
> Hmm ... shouldn't the WARN_ONCE() statement refer to
> scsi_device_block() 
> instead of __scsi_internal_device_block_nowait()?

The message references the function that failed, and prints the return
value of that function. It used to be scsi_internal_device_block()
before my patch, and it's __scsi_internal_device_block_nowait() now. 

But I agree that the message is ugly. I'll to come up with something
better.

Thanks,
Martin
diff mbox series

Patch

diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
index 496bdfc19c95..69fb7a9d8883 100644
--- a/drivers/scsi/scsi_lib.c
+++ b/drivers/scsi/scsi_lib.c
@@ -2781,13 +2781,12 @@  int scsi_internal_device_block_nowait(struct scsi_device *sdev)
 EXPORT_SYMBOL_GPL(scsi_internal_device_block_nowait);
 
 /**
- * scsi_internal_device_block - try to transition to the SDEV_BLOCK state
+ * scsi_device_block - try to transition to the SDEV_BLOCK state
  * @sdev: device to block
+ * @data: dummy argument, ignored
  *
  * Pause SCSI command processing on the specified device and wait until all
- * ongoing scsi_request_fn() / scsi_queue_rq() calls have finished. May sleep.
- *
- * Returns zero if successful or a negative error code upon failure.
+ * ongoing scsi_queue_rq() calls have finished. May sleep.
  *
  * Note:
  * This routine transitions the device to the SDEV_BLOCK state (which must be
@@ -2795,7 +2794,7 @@  EXPORT_SYMBOL_GPL(scsi_internal_device_block_nowait);
  * is paused until the device leaves the SDEV_BLOCK state. See also
  * scsi_internal_device_unblock().
  */
-static int scsi_internal_device_block(struct scsi_device *sdev)
+static void scsi_device_block(struct scsi_device *sdev, void *data)
 {
 	int err;
 
@@ -2805,7 +2804,8 @@  static int scsi_internal_device_block(struct scsi_device *sdev)
 		scsi_stop_queue(sdev, false);
 	mutex_unlock(&sdev->state_mutex);
 
-	return err;
+	WARN_ONCE(err, "__scsi_internal_device_block_nowait(%s) failed: err = %d\n",
+		  dev_name(&sdev->sdev_gendev), err);
 }
 
 /**
@@ -2888,23 +2888,12 @@  static int scsi_internal_device_unblock(struct scsi_device *sdev,
 	return ret;
 }
 
-static void
-device_block(struct scsi_device *sdev, void *data)
-{
-	int ret;
-
-	ret = scsi_internal_device_block(sdev);
-
-	WARN_ONCE(ret, "scsi_internal_device_block(%s) failed: ret = %d\n",
-		  dev_name(&sdev->sdev_gendev), ret);
-}
-
 static int
 target_block(struct device *dev, void *data)
 {
 	if (scsi_is_target_device(dev))
 		starget_for_each_device(to_scsi_target(dev), NULL,
-					device_block);
+					scsi_device_block);
 	return 0;
 }
 
@@ -2913,7 +2902,7 @@  scsi_target_block(struct device *dev)
 {
 	if (scsi_is_target_device(dev))
 		starget_for_each_device(to_scsi_target(dev), NULL,
-					device_block);
+					scsi_device_block);
 	else
 		device_for_each_child(dev, NULL, target_block);
 }