linux-kernel - Re: [PATCH RFT v2] driver/scsi/mpi3mr: Fix build warning for mpi3mr_start

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <562fa035-c732-4bfc-8439-2279d029f72a@acm.org>
Date: Wed, 12 Nov 2025 12:55:43 -0800
From: Bart Van Assche <bvanassche@....org>
To: Bartlomiej Kubik <kubik.bartlomiej@...il.com>,
 sathya.prakash@...adcom.com, kashyap.desai@...adcom.com,
 sumit.saxena@...adcom.com, sreekanth.reddy@...adcom.com
Cc: martin.petersen@...cle.com, mpi3mr-linuxdrv.pdl@...adcom.com,
 linux-scsi@...r.kernel.org, skhan@...uxfoundation.org, khalid@...nel.org,
 david.hunter.linux@...il.com,
 linux-kernel-mentees@...ts.linuxfoundation.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH RFT v2] driver/scsi/mpi3mr: Fix build warning for
 mpi3mr_start_watchdog

On 10/28/25 7:55 AM, Bartlomiej Kubik wrote:
> -	char watchdog_work_q_name[50];
> +	char watchdog_work_q_name[MPI3MR_WATCHDOG_NAME_LENGTH];

 From include/linux/workqueue.h:

	WQ_NAME_LEN		= 32,

	char			name[WQ_NAME_LEN]; /* I: workqueue name */

In other words, increasing the workqueue name length beyond 32
characters is not useful because it will get truncated to 32 characters
anyway. The workqueue implementation complains about longer names as one
can see in kernel/workqueue.c:

	if (name_len >= WQ_NAME_LEN)
		pr_warn_once("workqueue: name exceeds WQ_NAME_LEN. Truncating to: %s\n",
			     wq->name);

> diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c b/drivers/scsi/mpi3mr/mpi3mr_fw.c
> index 8fe6e0bf342e..18b176e358c5 100644
> --- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
> +++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
> @@ -2879,8 +2879,7 @@ void mpi3mr_start_watchdog(struct mpi3mr_ioc *mrioc)
> 
>   	INIT_DELAYED_WORK(&mrioc->watchdog_work, mpi3mr_watchdog_work);
>   	snprintf(mrioc->watchdog_work_q_name,
> -	    sizeof(mrioc->watchdog_work_q_name), "watchdog_%s%d", mrioc->name,
> -	    mrioc->id);
> +	    sizeof(mrioc->watchdog_work_q_name), "watchdog_%s", mrioc->name);
>   	mrioc->watchdog_work_q = alloc_ordered_workqueue(
>   		"%s", WQ_MEM_RECLAIM, mrioc->watchdog_work_q_name);
>   	if (!mrioc->watchdog_work_q) {
Leaving out mrioc->id from the workqueue name seems like an unacceptable
behavior change to me.

Please consider replacing the proposed changed with this untested patch:

diff --git a/drivers/scsi/mpi3mr/mpi3mr.h b/drivers/scsi/mpi3mr/mpi3mr.h
index 6742684e2990..050dcf111a4c 100644
--- a/drivers/scsi/mpi3mr/mpi3mr.h
+++ b/drivers/scsi/mpi3mr/mpi3mr.h
@@ -1076,7 +1076,6 @@ struct scmd_priv {
   * @fwevt_worker_thread: Firmware event worker thread
   * @fwevt_lock: Firmware event lock
   * @fwevt_list: Firmware event list
- * @watchdog_work_q_name: Fault watchdog worker thread name
   * @watchdog_work_q: Fault watchdog worker thread
   * @watchdog_work: Fault watchdog work
   * @watchdog_lock: Fault watchdog lock
@@ -1265,7 +1264,6 @@ struct mpi3mr_ioc {
  	spinlock_t fwevt_lock;
  	struct list_head fwevt_list;

-	char watchdog_work_q_name[50];
  	struct workqueue_struct *watchdog_work_q;
  	struct delayed_work watchdog_work;
  	spinlock_t watchdog_lock;
diff --git a/drivers/scsi/mpi3mr/mpi3mr_fw.c 
b/drivers/scsi/mpi3mr/mpi3mr_fw.c
index 8fe6e0bf342e..b564fe5980a6 100644
--- a/drivers/scsi/mpi3mr/mpi3mr_fw.c
+++ b/drivers/scsi/mpi3mr/mpi3mr_fw.c
@@ -2878,11 +2878,8 @@ void mpi3mr_start_watchdog(struct mpi3mr_ioc *mrioc)
  		return;

  	INIT_DELAYED_WORK(&mrioc->watchdog_work, mpi3mr_watchdog_work);
-	snprintf(mrioc->watchdog_work_q_name,
-	    sizeof(mrioc->watchdog_work_q_name), "watchdog_%s%d", mrioc->name,
-	    mrioc->id);
  	mrioc->watchdog_work_q = alloc_ordered_workqueue(
-		"%s", WQ_MEM_RECLAIM, mrioc->watchdog_work_q_name);
+		"watchdog_%s%d", WQ_MEM_RECLAIM, mrioc->name, mrioc->id);
  	if (!mrioc->watchdog_work_q) {
  		ioc_err(mrioc, "%s: failed (line=%d)\n", __func__, __LINE__);
  		return;

Thanks,

Bart.