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, lixiubo@xxxxxxxxxxxxxxxxxxxx wrote:
> > From: Xiubo Li <lixiubo@xxxxxxxxxxxxxxxxxxxx>
> >
> > 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 <bryantly@xxxxxxxxxxxxxxxxxx>
> > Signed-off-by: Xiubo Li <lixiubo@xxxxxxxxxxxxxxxxxxxx>
> > ---
> > 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��.n��������+%������w��{.n����j� ����{ay� ʇڙ���f���h������_�(�階�ݢj"��������G����?���&��
