On 8/17/24 06:55, Bart Van Assche wrote:
> Let alloc*_workqueue() format the workqueue names instead of calling
> snprintf() explicitly.
>
> Signed-off-by: Bart Van Assche <bvanassche@xxxxxxx>
In patch 1, you have all the changes for removing the use of
create_singlethread_workqueue() in a single patch, touching different drivers.
But the series has 17 more patches to further cleanup the workqueue API use in
various drivers. So why not have the changes in patch 1 split into these
different driver patches with a title like "Cleanup and simplify workqueue API
use" ? That would make reviewing easier I think and avoid having the patch 2-17
changing again code that was changed in patch 1...
> ---
> drivers/message/fusion/mptbase.c | 10 +++-------
> drivers/message/fusion/mptbase.h | 3 ---
> drivers/message/fusion/mptfc.c | 7 ++-----
> 3 files changed, 5 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
> index 4bf669c55649..738bc4e60a18 100644
> --- a/drivers/message/fusion/mptbase.c
> +++ b/drivers/message/fusion/mptbase.c
> @@ -1856,10 +1856,8 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
> /* Initialize workqueue */
> INIT_DELAYED_WORK(&ioc->fault_reset_work, mpt_fault_reset_work);
>
> - snprintf(ioc->reset_work_q_name, MPT_KOBJ_NAME_LEN,
> - "mpt_poll_%d", ioc->id);
> - ioc->reset_work_q = alloc_workqueue(ioc->reset_work_q_name,
> - WQ_MEM_RECLAIM, 0);
> + ioc->reset_work_q =
> + alloc_workqueue("mpt_poll_%d", WQ_MEM_RECLAIM, 0, ioc->id);
> if (!ioc->reset_work_q) {
> printk(MYIOC_s_ERR_FMT "Insufficient memory to add adapter!\n",
> ioc->name);
> @@ -1986,9 +1984,7 @@ mpt_attach(struct pci_dev *pdev, const struct pci_device_id *id)
>
> INIT_LIST_HEAD(&ioc->fw_event_list);
> spin_lock_init(&ioc->fw_event_lock);
> - snprintf(ioc->fw_event_q_name, MPT_KOBJ_NAME_LEN, "mpt/%d", ioc->id);
> - ioc->fw_event_q = alloc_workqueue(ioc->fw_event_q_name,
> - WQ_MEM_RECLAIM, 0);
> + ioc->fw_event_q = alloc_workqueue("mpt/%d", WQ_MEM_RECLAIM, 0, ioc->id);
> if (!ioc->fw_event_q) {
> printk(MYIOC_s_ERR_FMT "Insufficient memory to add adapter!\n",
> ioc->name);
> diff --git a/drivers/message/fusion/mptbase.h b/drivers/message/fusion/mptbase.h
> index 8031173c3655..b406fd676da0 100644
> --- a/drivers/message/fusion/mptbase.h
> +++ b/drivers/message/fusion/mptbase.h
> @@ -729,7 +729,6 @@ typedef struct _MPT_ADAPTER
> struct list_head fw_event_list;
> spinlock_t fw_event_lock;
> u8 fw_events_off; /* if '1', then ignore events */
> - char fw_event_q_name[MPT_KOBJ_NAME_LEN];
>
> struct mutex sas_discovery_mutex;
> u8 sas_discovery_runtime;
> @@ -764,7 +763,6 @@ typedef struct _MPT_ADAPTER
> u8 fc_link_speed[2];
> spinlock_t fc_rescan_work_lock;
> struct work_struct fc_rescan_work;
> - char fc_rescan_work_q_name[MPT_KOBJ_NAME_LEN];
> struct workqueue_struct *fc_rescan_work_q;
>
> /* driver forced bus resets count */
> @@ -778,7 +776,6 @@ typedef struct _MPT_ADAPTER
> spinlock_t scsi_lookup_lock;
> u64 dma_mask;
> u32 broadcast_aen_busy;
> - char reset_work_q_name[MPT_KOBJ_NAME_LEN];
> struct workqueue_struct *reset_work_q;
> struct delayed_work fault_reset_work;
>
> diff --git a/drivers/message/fusion/mptfc.c b/drivers/message/fusion/mptfc.c
> index a3c17c4fe69c..91242f26defb 100644
> --- a/drivers/message/fusion/mptfc.c
> +++ b/drivers/message/fusion/mptfc.c
> @@ -1349,11 +1349,8 @@ mptfc_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>
> /* initialize workqueue */
>
> - snprintf(ioc->fc_rescan_work_q_name, sizeof(ioc->fc_rescan_work_q_name),
> - "mptfc_wq_%d", sh->host_no);
> - ioc->fc_rescan_work_q =
> - alloc_ordered_workqueue(ioc->fc_rescan_work_q_name,
> - WQ_MEM_RECLAIM);
> + ioc->fc_rescan_work_q = alloc_ordered_workqueue(
> + "mptfc_wq_%d", WQ_MEM_RECLAIM, sh->host_no);
> if (!ioc->fc_rescan_work_q) {
> error = -ENOMEM;
> goto out_mptfc_host;
>
--
Damien Le Moal
Western Digital Research
