| Message ID | 20240402130645.653507-12-hch@lst.de (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b57089d32c2c72d0b963f11d612042579c665f27 |
| Headers | show |
| Series | [01/23] block: add a helper to cancel atomic queue limit updates | expand |
On 4/2/24 15:06, 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. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > Reviewed-by: Damien Le Moal <dlemoal@kernel.org> > --- > drivers/scsi/megaraid/megaraid_sas.h | 2 +- > drivers/scsi/megaraid/megaraid_sas_base.c | 29 ++++++++++++--------- > drivers/scsi/megaraid/megaraid_sas_fusion.c | 3 ++- > 3 files changed, 20 insertions(+), 14 deletions(-) > > diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h > index 56624cbf7fa5e7..5680c6cdb22193 100644 > --- a/drivers/scsi/megaraid/megaraid_sas.h > +++ b/drivers/scsi/megaraid/megaraid_sas.h > @@ -2701,7 +2701,7 @@ int megasas_get_ctrl_info(struct megasas_instance *instance); > int > megasas_sync_pd_seq_num(struct megasas_instance *instance, bool pend); > void megasas_set_dynamic_target_properties(struct scsi_device *sdev, > - bool is_target_prop); > + struct queue_limits *lim, bool is_target_prop); > int megasas_get_target_prop(struct megasas_instance *instance, > struct scsi_device *sdev); > void megasas_get_snapdump_properties(struct megasas_instance *instance); > diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c > index 3d4f13da1ae873..def0d905b6d9e3 100644 > --- a/drivers/scsi/megaraid/megaraid_sas_base.c > +++ b/drivers/scsi/megaraid/megaraid_sas_base.c > @@ -1888,7 +1888,7 @@ static struct megasas_instance *megasas_lookup_instance(u16 host_no) > * Returns void > */ > void megasas_set_dynamic_target_properties(struct scsi_device *sdev, > - bool is_target_prop) > + struct queue_limits *lim, bool is_target_prop) > { > u16 pd_index = 0, ld; > u32 device_id; > @@ -1915,8 +1915,10 @@ void megasas_set_dynamic_target_properties(struct scsi_device *sdev, > return; > raid = MR_LdRaidGet(ld, local_map_ptr); > > - if (raid->capability.ldPiMode == MR_PROT_INFO_TYPE_CONTROLLER) > - blk_queue_update_dma_alignment(sdev->request_queue, 0x7); > + if (raid->capability.ldPiMode == MR_PROT_INFO_TYPE_CONTROLLER) { > + if (lim) > + lim->dma_alignment = 0x7; > + } > > mr_device_priv_data->is_tm_capable = > raid->capability.tmCapable; > @@ -1967,7 +1969,8 @@ void megasas_set_dynamic_target_properties(struct scsi_device *sdev, > * > */ > static inline void > -megasas_set_nvme_device_properties(struct scsi_device *sdev, u32 max_io_size) > +megasas_set_nvme_device_properties(struct scsi_device *sdev, > + struct queue_limits *lim, u32 max_io_size) > { > struct megasas_instance *instance; > u32 mr_nvme_pg_size; > @@ -1976,10 +1979,10 @@ megasas_set_nvme_device_properties(struct scsi_device *sdev, u32 max_io_size) > mr_nvme_pg_size = max_t(u32, instance->nvme_page_size, > MR_DEFAULT_NVME_PAGE_SIZE); > > - blk_queue_max_hw_sectors(sdev->request_queue, (max_io_size / 512)); > + lim->max_hw_sectors = max_io_size / 512; > + lim->virt_boundary_mask = mr_nvme_pg_size - 1; > > blk_queue_flag_set(QUEUE_FLAG_NOMERGES, sdev->request_queue); > - blk_queue_virt_boundary(sdev->request_queue, mr_nvme_pg_size - 1); > } That now looks odd. We're taking great pains to have everything in queue_limits and avoid having to use the request_queue directly, yet this one call we're missing. Wouldn't it make sense to move that into queue_limits, too? Cheers, Hannes
On Wed, Apr 03, 2024 at 09:06:12AM +0200, Hannes Reinecke wrote: >> + lim->virt_boundary_mask = mr_nvme_pg_size - 1; >> blk_queue_flag_set(QUEUE_FLAG_NOMERGES, sdev->request_queue); >> - blk_queue_virt_boundary(sdev->request_queue, mr_nvme_pg_size - 1); >> } > That now looks odd. > We're taking great pains to have everything in queue_limits and avoid > having to use the request_queue directly, yet this one call we're missing. > Wouldn't it make sense to move that into queue_limits, too? The queue flags are in the queue, so there is no way to set them through the limits. I plan to eventually split out actual features and move them to the limits from the blk-mq internal state flags. That being said QUEUE_FLAG_NOMERGES is a really weird one and drivers shouldn't really be messing with it at all..
diff --git a/drivers/scsi/megaraid/megaraid_sas.h b/drivers/scsi/megaraid/megaraid_sas.h index 56624cbf7fa5e7..5680c6cdb22193 100644 --- a/drivers/scsi/megaraid/megaraid_sas.h +++ b/drivers/scsi/megaraid/megaraid_sas.h @@ -2701,7 +2701,7 @@ int megasas_get_ctrl_info(struct megasas_instance *instance); int megasas_sync_pd_seq_num(struct megasas_instance *instance, bool pend); void megasas_set_dynamic_target_properties(struct scsi_device *sdev, - bool is_target_prop); + struct queue_limits *lim, bool is_target_prop); int megasas_get_target_prop(struct megasas_instance *instance, struct scsi_device *sdev); void megasas_get_snapdump_properties(struct megasas_instance *instance); diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c b/drivers/scsi/megaraid/megaraid_sas_base.c index 3d4f13da1ae873..def0d905b6d9e3 100644 --- a/drivers/scsi/megaraid/megaraid_sas_base.c +++ b/drivers/scsi/megaraid/megaraid_sas_base.c @@ -1888,7 +1888,7 @@ static struct megasas_instance *megasas_lookup_instance(u16 host_no) * Returns void */ void megasas_set_dynamic_target_properties(struct scsi_device *sdev, - bool is_target_prop) + struct queue_limits *lim, bool is_target_prop) { u16 pd_index = 0, ld; u32 device_id; @@ -1915,8 +1915,10 @@ void megasas_set_dynamic_target_properties(struct scsi_device *sdev, return; raid = MR_LdRaidGet(ld, local_map_ptr); - if (raid->capability.ldPiMode == MR_PROT_INFO_TYPE_CONTROLLER) - blk_queue_update_dma_alignment(sdev->request_queue, 0x7); + if (raid->capability.ldPiMode == MR_PROT_INFO_TYPE_CONTROLLER) { + if (lim) + lim->dma_alignment = 0x7; + } mr_device_priv_data->is_tm_capable = raid->capability.tmCapable; @@ -1967,7 +1969,8 @@ void megasas_set_dynamic_target_properties(struct scsi_device *sdev, * */ static inline void -megasas_set_nvme_device_properties(struct scsi_device *sdev, u32 max_io_size) +megasas_set_nvme_device_properties(struct scsi_device *sdev, + struct queue_limits *lim, u32 max_io_size) { struct megasas_instance *instance; u32 mr_nvme_pg_size; @@ -1976,10 +1979,10 @@ megasas_set_nvme_device_properties(struct scsi_device *sdev, u32 max_io_size) mr_nvme_pg_size = max_t(u32, instance->nvme_page_size, MR_DEFAULT_NVME_PAGE_SIZE); - blk_queue_max_hw_sectors(sdev->request_queue, (max_io_size / 512)); + lim->max_hw_sectors = max_io_size / 512; + lim->virt_boundary_mask = mr_nvme_pg_size - 1; blk_queue_flag_set(QUEUE_FLAG_NOMERGES, sdev->request_queue); - blk_queue_virt_boundary(sdev->request_queue, mr_nvme_pg_size - 1); } /* @@ -2041,7 +2044,7 @@ static void megasas_set_fw_assisted_qd(struct scsi_device *sdev, * @is_target_prop true, if fw provided target properties. */ static void megasas_set_static_target_properties(struct scsi_device *sdev, - bool is_target_prop) + struct queue_limits *lim, bool is_target_prop) { u32 max_io_size_kb = MR_DEFAULT_NVME_MDTS_KB; struct megasas_instance *instance; @@ -2060,13 +2063,15 @@ static void megasas_set_static_target_properties(struct scsi_device *sdev, max_io_size_kb = le32_to_cpu(instance->tgt_prop->max_io_size_kb); if (instance->nvme_page_size && max_io_size_kb) - megasas_set_nvme_device_properties(sdev, (max_io_size_kb << 10)); + megasas_set_nvme_device_properties(sdev, lim, + max_io_size_kb << 10); megasas_set_fw_assisted_qd(sdev, is_target_prop); } -static int megasas_slave_configure(struct scsi_device *sdev) +static int megasas_device_configure(struct scsi_device *sdev, + struct queue_limits *lim) { u16 pd_index = 0; struct megasas_instance *instance; @@ -2096,10 +2101,10 @@ static int megasas_slave_configure(struct scsi_device *sdev) ret_target_prop = megasas_get_target_prop(instance, sdev); is_target_prop = (ret_target_prop == DCMD_SUCCESS) ? true : false; - megasas_set_static_target_properties(sdev, is_target_prop); + megasas_set_static_target_properties(sdev, lim, is_target_prop); /* This sdev property may change post OCR */ - megasas_set_dynamic_target_properties(sdev, is_target_prop); + megasas_set_dynamic_target_properties(sdev, lim, is_target_prop); mutex_unlock(&instance->reset_mutex); @@ -3507,7 +3512,7 @@ static const struct scsi_host_template megasas_template = { .module = THIS_MODULE, .name = "Avago SAS based MegaRAID driver", .proc_name = "megaraid_sas", - .slave_configure = megasas_slave_configure, + .device_configure = megasas_device_configure, .slave_alloc = megasas_slave_alloc, .slave_destroy = megasas_slave_destroy, .queuecommand = megasas_queue_command, diff --git a/drivers/scsi/megaraid/megaraid_sas_fusion.c b/drivers/scsi/megaraid/megaraid_sas_fusion.c index c60014e07b449e..6c1fb8149553a8 100644 --- a/drivers/scsi/megaraid/megaraid_sas_fusion.c +++ b/drivers/scsi/megaraid/megaraid_sas_fusion.c @@ -5119,7 +5119,8 @@ int megasas_reset_fusion(struct Scsi_Host *shost, int reason) ret_target_prop = megasas_get_target_prop(instance, sdev); is_target_prop = (ret_target_prop == DCMD_SUCCESS) ? true : false; - megasas_set_dynamic_target_properties(sdev, is_target_prop); + megasas_set_dynamic_target_properties(sdev, NULL, + is_target_prop); } status_reg = instance->instancet->read_fw_status_reg