[PATCHv4,0/4] tcmu: bug fix and dynamic growing data area support

Commit Message

Ilias Tsitsimpis March 23, 2017, 4:02 p.m. UTC

Hi Xiubo,

On Tue, Mar 21, 2017 at 04:36PM, [e-mail] wrote:
> [...]
>   tcmu: Fix possible overwrite of t_data_sg's last iov[]
>   tcmu: Fix wrongly calculating of the base_command_size

I tested these two patches, which try to fix the broken support for
BIDI commands in target/user.

Both look good to me, but unfortunately, there is also another
regression which got introduced with the use of the data_bitmap. More
specifically, in case of BIDI commands, the data bitmap records both the
Data-Out and the Data-In buffer, and so when gathering the data in the
tcmu_handle_completion() function, care should be taken in order to
discard the Data-Out buffer before gathering the Data-In buffer.
Something like this should do the trick:


With your patches, and the above diff, support for BIDI commands in the
target/user seems to be working again.

Could you please validate the above snippet, and update your patches to
include it?

Comments

Ilias Tsitsimpis March 25, 2017, 8:57 a.m. UTC | #1

On Fri, Mar 24, 2017 at 02:45AM, 李秀波 wrote:
> IliasTsitsimpis <[e-mail]>写到：
> >Could you please validate the above snippet, and update your patches to
> >include it?
> 
> They are actually two separate issues, but all for BIDI case.
> 
> I'll merged into my patch.

Since these are two separate issues, I would create a new patch.
Also, feel free to add 'Tested-by' me to the first two patches.

Patch

diff --git a/drivers/target/target_core_user.c b/drivers/target/target_core_user.c
index 1108bf5..7075161 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -610,10 +610,22 @@  static void tcmu_handle_completion(struct tcmu_cmd *cmd, struct tcmu_cmd_entry *
                               se_cmd->scsi_sense_length);
                free_data_area(udev, cmd);
        } else if (se_cmd->se_cmd_flags & SCF_BIDI) {
+               struct scatterlist *sg;
+               int n, block, sg_remaining = 0;
                DECLARE_BITMAP(bitmap, DATA_BLOCK_BITS);

-               /* Get Data-In buffer before clean up */
                bitmap_copy(bitmap, cmd->data_bitmap, DATA_BLOCK_BITS);
+
+               /* Discard Data-Out buffer */
+               for_each_sg(se_cmd->t_data_sg, sg, se_cmd->t_data_nents, n) {
+                       sg_remaining += sg->length;
+                       while (sg_remaining > 0) {
+                               block = find_first_bit(bitmap, DATA_BLOCK_BITS);
+                               clear_bit(block, bitmap);
+                               sg_remaining -= DATA_BLOCK_SIZE;
+                       }
+               }
+
+               /* Get Data-In buffer before clean up */
                gather_data_area(udev, bitmap,
                        se_cmd->t_bidi_data_sg, se_cmd->t_bidi_data_nents);
                free_data_area(udev, cmd);