From: Arne R. <arn...@go...> - 2011-01-31 19:49:59
|
2011/1/31 Ross S. W. Walker <RW...@me...>: > Arne Redlich [mailto:arn...@go...] wrote: >> 2011/1/31 Ross S. W. Walker <RW...@me...>: >> > >> > Arne, >> > >> > Can you use your git-svn fu to highlight the changes in nthread.c and >> > iscsi.c between 1.4.19 and 1.4.20.2? >> > >> > I suspect it might have resulted from the changes we did to work around >> > the MC/S bug. >> >> No git svn fu required, even though it helps greatly to overcome the >> slowness of svn diff. :) >> Anyway, diff is attached. > > I suspect it's the iscsi_session_push_cmnd coupled with the handling > of null length PDUs coming in from GPXE, probably cause it's running > dumb and blind. > > static void data_out_end(struct iscsi_conn *conn, struct iscsi_cmnd *cmnd) > { > struct iscsi_data_out_hdr *req = (struct iscsi_data_out_hdr *) &cmnd->pdu.bhs; > @@ -1116,8 +1128,7 @@ static void data_out_end(struct iscsi_conn *conn, struct iscsi_cmnd *cmnd) > if (req->ttt == cpu_to_be32(ISCSI_RESERVED_TAG)) { > if (req->flags & ISCSI_FLG_FINAL) { > scsi_cmnd->is_unsolicited_data = 0; > - if (!cmnd_pending(scsi_cmnd)) > - scsi_cmnd_exec(scsi_cmnd); > + iscsi_session_push_cmnd(scsi_cmnd); > } > } else { > /* TODO : proper error handling */ > @@ -1132,7 +1143,7 @@ static void data_out_end(struct iscsi_conn *conn, struct iscsi_cmnd *cmnd) > if (scsi_cmnd->r2t_length == 0) > assert(list_empty(&scsi_cmnd->pdu_list)); > > - scsi_cmnd_exec(scsi_cmnd); > + iscsi_session_push_cmnd(scsi_cmnd); > } > > out: > @@ -1140,35 +1151,64 @@ out: > return; > } > > Also the tail end of data_out_end: > > } else { > /* TODO : proper error handling */ > if (!(req->flags & ISCSI_FLG_FINAL) && scsi_cmnd->r2t_length == 0) > eprintk("initiator error %x\n", cmnd_itt(scsi_cmnd)); > > if (!(req->flags & ISCSI_FLG_FINAL)) > goto out; > > scsi_cmnd->outstanding_r2t--; > > if (scsi_cmnd->r2t_length == 0) > assert(list_empty(&scsi_cmnd->pdu_list)); > > iscsi_session_push_cmnd(scsi_cmnd); > } > > Seems we hit the assert down here, cause of the null PDU > with the final bit set. > > Where should we filter out improper length PDUs, probably > in data_out_start()? AFAICT there is a test already that leads to the PDU being skipped (the one issueing the "invalid data length" message), so I presume the subsequent cleanup of the PDU is incorrect. Arne |