Re: [PATCH 3/3] target: simplify target_wait_for_sess_cmds()

2013-05-14 Thread Nicholas A. Bellinger
On Tue, 2013-05-14 at 22:19 -0400, Jörn Engel wrote: > On Tue, 14 May 2013 20:05:30 -0700, Nicholas A. Bellinger wrote: > > On Tue, 2013-05-14 at 12:29 -0400, Jörn Engel wrote: > > > On Mon, 13 May 2013 20:08:44 -0700, Nicholas A. Bellinger wrote: > > > > On Mon, 2013-05-13 at 18:00 -0400, Jörn Eng

Re: [PATCH 3/3] target: simplify target_wait_for_sess_cmds()

2013-05-14 Thread Jörn Engel
On Tue, 14 May 2013 20:05:30 -0700, Nicholas A. Bellinger wrote: > On Tue, 2013-05-14 at 12:29 -0400, Jörn Engel wrote: > > On Mon, 13 May 2013 20:08:44 -0700, Nicholas A. Bellinger wrote: > > > On Mon, 2013-05-13 at 18:00 -0400, Jörn Engel wrote: > > > > > > > > I agree that the overhead doesn't

Re: [PATCH 3/3] target: simplify target_wait_for_sess_cmds()

2013-05-14 Thread Nicholas A. Bellinger
On Tue, 2013-05-14 at 12:29 -0400, Jörn Engel wrote: > On Mon, 13 May 2013 20:08:44 -0700, Nicholas A. Bellinger wrote: > > On Mon, 2013-05-13 at 18:00 -0400, Jörn Engel wrote: > > > > > > I agree that the overhead doesn't matter. The msleep(100) spells this > > > out rather explicitly. What doe

Re: [PATCH 3/3] target: simplify target_wait_for_sess_cmds()

2013-05-14 Thread Jörn Engel
On Mon, 13 May 2013 20:08:44 -0700, Nicholas A. Bellinger wrote: > On Mon, 2013-05-13 at 18:00 -0400, Jörn Engel wrote: > > > > I agree that the overhead doesn't matter. The msleep(100) spells this > > out rather explicitly. What does matter is that a) the patch retains > > old behaviour with mu

Re: [PATCH 3/3] target: simplify target_wait_for_sess_cmds()

2013-05-13 Thread Nicholas A. Bellinger
On Mon, 2013-05-13 at 18:00 -0400, Jörn Engel wrote: > On Mon, 13 May 2013 16:00:03 -0700, Nicholas A. Bellinger wrote: > > On Mon, 2013-05-13 at 16:30 -0400, Joern Engel wrote: > > > The second parameter was always 0, leading to effectively dead code. It > > > called list_del() and se_cmd->se_tfo

Re: [PATCH 3/3] target: simplify target_wait_for_sess_cmds()

2013-05-13 Thread Jörn Engel
On Mon, 13 May 2013 16:00:03 -0700, Nicholas A. Bellinger wrote: > On Mon, 2013-05-13 at 16:30 -0400, Joern Engel wrote: > > The second parameter was always 0, leading to effectively dead code. It > > called list_del() and se_cmd->se_tfo->release_cmd(), and had to set a > > flag to prevent target_

Re: [PATCH 3/3] target: simplify target_wait_for_sess_cmds()

2013-05-13 Thread Nicholas A. Bellinger
On Mon, 2013-05-13 at 16:30 -0400, Joern Engel wrote: > The second parameter was always 0, leading to effectively dead code. It > called list_del() and se_cmd->se_tfo->release_cmd(), and had to set a > flag to prevent target_release_cmd_kref() from doing the same. Look again. The call to transpo

[PATCH 3/3] target: simplify target_wait_for_sess_cmds()

2013-05-13 Thread Joern Engel
The second parameter was always 0, leading to effectively dead code. It called list_del() and se_cmd->se_tfo->release_cmd(), and had to set a flag to prevent target_release_cmd_kref() from doing the same. But most of all, it iterated the list without taking se_sess->sess_cmd_lock, leading to race