On Tue, 2017-04-04 at 10:36 +0530, Varun Prakash wrote:
> On Sun, Apr 02, 2017 at 03:59:05PM -0700, Nicholas A. Bellinger wrote:
> > On Thu, 2017-03-30 at 10:12 -0700, Bart Van Assche wrote:
> > > While releasing a command __iscsit_free_cmd() can be called multiple
> > > times but .iscsit_release_cmd() must be called only once. Hence move
> > > the .iscsit_release_cmd() call into iscsit_release_cmd(). The latter
> > > function is only called once per command. The only driver that defines
> > > the .iscsit_release_cmd() callback is the cxgbit driver so this change
> > > only affects the cxgbit driver.
> > >
> > > Fixes: 7ec811a8e9c3 ("iscsi-target: add void (*iscsit_release_cmd)()")
> > > Signed-off-by: Bart Van Assche <bart.vanassche@xxxxxxxxxxx>
> > > Cc: Varun Prakash <varun@xxxxxxxxxxx>
> > > Cc: Nicholas Bellinger <nab@xxxxxxxxxxxxxxx>
> > > Cc: <stable@xxxxxxxxxxxxxxx>
> > > ---
> > > drivers/target/iscsi/iscsi_target_util.c | 15 +++++++++------
> > > 1 file changed, 9 insertions(+), 6 deletions(-)
> > >
> >
> > Applied to target-pending/for-next, but dropping the stable CC' because
> > the single caller in cxgbit_release_cmd() is already checking to ensure
> > resources are only released on the first invocation.
> >
> > So it's not a bug-fix.
>
> In case of DDP cxgbit driver assigns cmd->se_cmd.t_data_sg to ttinfo->sgl
> and calls dma_map_sg(), cxgbit_release_cmd() calls dma_unmap_sg(), it needs
> a valid sg(ttinfo->sgl), before calling iscsit_release_cmd()
> cmd->se_cmd.t_data_sg gets freed so ttinfo->sgl will not be valid if we move
> ->iscsit_release_cmd() to iscsit_release_cmd().
(replying to an e-mail of six months ago)
Hello Varun,
Have you noticed that commit febe562c20df (target: Fix LUN_RESET active I/O
handling for ACK_KREF; January 2016) moved the transport_free_pages() call
from transport_put_cmd() to target_release_cmd_kref()? I think that means
that it is now safe to call .iscsit_release_cmd() after
transport_generic_free_cmd().
Thanks,
Bart.