On Thu, 2017-02-09 at 23:20 -0800, Nicholas A. Bellinger wrote: > On Fri, 2017-02-10 at 01:13 +0000, Bart Van Assche wrote: > > On Thu, 2017-02-09 at 17:07 -0800, Nicholas A. Bellinger wrote: > > > On Thu, 2017-02-09 at 17:24 +0000, Bart Van Assche wrote: > > > > On Thu, 2017-02-09 at 01:39 -0800, Nicholas A. Bellinger wrote: > > > > > There is a deadlock here. While se_sess->sess_cmd_lock is held and > > > > > transport_lookup_tmr_lun() is called above, se_device->se_tmr_lock is > > > > > also taken. > > > > > > > > I'll move the transport_lookup_tmr_lun() further down. > > > > > > It should be done in transport_generic_handle_tmr(). > > > > Please explain why you think that's how it should be done. I'm not convinced. > > Doing it 'further down' is totally broken for LUN != 0. > > transport_lookup_tmr_lun() is responsible for adding se_tmr to > se_device->tmr_list, but it's also responsible for se_cmd->orig_fe_lun > assignment based on incoming unpacked_lun from fabric driver code. > > Since this patch tries to call transport_lookup_tmr_lun() from within > tmr_wq context using a se_cmd->orig_fe_lun that is always zero, it's > always going to be issuing all TMRs to LUN=0. Your comment seems to apply to the se_cmd structure of the task management function. But that's not what is passed to transport_lookup_tmr_lun(). The LUN passed in my patch to that function is the LUN of the command that is being aborted. Had you perhaps misinterpret my patch? Bart.-- To unsubscribe from this list: send the line "unsubscribe target-devel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
