Re: [PATCH 21/23] mpi3mr: switch to using ->device_configure

[Damien Le Moal]

On 3/25/24 08:54, Christoph Hellwig wrote:
> Switch to the ->device_configure method instead of ->slave_configure
> and update the block limits on the passed in queue_limits instead
> of using the per-limit accessors.
> 
> Note that mpi3mr also updates the limits from an event handler that
> iterates all SCSI devices.  This is also updated to use the
> queue_limits, but the complete locking of this path probably means
> it already is completely broken and needs a proper audit.
> 
> Signed-off-by: Christoph Hellwig 

Looks OK to me.

Reviewed-by: Damien Le Moal 

-- 
Damien Le Moal
Western Digital Research

-- 
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to open-iscsi+unsubscr...@googlegroups.com.
To view this discussion on the web visit 
https://groups.google.com/d/msgid/open-iscsi/66169fb4-efeb-4da5-8280-a0585537acfd%40kernel.org.

[PATCH 21/23] mpi3mr: switch to using ->device_configure

[Christoph Hellwig]

Switch to the ->device_configure method instead of ->slave_configure
and update the block limits on the passed in queue_limits instead
of using the per-limit accessors.

Note that mpi3mr also updates the limits from an event handler that
iterates all SCSI devices.  This is also updated to use the
queue_limits, but the complete locking of this path probably means
it already is completely broken and needs a proper audit.

Signed-off-by: Christoph Hellwig 
---
 drivers/scsi/mpi3mr/mpi3mr.h|  1 -
 drivers/scsi/mpi3mr/mpi3mr_os.c | 76 ++---
 2 files changed, 31 insertions(+), 46 deletions(-)

diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
index 3de1ee05c44e4c..565d6047f68cb8 100644
--- a/drivers/scsi/mpi3mr/mpi3mr.h
+++ b/drivers/scsi/mpi3mr/mpi3mr.h
@@ -1348,7 +1348,6 @@ void mpi3mr_wait_for_host_io(struct mpi3mr_ioc *mrioc, 
u32 timeout);
 void mpi3mr_cleanup_fwevt_list(struct mpi3mr_ioc *mrioc);
 void mpi3mr_flush_host_io(struct mpi3mr_ioc *mrioc);
 void mpi3mr_invalidate_devhandles(struct mpi3mr_ioc *mrioc);
-void mpi3mr_rfresh_tgtdevs(struct mpi3mr_ioc *mrioc);
 void mpi3mr_flush_delayed_cmd_lists(struct mpi3mr_ioc *mrioc);
 void mpi3mr_check_rh_fault_ioc(struct mpi3mr_ioc *mrioc, u32 reason_code);
 void mpi3mr_print_fault_info(struct mpi3mr_ioc *mrioc);
diff --git a/drivers/scsi/mpi3mr/mpi3mr_os.c b/drivers/scsi/mpi3mr/mpi3mr_os.c
index 73c831a97d276a..de3e02446c6814 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_os.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_os.c
@@ -986,6 +986,25 @@ static int mpi3mr_change_queue_depth(struct scsi_device 
*sdev,
return retval;
 }
 
+static void mpi3mr_configure_nvme_dev(struct mpi3mr_tgt_dev *tgt_dev,
+   struct queue_limits *lim)
+{
+   u8 pgsz = tgt_dev->dev_spec.pcie_inf.pgsz ? : MPI3MR_DEFAULT_PGSZEXP;
+
+   lim->max_hw_sectors = tgt_dev->dev_spec.pcie_inf.mdts / 512;
+   lim->virt_boundary_mask = (1 << pgsz) - 1;
+}
+
+static void mpi3mr_configure_tgt_dev(struct mpi3mr_tgt_dev *tgt_dev,
+   struct queue_limits *lim)
+{
+   if (tgt_dev->dev_type == MPI3_DEVICE_DEVFORM_PCIE &&
+   (tgt_dev->dev_spec.pcie_inf.dev_info &
+MPI3_DEVICE0_PCIE_DEVICE_INFO_TYPE_MASK) ==
+   MPI3_DEVICE0_PCIE_DEVICE_INFO_TYPE_NVME_DEVICE)
+   mpi3mr_configure_nvme_dev(tgt_dev, lim);
+}
+
 /**
  * mpi3mr_update_sdev - Update SCSI device information
  * @sdev: SCSI device reference
@@ -1001,31 +1020,17 @@ static void
 mpi3mr_update_sdev(struct scsi_device *sdev, void *data)
 {
struct mpi3mr_tgt_dev *tgtdev;
+   struct queue_limits lim;
 
tgtdev = (struct mpi3mr_tgt_dev *)data;
if (!tgtdev)
return;
 
mpi3mr_change_queue_depth(sdev, tgtdev->q_depth);
-   switch (tgtdev->dev_type) {
-   case MPI3_DEVICE_DEVFORM_PCIE:
-   /*The block layer hw sector size = 512*/
-   if ((tgtdev->dev_spec.pcie_inf.dev_info &
-   MPI3_DEVICE0_PCIE_DEVICE_INFO_TYPE_MASK) ==
-   MPI3_DEVICE0_PCIE_DEVICE_INFO_TYPE_NVME_DEVICE) {
-   blk_queue_max_hw_sectors(sdev->request_queue,
-   tgtdev->dev_spec.pcie_inf.mdts / 512);
-   if (tgtdev->dev_spec.pcie_inf.pgsz == 0)
-   blk_queue_virt_boundary(sdev->request_queue,
-   ((1 << MPI3MR_DEFAULT_PGSZEXP) - 1));
-   else
-   blk_queue_virt_boundary(sdev->request_queue,
-   ((1 << tgtdev->dev_spec.pcie_inf.pgsz) - 
1));
-   }
-   break;
-   default:
-   break;
-   }
+
+   lim = queue_limits_start_update(sdev->request_queue);
+   mpi3mr_configure_tgt_dev(tgtdev, &lim);
+   WARN_ON_ONCE(queue_limits_commit_update(sdev->request_queue, &lim));
 }
 
 /**
@@ -1038,8 +1043,7 @@ mpi3mr_update_sdev(struct scsi_device *sdev, void *data)
  *
  * Return: Nothing.
  */
-
-void mpi3mr_rfresh_tgtdevs(struct mpi3mr_ioc *mrioc)
+static void mpi3mr_rfresh_tgtdevs(struct mpi3mr_ioc *mrioc)
 {
struct mpi3mr_tgt_dev *tgtdev, *tgtdev_next;
struct mpi3mr_stgt_priv_data *tgt_priv;
@@ -4393,15 +4397,17 @@ static void mpi3mr_target_destroy(struct scsi_target 
*starget)
 }
 
 /**
- * mpi3mr_slave_configure - Slave configure callback handler
+ * mpi3mr_device_configure - Slave configure callback handler
  * @sdev: SCSI device reference
+ * @lim: queue limits
  *
  * Configure queue depth, max hardware sectors and virt boundary
  * as required
  *
  * Return: 0 always.
  */
-static int mpi3mr_slave_configure(struct scsi_device *sdev)
+static int mpi3mr_device_configure(struct scsi_device *sdev,
+   struct queue_limits *lim)
 {
struct scsi_target *starget;
struct Scsi_Host *shost;
@@ -4432,28 +4438,8 @@ static int mpi3mr_slave_configure(struct scsi_device