| Message ID | 20220513062745.3245-1-mingzhe.zou@easystack.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | scsi: target: fixup incorrect use of 'cpumask_t' | expand |
On 5/13/22 1:27 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 (!alloc_cpumask_var(&mask, GFP_KERNEL)) > return -ENOMEM; > ... use 'mask' here as a ... > free_cpumask_var(mask); > ``` > Add a: Fixes: d72d827f2f26 ("scsi: target: Add iscsi/cpus_allowed_list in configfs") to make it easier for people to pick up. > 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 | 20 ++++++++---- > 2 files changed, 36 insertions(+), 16 deletions(-) > > diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c > index 6fe6a6bab3f4..3f9f5b8879fe 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 (!alloc_cpumask_var(&conn_allowed_cpumask, GFP_KERNEL)) { Do a zalloc_cpumask_var to make sure it's all initialized. > + 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..ae9319934cd0 100644 > --- a/drivers/target/iscsi/iscsi_target_configfs.c > +++ b/drivers/target/iscsi/iscsi_target_configfs.c > @@ -1139,20 +1139,28 @@ static ssize_t lio_target_wwn_cpus_allowed_list_store( > { > int ret; > char *orig; > - cpumask_t new_allowed_cpumask; > + cpumask_var_t new_allowed_cpumask; > + > + if (!alloc_cpumask_var(&new_allowed_cpumask, GFP_KERNEL)) > + return -ENOMEM; You can do zalloc_cpumask_var and then drop the clear call below. > > orig = kstrdup(page, GFP_KERNEL); > - if (!orig) > + if (!orig) { > + free_cpumask_var(new_allowed_cpumask); > return -ENOMEM; Just add a goto at the bottom before free_cpumask_var then have this and the other failure path use it. Make sure to also fix up the return code (set count to -ENOMEM/ret or use ret at the bottom and set count to ret). > + } > > - cpumask_clear(&new_allowed_cpumask); > - ret = cpulist_parse(orig, &new_allowed_cpumask); > + cpumask_clear(new_allowed_cpumask); > + ret = cpulist_parse(orig, new_allowed_cpumask); > > kfree(orig); > - if (ret != 0) > + if (ret != 0) { > + free_cpumask_var(new_allowed_cpumask); > return ret; > + } > > - cpumask_copy(iscsit_global->allowed_cpumask, &new_allowed_cpumask); > + cpumask_copy(iscsit_global->allowed_cpumask, new_allowed_cpumask); > + free_cpumask_var(new_allowed_cpumask); > return count; > } >
diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c index 6fe6a6bab3f4..3f9f5b8879fe 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 (!alloc_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..ae9319934cd0 100644 --- a/drivers/target/iscsi/iscsi_target_configfs.c +++ b/drivers/target/iscsi/iscsi_target_configfs.c @@ -1139,20 +1139,28 @@ static ssize_t lio_target_wwn_cpus_allowed_list_store( { int ret; char *orig; - cpumask_t new_allowed_cpumask; + cpumask_var_t new_allowed_cpumask; + + if (!alloc_cpumask_var(&new_allowed_cpumask, GFP_KERNEL)) + return -ENOMEM; orig = kstrdup(page, GFP_KERNEL); - if (!orig) + if (!orig) { + free_cpumask_var(new_allowed_cpumask); return -ENOMEM; + } - cpumask_clear(&new_allowed_cpumask); - ret = cpulist_parse(orig, &new_allowed_cpumask); + cpumask_clear(new_allowed_cpumask); + ret = cpulist_parse(orig, new_allowed_cpumask); kfree(orig); - if (ret != 0) + if (ret != 0) { + free_cpumask_var(new_allowed_cpumask); return ret; + } - cpumask_copy(iscsit_global->allowed_cpumask, &new_allowed_cpumask); + cpumask_copy(iscsit_global->allowed_cpumask, new_allowed_cpumask); + free_cpumask_var(new_allowed_cpumask); return count; }