diff mbox series

target: add iscsi/cpus_allowed_list in configfs

Message ID 20220125083821.18225-1-mingzhe.zou@easystack.cn
State New
Headers show
Series target: add iscsi/cpus_allowed_list in configfs | expand

Commit Message

Zou Mingzhe Jan. 25, 2022, 8:38 a.m. UTC
From: Mingzhe Zou <mingzhe.zou@easystack.cn>

The RX/TX threads for iSCSI connection can be scheduled to
any online cpus, and will not be rescheduled.

If bind other heavy load threads with iSCSI connection
RX/TX thread to the same cpu, the iSCSI performance will
be worse.

This patch add iscsi/cpus_allowed_list in configfs. The
available cpus set of iSCSI connection RX/TX threads is
allowed_cpus & online_cpus. If it is modified, all RX/TX
threads will be rescheduled.

Signed-off-by: Mingzhe Zou <mingzhe.zou@easystack.cn>
---
 drivers/target/iscsi/iscsi_target.c          | 21 ++++++++++--
 drivers/target/iscsi/iscsi_target.h          | 17 ++++++++++
 drivers/target/iscsi/iscsi_target_configfs.c | 34 ++++++++++++++++++++
 drivers/target/iscsi/iscsi_target_login.c    |  7 ++++
 include/target/iscsi/iscsi_target_core.h     |  1 +
 5 files changed, 78 insertions(+), 2 deletions(-)

Comments

Mike Christie Feb. 8, 2022, 5:50 p.m. UTC | #1
On 1/25/22 2:38 AM, mingzhe.zou@easystack.cn wrote:
> From: Mingzhe Zou <mingzhe.zou@easystack.cn>
> 
> The RX/TX threads for iSCSI connection can be scheduled to
> any online cpus, and will not be rescheduled.
> 
> If bind other heavy load threads with iSCSI connection
> RX/TX thread to the same cpu, the iSCSI performance will
> be worse.
> 
> This patch add iscsi/cpus_allowed_list in configfs. The
> available cpus set of iSCSI connection RX/TX threads is
> allowed_cpus & online_cpus. If it is modified, all RX/TX
> threads will be rescheduled.
> 
> Signed-off-by: Mingzhe Zou <mingzhe.zou@easystack.cn>
> ---
>  drivers/target/iscsi/iscsi_target.c          | 21 ++++++++++--
>  drivers/target/iscsi/iscsi_target.h          | 17 ++++++++++
>  drivers/target/iscsi/iscsi_target_configfs.c | 34 ++++++++++++++++++++
>  drivers/target/iscsi/iscsi_target_login.c    |  7 ++++
>  include/target/iscsi/iscsi_target_core.h     |  1 +
>  5 files changed, 78 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
> index 2c54c5d8412d..a18d3fc3cfd1 100644
> --- a/drivers/target/iscsi/iscsi_target.c
> +++ b/drivers/target/iscsi/iscsi_target.c
> @@ -693,6 +693,11 @@ static int __init iscsi_target_init_module(void)
>  	mutex_init(&auth_id_lock);
>  	idr_init(&tiqn_idr);
>  
> +	/*
> +	 * allow all cpus run iscsi_ttx and iscsi_trx

I would just drop the comment. The "setall" in the function name
is pretty clear already.


> +	 */
> +	cpumask_setall(&__iscsi_allowed_cpumask);
> +
>  	ret = target_register_template(&iscsi_ops);
>  	if (ret)
>  		goto out;
> @@ -3587,6 +3592,15 @@ static int iscsit_send_reject(
>  void iscsit_thread_get_cpumask(struct iscsi_conn *conn)
>  {
>  	int ord, cpu;
> +	cpumask_t conn_allowed_cpumask;
> +
> +	/*
> +	 * The available cpus set of iSCSI connection's RX/TX threads
> +	 */

Probably can drop the comment too since the variable names make it
known what we are doing.


> +	cpumask_and(&conn_allowed_cpumask,
> +		&__iscsi_allowed_cpumask,
> +		cpu_online_mask);

The formatting needs some fix ups. I think __iscsi_allowed_cpumask can fit on the
line above it and then cpu_online_mask should be moved over a couple spaces to
align with the opening "(".


> +
>  	/*
>  	 * bitmap_id is assigned from iscsit_global->ts_bitmap from
>  	 * within iscsit_start_kthreads()
> @@ -3595,8 +3609,9 @@ void iscsit_thread_get_cpumask(struct iscsi_conn *conn)
>  	 * iSCSI connection's RX/TX threads will be scheduled to
>  	 * execute upon.
>  	 */
> -	ord = conn->bitmap_id % cpumask_weight(cpu_online_mask);
> -	for_each_online_cpu(cpu) {
> +	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;
> @@ -3821,6 +3836,7 @@ int iscsi_target_tx_thread(void *arg)
>  		 * Ensure that both TX and RX per connection kthreads
>  		 * are scheduled to run on the same CPU.
>  		 */
> +		iscsit_thread_reschedule(conn);


If we have multiple sessions to the same portal, could we end up racing
where session0's tx/rx threads call iscsit_thread_reschedule and
iscsit_thread_check_cpumask at the same time as session1's threads and
they end up taking the same cpus. They both could be running
iscsit_thread_get_cpumask at the same time, see he same masks values
and in the for_each_cpu loop think the same cpu is free.


>  		iscsit_thread_check_cpumask(conn, current, 1);
>  
>  		wait_event_interruptible(conn->queues_wq,
> @@ -3966,6 +3982,7 @@ static void iscsit_get_rx_pdu(struct iscsi_conn *conn)
>  		 * Ensure that both TX and RX per connection kthreads
>  		 * are scheduled to run on the same CPU.
>  		 */
> +		iscsit_thread_reschedule(conn);
>  		iscsit_thread_check_cpumask(conn, current, 0);
>  
>  		memset(&iov, 0, sizeof(struct kvec));
> diff --git a/drivers/target/iscsi/iscsi_target.h b/drivers/target/iscsi/iscsi_target.h
> index b35a96ded9c1..cb97a316d76d 100644
> --- a/drivers/target/iscsi/iscsi_target.h
> +++ b/drivers/target/iscsi/iscsi_target.h
> @@ -57,4 +57,21 @@ extern struct kmem_cache *lio_r2t_cache;
>  extern struct ida sess_ida;
>  extern struct mutex auth_id_lock;
>  
> +extern cpumask_t __iscsi_allowed_cpumask;

I would rename to:

iscsit_allowed_cpumask


> +
> +static inline void iscsit_thread_reschedule(struct iscsi_conn *conn)
> +{
> +	/*
> +	 * If __iscsi_allowed_cpumask modified, reschedule iSCSI connection's
> +	 * RX/TX threads update conn->allowed_cpumask.
> +	 */
> +	if (!cpumask_equal(&__iscsi_allowed_cpumask, conn->allowed_cpumask)) {
> +		iscsit_thread_get_cpumask(conn);
> +		conn->conn_tx_reset_cpumask = 1;
> +		conn->conn_rx_reset_cpumask = 1;
> +		cpumask_copy(conn->allowed_cpumask,
> +			&__iscsi_allowed_cpumask);

Fix up formatting like above.

> +	}
> +}
> +
>  #endif   /*** ISCSI_TARGET_H ***/
> diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c
> index 2a9de24a8bbe..dc12b1695838 100644
> --- a/drivers/target/iscsi/iscsi_target_configfs.c
> +++ b/drivers/target/iscsi/iscsi_target_configfs.c
> @@ -1127,8 +1127,42 @@ static ssize_t lio_target_wwn_lio_version_show(struct config_item *item,
>  
>  CONFIGFS_ATTR_RO(lio_target_wwn_, lio_version);
>  
> +cpumask_t __iscsi_allowed_cpumask;

Maybe better to put this in iscsi_target.c with the other vars like
it.

> +
> +static ssize_t lio_target_wwn_cpus_allowed_list_show(
> +		struct config_item *item, char *page)
> +{
> +	return sprintf(page, "%*pbl\n",
> +		cpumask_pr_args(&__iscsi_allowed_cpumask));

Formatting like above.

> +}
> +
> +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;
> +
> +	orig = kstrdup(page, GFP_KERNEL);
> +	if (!orig)
> +		return -ENOMEM;
> +
> +	cpumask_clear(&new_allowed_cpumask);
> +	ret = cpulist_parse(orig, &new_allowed_cpumask);

Are you supposed to do a zalloc_cpumask_var/free_cpumask_var before
using new_allowed_cpumask? It looks like other callers are doing it.

> +
> +	kfree(orig);
> +	if (ret != 0)
> +		return ret;
> +
> +	cpumask_copy(&__iscsi_allowed_cpumask, &new_allowed_cpumask);
> +	return count;
> +}
> +
> +CONFIGFS_ATTR(lio_target_wwn_, cpus_allowed_list);
> +
>  static struct configfs_attribute *lio_target_wwn_attrs[] = {
>  	&lio_target_wwn_attr_lio_version,
> +	&lio_target_wwn_attr_cpus_allowed_list,
>  	NULL,
>  };
>  
> diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c
> index 1a9c50401bdb..910f35e4648a 100644
> --- a/drivers/target/iscsi/iscsi_target_login.c
> +++ b/drivers/target/iscsi/iscsi_target_login.c
> @@ -1129,8 +1129,15 @@ static struct iscsi_conn *iscsit_alloc_conn(struct iscsi_np *np)
>  		goto free_conn_ops;
>  	}
>  
> +	if (!zalloc_cpumask_var(&conn->allowed_cpumask, GFP_KERNEL)) {
> +		pr_err("Unable to allocate conn->allowed_cpumask\n");
> +		goto free_conn_cpumask;
> +	}

I think in iscsit_free_conn you need a:

free_cpumask_var(conn->allowed_cpumask);

to go with this allocation.


> +
>  	return conn;
>  
> +free_conn_cpumask:
> +	free_cpumask_var(conn->allowed_cpumask);

I think you wanted conn->conn_cpumask here.

>  free_conn_ops:
>  	kfree(conn->conn_ops);
>  put_transport:
> diff --git a/include/target/iscsi/iscsi_target_core.h b/include/target/iscsi/iscsi_target_core.h
> index 1eccb2ac7d02..c5e9cad187cf 100644
> --- a/include/target/iscsi/iscsi_target_core.h
> +++ b/include/target/iscsi/iscsi_target_core.h
> @@ -580,6 +580,7 @@ struct iscsi_conn {
>  	struct ahash_request	*conn_tx_hash;
>  	/* Used for scheduling TX and RX connection kthreads */
>  	cpumask_var_t		conn_cpumask;
> +	cpumask_var_t		allowed_cpumask;
>  	unsigned int		conn_rx_reset_cpumask:1;
>  	unsigned int		conn_tx_reset_cpumask:1;
>  	/* list_head of struct iscsi_cmd for this connection */
Zou Mingzhe Feb. 9, 2022, 11:48 a.m. UTC | #2
在 2022/2/9 上午1:50, Mike Christie 写道:
> On 1/25/22 2:38 AM, mingzhe.zou@easystack.cn wrote:
>> From: Mingzhe Zou <mingzhe.zou@easystack.cn>
>>
>> The RX/TX threads for iSCSI connection can be scheduled to
>> any online cpus, and will not be rescheduled.
>>
>> If bind other heavy load threads with iSCSI connection
>> RX/TX thread to the same cpu, the iSCSI performance will
>> be worse.
>>
>> This patch add iscsi/cpus_allowed_list in configfs. The
>> available cpus set of iSCSI connection RX/TX threads is
>> allowed_cpus & online_cpus. If it is modified, all RX/TX
>> threads will be rescheduled.
>>
>> Signed-off-by: Mingzhe Zou <mingzhe.zou@easystack.cn>
>> ---
>>   drivers/target/iscsi/iscsi_target.c          | 21 ++++++++++--
>>   drivers/target/iscsi/iscsi_target.h          | 17 ++++++++++
>>   drivers/target/iscsi/iscsi_target_configfs.c | 34 ++++++++++++++++++++
>>   drivers/target/iscsi/iscsi_target_login.c    |  7 ++++
>>   include/target/iscsi/iscsi_target_core.h     |  1 +
>>   5 files changed, 78 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
>> index 2c54c5d8412d..a18d3fc3cfd1 100644
>> --- a/drivers/target/iscsi/iscsi_target.c
>> +++ b/drivers/target/iscsi/iscsi_target.c
>> @@ -693,6 +693,11 @@ static int __init iscsi_target_init_module(void)
>>   	mutex_init(&auth_id_lock);
>>   	idr_init(&tiqn_idr);
>>   
>> +	/*
>> +	 * allow all cpus run iscsi_ttx and iscsi_trx
> I would just drop the comment. The "setall" in the function name
> is pretty clear already.
I will remove it.
>
>
>> +	 */
>> +	cpumask_setall(&__iscsi_allowed_cpumask);
>> +
>>   	ret = target_register_template(&iscsi_ops);
>>   	if (ret)
>>   		goto out;
>> @@ -3587,6 +3592,15 @@ static int iscsit_send_reject(
>>   void iscsit_thread_get_cpumask(struct iscsi_conn *conn)
>>   {
>>   	int ord, cpu;
>> +	cpumask_t conn_allowed_cpumask;
>> +
>> +	/*
>> +	 * The available cpus set of iSCSI connection's RX/TX threads
>> +	 */
> Probably can drop the comment too since the variable names make it
> known what we are doing.
I will remove it.
>
>
>> +	cpumask_and(&conn_allowed_cpumask,
>> +		&__iscsi_allowed_cpumask,
>> +		cpu_online_mask);
> The formatting needs some fix ups. I think __iscsi_allowed_cpumask can fit on the
> line above it and then cpu_online_mask should be moved over a couple spaces to
> align with the opening "(".

I will reformat.

>
>
>> +
>>   	/*
>>   	 * bitmap_id is assigned from iscsit_global->ts_bitmap from
>>   	 * within iscsit_start_kthreads()
>> @@ -3595,8 +3609,9 @@ void iscsit_thread_get_cpumask(struct iscsi_conn *conn)
>>   	 * iSCSI connection's RX/TX threads will be scheduled to
>>   	 * execute upon.
>>   	 */
>> -	ord = conn->bitmap_id % cpumask_weight(cpu_online_mask);
>> -	for_each_online_cpu(cpu) {
>> +	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;
>> @@ -3821,6 +3836,7 @@ int iscsi_target_tx_thread(void *arg)
>>   		 * Ensure that both TX and RX per connection kthreads
>>   		 * are scheduled to run on the same CPU.
>>   		 */
>> +		iscsit_thread_reschedule(conn);
>
> If we have multiple sessions to the same portal, could we end up racing
> where session0's tx/rx threads call iscsit_thread_reschedule and
> iscsit_thread_check_cpumask at the same time as session1's threads and
> they end up taking the same cpus. They both could be running
> iscsit_thread_get_cpumask at the same time, see he same masks values
> and in the for_each_cpu loop think the same cpu is free.

Simply, this patch just adds a config item to skip some CPUs that are 
bound to other threads,

not to change the calculation method of cpumask.

When the cpus_allowed_list in sysfs is modified, the tx/rx threads of 
all sessions will clear current

cpumask and call iscsit_thread_reschedule and 
iscsit_thread_check_cpumask to get a new value.

Because each session is allocated a unique bitmap_id in advance, then 
calculate the cpumask

via bitmap_id. So the tx/rx threads of different sessions will be 
scheduled to different CPUs (when

the number of CPUs is sufficient).

>
>
>>   		iscsit_thread_check_cpumask(conn, current, 1);
>>   
>>   		wait_event_interruptible(conn->queues_wq,
>> @@ -3966,6 +3982,7 @@ static void iscsit_get_rx_pdu(struct iscsi_conn *conn)
>>   		 * Ensure that both TX and RX per connection kthreads
>>   		 * are scheduled to run on the same CPU.
>>   		 */
>> +		iscsit_thread_reschedule(conn);
>>   		iscsit_thread_check_cpumask(conn, current, 0);
>>   
>>   		memset(&iov, 0, sizeof(struct kvec));
>> diff --git a/drivers/target/iscsi/iscsi_target.h b/drivers/target/iscsi/iscsi_target.h
>> index b35a96ded9c1..cb97a316d76d 100644
>> --- a/drivers/target/iscsi/iscsi_target.h
>> +++ b/drivers/target/iscsi/iscsi_target.h
>> @@ -57,4 +57,21 @@ extern struct kmem_cache *lio_r2t_cache;
>>   extern struct ida sess_ida;
>>   extern struct mutex auth_id_lock;
>>   
>> +extern cpumask_t __iscsi_allowed_cpumask;
> I would rename to:
>
> iscsit_allowed_cpumask
>
I will rename to iscsit_allowed_cpumas.
>> +
>> +static inline void iscsit_thread_reschedule(struct iscsi_conn *conn)
>> +{
>> +	/*
>> +	 * If __iscsi_allowed_cpumask modified, reschedule iSCSI connection's
>> +	 * RX/TX threads update conn->allowed_cpumask.
>> +	 */
>> +	if (!cpumask_equal(&__iscsi_allowed_cpumask, conn->allowed_cpumask)) {
>> +		iscsit_thread_get_cpumask(conn);
>> +		conn->conn_tx_reset_cpumask = 1;
>> +		conn->conn_rx_reset_cpumask = 1;
>> +		cpumask_copy(conn->allowed_cpumask,
>> +			&__iscsi_allowed_cpumask);
> Fix up formatting like above.
I will reformat.
>
>> +	}
>> +}
>> +
>>   #endif   /*** ISCSI_TARGET_H ***/
>> diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c
>> index 2a9de24a8bbe..dc12b1695838 100644
>> --- a/drivers/target/iscsi/iscsi_target_configfs.c
>> +++ b/drivers/target/iscsi/iscsi_target_configfs.c
>> @@ -1127,8 +1127,42 @@ static ssize_t lio_target_wwn_lio_version_show(struct config_item *item,
>>   
>>   CONFIGFS_ATTR_RO(lio_target_wwn_, lio_version);
>>   
>> +cpumask_t __iscsi_allowed_cpumask;
> Maybe better to put this in iscsi_target.c with the other vars like
> it.

Originally I wanted to put it in struct iscsit_global and use it in 
iscsit_thread_check_cpumask.

However iscsit_thread_check_cpumask is also called in cxgbit_target.c. I 
don't know if I also

need to modify cxgbit at the same time, and I only modified two calls in 
iscsi_target.c. I would

like to know how to handle in cxgbit_target.c?

I want to move 'static inline void iscsit_thread_check_cpumask' from 
iscsi_target_core.h to

iscsi_target.c and EXPORT_SYMBOL(iscsit_thread_check_cpumask). Do you 
agree it?

>
>> +
>> +static ssize_t lio_target_wwn_cpus_allowed_list_show(
>> +		struct config_item *item, char *page)
>> +{
>> +	return sprintf(page, "%*pbl\n",
>> +		cpumask_pr_args(&__iscsi_allowed_cpumask));
> Formatting like above.
I will reformat.
>
>> +}
>> +
>> +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;
>> +
>> +	orig = kstrdup(page, GFP_KERNEL);
>> +	if (!orig)
>> +		return -ENOMEM;
>> +
>> +	cpumask_clear(&new_allowed_cpumask);
>> +	ret = cpulist_parse(orig, &new_allowed_cpumask);
> Are you supposed to do a  before
> using new_allowed_cpumask? It looks like other callers are doing it.

Other callers used cpumask_var_t, but the new_allowed_cpumask is cpumask_t.

I think just call cpumask_clear(&new_allowed_cpumask).


typedef struct cpumask { DECLARE_BITMAP(bits, NR_CPUS); } cpumask_t;

typedef struct cpumask cpumask_var_t[1];

>
>> +
>> +	kfree(orig);
>> +	if (ret != 0)
>> +		return ret;
>> +
>> +	cpumask_copy(&__iscsi_allowed_cpumask, &new_allowed_cpumask);
>> +	return count;
>> +}
>> +
>> +CONFIGFS_ATTR(lio_target_wwn_, cpus_allowed_list);
>> +
>>   static struct configfs_attribute *lio_target_wwn_attrs[] = {
>>   	&lio_target_wwn_attr_lio_version,
>> +	&lio_target_wwn_attr_cpus_allowed_list,
>>   	NULL,
>>   };
>>   
>> diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c
>> index 1a9c50401bdb..910f35e4648a 100644
>> --- a/drivers/target/iscsi/iscsi_target_login.c
>> +++ b/drivers/target/iscsi/iscsi_target_login.c
>> @@ -1129,8 +1129,15 @@ static struct iscsi_conn *iscsit_alloc_conn(struct iscsi_np *np)
>>   		goto free_conn_ops;
>>   	}
>>   
>> +	if (!zalloc_cpumask_var(&conn->allowed_cpumask, GFP_KERNEL)) {
>> +		pr_err("Unable to allocate conn->allowed_cpumask\n");
>> +		goto free_conn_cpumask;
>> +	}
> I think in iscsit_free_conn you need a:
>
> free_cpumask_var(conn->allowed_cpumask);
>
> to go with this allocation.
This is bug. I will fix up in v2.
>
>> +
>>   	return conn;
>>   
>> +free_conn_cpumask:
>> +	free_cpumask_var(conn->allowed_cpumask);
> I think you wanted conn->conn_cpumask here.
This is bug. I will fix up in v2.
>>   free_conn_ops:
>>   	kfree(conn->conn_ops);
>>   put_transport:
>> diff --git a/include/target/iscsi/iscsi_target_core.h b/include/target/iscsi/iscsi_target_core.h
>> index 1eccb2ac7d02..c5e9cad187cf 100644
>> --- a/include/target/iscsi/iscsi_target_core.h
>> +++ b/include/target/iscsi/iscsi_target_core.h
>> @@ -580,6 +580,7 @@ struct iscsi_conn {
>>   	struct ahash_request	*conn_tx_hash;
>>   	/* Used for scheduling TX and RX connection kthreads */
>>   	cpumask_var_t		conn_cpumask;
>> +	cpumask_var_t		allowed_cpumask;
>>   	unsigned int		conn_rx_reset_cpumask:1;
>>   	unsigned int		conn_tx_reset_cpumask:1;
>>   	/* list_head of struct iscsi_cmd for this connection */
>
Mike Christie Feb. 16, 2022, 5:13 p.m. UTC | #3
On 2/9/22 5:48 AM, Zou Mingzhe wrote:
>>>   +cpumask_t __iscsi_allowed_cpumask;
>> Maybe better to put this in iscsi_target.c with the other vars like
>> it.
> 
> Originally I wanted to put it in struct iscsit_global and use it in iscsit_thread_check_cpumask.
> 
> However iscsit_thread_check_cpumask is also called in cxgbit_target.c. I don't know if I also
> 
> need to modify cxgbit at the same time, and I only modified two calls in iscsi_target.c. I would
> 

The new configfs file shows up for software iscsi and cxgb right? If so I think you need to
modify both, or you end up with a file that returns success but doesn't do anything and it's
confusing to users. It's also a simple change to cxgb.


> like to know how to handle in cxgbit_target.c?
> 
> I want to move 'static inline void iscsit_thread_check_cpumask' from iscsi_target_core.h to
> 
> iscsi_target.c and EXPORT_SYMBOL(iscsit_thread_check_cpumask). Do you agree it?
> 

I think that is ok.
diff mbox series

Patch

diff --git a/drivers/target/iscsi/iscsi_target.c b/drivers/target/iscsi/iscsi_target.c
index 2c54c5d8412d..a18d3fc3cfd1 100644
--- a/drivers/target/iscsi/iscsi_target.c
+++ b/drivers/target/iscsi/iscsi_target.c
@@ -693,6 +693,11 @@  static int __init iscsi_target_init_module(void)
 	mutex_init(&auth_id_lock);
 	idr_init(&tiqn_idr);
 
+	/*
+	 * allow all cpus run iscsi_ttx and iscsi_trx
+	 */
+	cpumask_setall(&__iscsi_allowed_cpumask);
+
 	ret = target_register_template(&iscsi_ops);
 	if (ret)
 		goto out;
@@ -3587,6 +3592,15 @@  static int iscsit_send_reject(
 void iscsit_thread_get_cpumask(struct iscsi_conn *conn)
 {
 	int ord, cpu;
+	cpumask_t conn_allowed_cpumask;
+
+	/*
+	 * The available cpus set of iSCSI connection's RX/TX threads
+	 */
+	cpumask_and(&conn_allowed_cpumask,
+		&__iscsi_allowed_cpumask,
+		cpu_online_mask);
+
 	/*
 	 * bitmap_id is assigned from iscsit_global->ts_bitmap from
 	 * within iscsit_start_kthreads()
@@ -3595,8 +3609,9 @@  void iscsit_thread_get_cpumask(struct iscsi_conn *conn)
 	 * iSCSI connection's RX/TX threads will be scheduled to
 	 * execute upon.
 	 */
-	ord = conn->bitmap_id % cpumask_weight(cpu_online_mask);
-	for_each_online_cpu(cpu) {
+	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;
@@ -3821,6 +3836,7 @@  int iscsi_target_tx_thread(void *arg)
 		 * Ensure that both TX and RX per connection kthreads
 		 * are scheduled to run on the same CPU.
 		 */
+		iscsit_thread_reschedule(conn);
 		iscsit_thread_check_cpumask(conn, current, 1);
 
 		wait_event_interruptible(conn->queues_wq,
@@ -3966,6 +3982,7 @@  static void iscsit_get_rx_pdu(struct iscsi_conn *conn)
 		 * Ensure that both TX and RX per connection kthreads
 		 * are scheduled to run on the same CPU.
 		 */
+		iscsit_thread_reschedule(conn);
 		iscsit_thread_check_cpumask(conn, current, 0);
 
 		memset(&iov, 0, sizeof(struct kvec));
diff --git a/drivers/target/iscsi/iscsi_target.h b/drivers/target/iscsi/iscsi_target.h
index b35a96ded9c1..cb97a316d76d 100644
--- a/drivers/target/iscsi/iscsi_target.h
+++ b/drivers/target/iscsi/iscsi_target.h
@@ -57,4 +57,21 @@  extern struct kmem_cache *lio_r2t_cache;
 extern struct ida sess_ida;
 extern struct mutex auth_id_lock;
 
+extern cpumask_t __iscsi_allowed_cpumask;
+
+static inline void iscsit_thread_reschedule(struct iscsi_conn *conn)
+{
+	/*
+	 * If __iscsi_allowed_cpumask modified, reschedule iSCSI connection's
+	 * RX/TX threads update conn->allowed_cpumask.
+	 */
+	if (!cpumask_equal(&__iscsi_allowed_cpumask, conn->allowed_cpumask)) {
+		iscsit_thread_get_cpumask(conn);
+		conn->conn_tx_reset_cpumask = 1;
+		conn->conn_rx_reset_cpumask = 1;
+		cpumask_copy(conn->allowed_cpumask,
+			&__iscsi_allowed_cpumask);
+	}
+}
+
 #endif   /*** ISCSI_TARGET_H ***/
diff --git a/drivers/target/iscsi/iscsi_target_configfs.c b/drivers/target/iscsi/iscsi_target_configfs.c
index 2a9de24a8bbe..dc12b1695838 100644
--- a/drivers/target/iscsi/iscsi_target_configfs.c
+++ b/drivers/target/iscsi/iscsi_target_configfs.c
@@ -1127,8 +1127,42 @@  static ssize_t lio_target_wwn_lio_version_show(struct config_item *item,
 
 CONFIGFS_ATTR_RO(lio_target_wwn_, lio_version);
 
+cpumask_t __iscsi_allowed_cpumask;
+
+static ssize_t lio_target_wwn_cpus_allowed_list_show(
+		struct config_item *item, char *page)
+{
+	return sprintf(page, "%*pbl\n",
+		cpumask_pr_args(&__iscsi_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;
+
+	orig = kstrdup(page, GFP_KERNEL);
+	if (!orig)
+		return -ENOMEM;
+
+	cpumask_clear(&new_allowed_cpumask);
+	ret = cpulist_parse(orig, &new_allowed_cpumask);
+
+	kfree(orig);
+	if (ret != 0)
+		return ret;
+
+	cpumask_copy(&__iscsi_allowed_cpumask, &new_allowed_cpumask);
+	return count;
+}
+
+CONFIGFS_ATTR(lio_target_wwn_, cpus_allowed_list);
+
 static struct configfs_attribute *lio_target_wwn_attrs[] = {
 	&lio_target_wwn_attr_lio_version,
+	&lio_target_wwn_attr_cpus_allowed_list,
 	NULL,
 };
 
diff --git a/drivers/target/iscsi/iscsi_target_login.c b/drivers/target/iscsi/iscsi_target_login.c
index 1a9c50401bdb..910f35e4648a 100644
--- a/drivers/target/iscsi/iscsi_target_login.c
+++ b/drivers/target/iscsi/iscsi_target_login.c
@@ -1129,8 +1129,15 @@  static struct iscsi_conn *iscsit_alloc_conn(struct iscsi_np *np)
 		goto free_conn_ops;
 	}
 
+	if (!zalloc_cpumask_var(&conn->allowed_cpumask, GFP_KERNEL)) {
+		pr_err("Unable to allocate conn->allowed_cpumask\n");
+		goto free_conn_cpumask;
+	}
+
 	return conn;
 
+free_conn_cpumask:
+	free_cpumask_var(conn->allowed_cpumask);
 free_conn_ops:
 	kfree(conn->conn_ops);
 put_transport:
diff --git a/include/target/iscsi/iscsi_target_core.h b/include/target/iscsi/iscsi_target_core.h
index 1eccb2ac7d02..c5e9cad187cf 100644
--- a/include/target/iscsi/iscsi_target_core.h
+++ b/include/target/iscsi/iscsi_target_core.h
@@ -580,6 +580,7 @@  struct iscsi_conn {
 	struct ahash_request	*conn_tx_hash;
 	/* Used for scheduling TX and RX connection kthreads */
 	cpumask_var_t		conn_cpumask;
+	cpumask_var_t		allowed_cpumask;
 	unsigned int		conn_rx_reset_cpumask:1;
 	unsigned int		conn_tx_reset_cpumask:1;
 	/* list_head of struct iscsi_cmd for this connection */