Xiubo, Well done ! This fixed my problem. The ZBC test suite now passes all tests on my target without crashing the kernel.
Please see some comments/nitpicks below. Otherwise, please feel free to add my "tested-by" On Tue, 2017-07-11 at 17:06 +0800, Xiubo Li wrote: > On 2017年07月11日 16:05, [email protected] wrote: > > From: Xiubo Li <[email protected]> > > > > For all the entries allocated from the ring cmd area, the memory > > is something like the stack, which will reserve the old data, so > > the entry->req.iov_bidi_cnt maybe none zero. > > > > To fix this, just memset all the entry memory before using it, and > > also to be more readable we adjust the bidi code. > > > > Fixed: fe25cc34795(tcmu: Recalculate the tcmu_cmd size to save cmd area > > memories) > > Reported-by: Bryant G. Ly <[email protected]> > > Signed-off-by: Xiubo Li <[email protected]> > > --- > > drivers/target/target_core_user.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/target/target_core_user.c > > b/drivers/target/target_core_user.c > > index 2f1fa92..be62c86 100644 > > --- a/drivers/target/target_core_user.c > > +++ b/drivers/target/target_core_user.c > > @@ -840,6 +840,7 @@ static inline size_t tcmu_cmd_get_cmd_size(struct > > tcmu_cmd *tcmu_cmd, > > } > > > > entry = (void *) mb + CMDR_OFF + cmd_head; > > + memset(entry, 0, command_size); > > tcmu_hdr_set_op(&entry->hdr.len_op, TCMU_OP_CMD); > > entry->hdr.cmd_id = tcmu_cmd->cmd_id; > > entry->hdr.kflags = 0; The added memset allows removing the 0 assignment here, and the one that follows this one too. > > @@ -865,8 +866,8 @@ static inline size_t tcmu_cmd_get_cmd_size(struct > > tcmu_cmd *tcmu_cmd, > > entry->req.iov_dif_cnt = 0; > > > > /* Handle BIDI commands */ > > + iov_cnt = 0; > > if (se_cmd->se_cmd_flags & SCF_BIDI) { > > - iov_cnt = 0; > > iov++; > > ret = scatter_data_area(udev, tcmu_cmd, > > se_cmd->t_bidi_data_sg, > > @@ -879,8 +880,8 @@ static inline size_t tcmu_cmd_get_cmd_size(struct > > tcmu_cmd *tcmu_cmd, > > pr_err("tcmu: alloc and scatter bidi data > > failed\n"); > > return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; > > } > > - entry->req.iov_bidi_cnt = iov_cnt; > > } > > + entry->req.iov_bidi_cnt = iov_cnt; > > > > /* > > * Recalaulate the command's base size and size according For reference, here is the actual patch I used for testing: diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c index 2f1fa92..3b25ef3 100644 --- a/drivers/target/target_core_user.c +++ b/drivers/target/target_core_user.c @@ -563,7 +563,7 @@ static int scatter_data_area(struct tcmu_dev *udev, to_offset = get_block_offset_user(udev, dbi, block_remaining); offset = DATA_BLOCK_SIZE - block_remaining; - to = (void *)(unsigned long)to + offset; + to += offset; if (*iov_cnt != 0 && to_offset == iov_tail(udev, *iov)) { @@ -636,7 +636,7 @@ static void gather_data_area(struct tcmu_dev *udev, struct tcmu_cmd *cmd, copy_bytes = min_t(size_t, sg_remaining, block_remaining); offset = DATA_BLOCK_SIZE - block_remaining; - from = (void *)(unsigned long)from + offset; + from += offset; tcmu_flush_dcache_range(from, copy_bytes); memcpy(to + sg->length - sg_remaining, from, copy_bytes); @@ -840,10 +840,9 @@ tcmu_queue_cmd_ring(struct tcmu_cmd *tcmu_cmd) } entry = (void *) mb + CMDR_OFF + cmd_head; + memset(entry, 0, command_size); tcmu_hdr_set_op(&entry->hdr.len_op, TCMU_OP_CMD); entry->hdr.cmd_id = tcmu_cmd->cmd_id; - entry->hdr.kflags = 0; - entry->hdr.uflags = 0; /* Handle allocating space from the data area */ tcmu_cmd_reset_dbi_cur(tcmu_cmd); @@ -862,11 +861,10 @@ tcmu_queue_cmd_ring(struct tcmu_cmd *tcmu_cmd) return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; } entry->req.iov_cnt = iov_cnt; - entry->req.iov_dif_cnt = 0; /* Handle BIDI commands */ + iov_cnt = 0; if (se_cmd->se_cmd_flags & SCF_BIDI) { - iov_cnt = 0; iov++; ret = scatter_data_area(udev, tcmu_cmd, se_cmd->t_bidi_data_sg, @@ -879,8 +877,8 @@ tcmu_queue_cmd_ring(struct tcmu_cmd *tcmu_cmd) pr_err("tcmu: alloc and scatter bidi data failed\n"); return TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE; } - entry->req.iov_bidi_cnt = iov_cnt; } + entry->req.iov_bidi_cnt = iov_cnt; /* * Recalaulate the command's base size and size according -- Best regards. -- Damien Le Moal Western Digital
