lkml.org 
[lkml]   [2018]   [May]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
SubjectRe: [PATCH 1/2] Convert target drivers to use sbitmap
From
Date
On 5/15/18 10:00 AM, Matthew Wilcox wrote:
> From: Matthew Wilcox <mawilcox@microsoft.com>
>
> The sbitmap and the percpu_ida perform essentially the same task,
> allocating tags for commands. Since the sbitmap is more used than
> the percpu_ida, convert the percpu_ida users to the sbitmap API.

It should also be the same performance as percpu_ida in light use, and
performs much better at > 50% utilization of the tag space. I think
that's better justification than "more used than".

> diff --git a/drivers/target/iscsi/iscsi_target_util.c b/drivers/target/iscsi/iscsi_target_util.c
> index 4435bf374d2d..28bcffae609f 100644
> --- a/drivers/target/iscsi/iscsi_target_util.c
> +++ b/drivers/target/iscsi/iscsi_target_util.c
> @@ -17,7 +17,7 @@
> ******************************************************************************/
>
> #include <linux/list.h>
> -#include <linux/percpu_ida.h>
> +#include <linux/sched/signal.h>
> #include <net/ipv6.h> /* ipv6_addr_equal() */
> #include <scsi/scsi_tcq.h>
> #include <scsi/iscsi_proto.h>
> @@ -147,6 +147,28 @@ void iscsit_free_r2ts_from_list(struct iscsi_cmd *cmd)
> spin_unlock_bh(&cmd->r2t_lock);
> }
>
> +int iscsit_wait_for_tag(struct se_session *se_sess, int state, int *cpup)
> +{
> + int tag = -1;
> + DEFINE_WAIT(wait);
> + struct sbq_wait_state *ws;
> +
> + if (state == TASK_RUNNING)
> + return tag;
> +
> + ws = &se_sess->sess_tag_pool.ws[0];
> + for (;;) {
> + prepare_to_wait_exclusive(&ws->wait, &wait, state);
> + if (signal_pending_state(state, current))
> + break;
> + schedule();
> + tag = sbitmap_queue_get(&se_sess->sess_tag_pool, cpup);
> + }
> +
> + finish_wait(&ws->wait, &wait);
> + return tag;
> +}

Seems like that should be:


ws = &se_sess->sess_tag_pool.ws[0];
for (;;) {
prepare_to_wait_exclusive(&ws->wait, &wait, state);
if (signal_pending_state(state, current))
break;
tag = sbitmap_queue_get(&se_sess->sess_tag_pool, cpup);
if (tag != -1)
break;
schedule();
}

finish_wait(&ws->wait, &wait);
return tag;

> /*
> * May be called from software interrupt (timer) context for allocating
> * iSCSI NopINs.
> @@ -155,9 +177,11 @@ struct iscsi_cmd *iscsit_allocate_cmd(struct iscsi_conn *conn, int state)
> {
> struct iscsi_cmd *cmd;
> struct se_session *se_sess = conn->sess->se_sess;
> - int size, tag;
> + int size, tag, cpu;
>
> - tag = percpu_ida_alloc(&se_sess->sess_tag_pool, state);
> + tag = sbitmap_queue_get(&se_sess->sess_tag_pool, &cpu);
> + if (tag < 0)
> + tag = iscsit_wait_for_tag(se_sess, state, &cpu);
> if (tag < 0)
> return NULL;

Might make sense to just roll the whole thing into iscsi_get_tag(), that
would be cleaner.

sbitmap should provide a helper for that, but we can do that cleanup
later. That would encapsulate things like the per-cpu caching hint too,
for instance.

Rest looks fine to me.

--
Jens Axboe

\
 
 \ /
  Last update: 2018-05-15 18:12    [W:0.164 / U:0.464 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site