diff mbox series

[v2] scsi: target: fixup incorrect use of 'cpumask_t'

Message ID 20220516054721.1548-1-mingzhe.zou@easystack.cn (mailing list archive)
State Accepted
Commit 525f447f88b14a64424642de670f77424e067d5b
Headers show
Series [v2] scsi: target: fixup incorrect use of 'cpumask_t' | expand

Commit Message

Zou Mingzhe May 16, 2022, 5:47 a.m. UTC 
From: mingzhe <mingzhe.zou@easystack.cn>

In commit d72d827f2f26, I used 'cpumask_t' incorrectly.
```
void iscsit_thread_get_cpumask(struct iscsi_conn *conn)
{
        int ord, cpu;
        cpumask_t conn_allowed_cpumask;
        ......
}

static ssize_t lio_target_wwn_cpus_allowed_list_store(
               struct config_item *item, const char *page, size_t count)
{
        int ret;
        char *orig;
        cpumask_t new_allowed_cpumask;
        ......
}
```

So, that the correct pattern should be as follows:
```
cpumask_var_t mask;

if (!zalloc_cpumask_var(&mask, GFP_KERNEL))
        return -ENOMEM;
... use 'mask' here as a  ...
free_cpumask_var(mask);
```

Fixes: d72d827f2f26 ("scsi: target: Add iscsi/cpus_allowed_list in configfs")
Reported-by: Test Bot <zgrieee@gmail.com>
Signed-off-by: mingzhe <mingzhe.zou@easystack.cn>
---
 drivers/target/iscsi/iscsi_target.c          | 32 ++++++++++++++------
 drivers/target/iscsi/iscsi_target_configfs.c | 24 +++++++++------
 2 files changed, 36 insertions(+), 20 deletions(-)

Comments

Mike Christie May 18, 2022, 12:33 a.m. UTC | #1 
On 5/16/22 12:47 AM, mingzhe.zou@easystack.cn wrote:
> From: mingzhe <mingzhe.zou@easystack.cn>
> 
> In commit d72d827f2f26, I used 'cpumask_t' incorrectly.
> ```
> void iscsit_thread_get_cpumask(struct iscsi_conn *conn)
> {
>         int ord, cpu;
>         cpumask_t conn_allowed_cpumask;
>         ......
> }
> 
> static ssize_t lio_target_wwn_cpus_allowed_list_store(
>                struct config_item *item, const char *page, size_t count)
> {
>         int ret;
>         char *orig;
>         cpumask_t new_allowed_cpumask;
>         ......
> }
> ```
> 
> So, that the correct pattern should be as follows:
> ```
> cpumask_var_t mask;
> 
> if (!zalloc_cpumask_var(&mask, GFP_KERNEL))
>         return -ENOMEM;
> ... use 'mask' here as a  ...
> free_cpumask_var(mask);
> ```
> 
> Fixes: d72d827f2f26 ("scsi: target: Add iscsi/cpus_allowed_list in configfs")
> Reported-by: Test Bot <zgrieee@gmail.com>
> Signed-off-by: mingzhe <mingzhe.zou@easystack.cn>
>

Reviewed-by: Mike Christie <michael.christie@oracle.com>
Martin K. Petersen May 18, 2022, 1:55 a.m. UTC | #2 
On Mon, 16 May 2022 13:47:21 +0800, mingzhe.zou@easystack.cn wrote:

> From: mingzhe <mingzhe.zou@easystack.cn>
> 
> In commit d72d827f2f26, I used 'cpumask_t' incorrectly.
> ```
> void iscsit_thread_get_cpumask(struct iscsi_conn *conn)
> {
>         int ord, cpu;
>         cpumask_t conn_allowed_cpumask;
>         ......
> }
> 
> [...]

Applied to 5.18/scsi-fixes, thanks!

[1/1] scsi: target: fixup incorrect use of 'cpumask_t'
      https://git.kernel.org/mkp/scsi/c/525f447f88b1
diff mbox series

Patch

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 6fe6a6bab3f4..ddf6c2a7212b 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -3596,10 +3596,7 @@  static int iscsit_send_reject(
 void iscsit_thread_get_cpumask(struct iscsi_conn *conn)
 {
 	int ord, cpu;
-	cpumask_t conn_allowed_cpumask;
-
-	cpumask_and(&conn_allowed_cpumask, iscsit_global->allowed_cpumask,
-		    cpu_online_mask);
+	cpumask_var_t conn_allowed_cpumask;
 
 	/*
 	 * bitmap_id is assigned from iscsit_global->ts_bitmap from
@@ -3609,13 +3606,28 @@  void iscsit_thread_get_cpumask(struct iscsi_conn *conn)
 	 * iSCSI connection's RX/TX threads will be scheduled to
 	 * execute upon.
 	 */
-	cpumask_clear(conn->conn_cpumask);
-	ord = conn->bitmap_id % cpumask_weight(&conn_allowed_cpumask);
-	for_each_cpu(cpu, &conn_allowed_cpumask) {
-		if (ord-- == 0) {
-			cpumask_set_cpu(cpu, conn->conn_cpumask);
-			return;
+	if (!zalloc_cpumask_var(&conn_allowed_cpumask, GFP_KERNEL)) {
+		ord = conn->bitmap_id % cpumask_weight(cpu_online_mask);
+		for_each_online_cpu(cpu) {
+			if (ord-- == 0) {
+				cpumask_set_cpu(cpu, conn->conn_cpumask);
+				return;
+			}
+		}
+	} else {
+		cpumask_and(conn_allowed_cpumask, iscsit_global->allowed_cpumask,
+			cpu_online_mask);
+
+		cpumask_clear(conn->conn_cpumask);
+		ord = conn->bitmap_id % cpumask_weight(conn_allowed_cpumask);
+		for_each_cpu(cpu, conn_allowed_cpumask) {
+			if (ord-- == 0) {
+				cpumask_set_cpu(cpu, conn->conn_cpumask);
+				free_cpumask_var(conn_allowed_cpumask);
+				return;
+			}
 		}
+		free_cpumask_var(conn_allowed_cpumask);
 	}
 	/*
 	 * This should never be reached..
diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c
index 0cedcfe207b5..57b4fd56d92a 100644
--- a/drivers/target/iscsi/iscsi_target_configfs.c
+++ b/drivers/target/iscsi/iscsi_target_configfs.c
@@ -1137,23 +1137,27 @@  static ssize_t lio_target_wwn_cpus_allowed_list_show(
 static ssize_t lio_target_wwn_cpus_allowed_list_store(
 		struct config_item *item, const char *page, size_t count)
 {
-	int ret;
+	int ret = -ENOMEM;
 	char *orig;
-	cpumask_t new_allowed_cpumask;
+	cpumask_var_t new_allowed_cpumask;
+
+	if (!zalloc_cpumask_var(&new_allowed_cpumask, GFP_KERNEL))
+		goto out;
 
 	orig = kstrdup(page, GFP_KERNEL);
 	if (!orig)
-		return -ENOMEM;
+		goto out_free_cpumask;
 
-	cpumask_clear(&new_allowed_cpumask);
-	ret = cpulist_parse(orig, &new_allowed_cpumask);
+	ret = cpulist_parse(orig, new_allowed_cpumask);
+	if (!ret)
+		cpumask_copy(iscsit_global->allowed_cpumask,
+			     new_allowed_cpumask);
 
 	kfree(orig);
-	if (ret != 0)
-		return ret;
-
-	cpumask_copy(iscsit_global->allowed_cpumask, &new_allowed_cpumask);
-	return count;
+out_free_cpumask:
+	free_cpumask_var(new_allowed_cpumask);
+out:
+	return ret ? ret : count;
 }
 
 CONFIGFS_ATTR(lio_target_wwn_, cpus_allowed_list);