search for: signal_pending_state

...> + 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; This looks weird to me. Shouldn't target code ignore signals instead of causing tag allocation to fail if a signal is received? > + schedule(); > + tag = sbitmap_queue_get(&se_sess->sess_tag_pool, cpup); > + } > + > + finish_wait(&w...

...> + 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; This looks weird to me. Shouldn't target code ignore signals instead of causing tag allocation to fail if a signal is received? > + schedule(); > + tag = sbitmap_queue_get(&se_sess->sess_tag_pool, cpup); > + } > + > + finish_wait(&w...

...> + 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 (;;)...

...> + 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 (;;)...

...AIT(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: > > &...

...; > > + 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; > > This looks weird to me. Shouldn't target code ignore signals instead of causing > tag allocation to fail if a signal is received? It's what the current code did: - if (signal_pending_state(state, current)) { -...

From: Matthew Wilcox <mawilcox at microsoft.com> This is a pretty rough-and-ready conversion of the target drivers from using percpu_ida to sbitmap. It compiles; I don't have a target setup, so it's completely untested. I haven't tried to do anything particularly clever here, so it's possible that, for example, the wait queue in iscsi_target_util could be more clever, like

...iver ever has had any users other than the developer of that driver. > > This looks weird to me. Shouldn't target code ignore signals instead of causing > > tag allocation to fail if a signal is received? > > It's what the current code did: > > - if (signal_pending_state(state, current)) { > - tag = -ERESTARTSYS; > - break; > - } > > and the current callers literally indicate that they want signals: > > drivers/infiniband/ulp/isert/ib_isert.c: cmd = iscsit_allocate_cmd(conn, TAS...

Removing the percpu_ida code nets over 400 lines of removal. It's not as spectacular as deleting an entire architecture, but it's still a worthy reduction in lines of code. Untested due to lack of hardware and not understanding how to set up a target platform. Changes from v1: - Fixed bugs pointed out by Jens in iscsit_wait_for_tag() - Abstracted out tag freeing as requested by Bart

Removing the percpu_ida code nets over 400 lines of removal. It's not as spectacular as deleting an entire architecture, but it's still a worthy reduction in lines of code. Untested due to lack of hardware and not understanding how to set up a target platform. Changes from v1: - Fixed bugs pointed out by Jens in iscsit_wait_for_tag() - Abstracted out tag freeing as requested by Bart

...g(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; +} + /* * May be called from software interrupt (timer) context for allocating * iSCSI NopINs. @@ -155,9 +177,11 @@ struc...