Re: [PATCH v2 1/2] mmc: block: remove the check of packed for end_packed_req and hdr_wrq_prep

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 24 August 2016 at 09:58, Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> wrote:
> packed should always exist without calling its cleanup function
> explicitly. Moreover, we have use it when preparing packed list.
> So I don't believe we should ever fall into this check again when
> doing mmc_blk_packed_hdr_wrq_prep or mmc_blk_end_packed_req. And
> the code of mmc_blk_end_packed_req is trying to use packed before
> checking it which makes it quite weird. This patch is trying to
> remove these two checks and move it to the mmc_blk_prep_packed_list.
> If we find packed is null, then we should never use MMC_PACKED_NONE
> and MMC_BLK_PACKED_CMD. By doing this, we could fall back to
> non-packed request if finding null packed, though it's impossible
> theoretically.
>
> Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx>
>
> ---
>
> Changes in v2:
> - remove BUG_ON and add fallback for packed instead of
>   panic it.
>
>  drivers/mmc/card/block.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c
> index 48a5dd7..c7a9b28 100644
> --- a/drivers/mmc/card/block.c
> +++ b/drivers/mmc/card/block.c
> @@ -1673,6 +1673,18 @@ static u8 mmc_blk_prep_packed_list(struct mmc_queue *mq, struct request *req)
>         u8 max_packed_rw = 0;
>         u8 reqs = 0;
>
> +       /*
> +        * We don't need to check packed for any further
> +        * operation of packed stuff as we set MMC_PACKED_NONE
> +        * and return zero for reqs if geting null packed. Also
> +        * we clean the flag of MMC_BLK_PACKED_CMD to avoid doing
> +        * it again when removing blk req.
> +        */
> +       if (!mqrq->packed) {
> +               md->flags &= (~MMC_BLK_PACKED_CMD);
> +               goto no_packed;
> +       }
> +
>         if (!(md->flags & MMC_BLK_PACKED_CMD))
>                 goto no_packed;
>
> @@ -1781,8 +1793,6 @@ static void mmc_blk_packed_hdr_wrq_prep(struct mmc_queue_req *mqrq,
>         u8 hdr_blocks;
>         u8 i = 1;
>
> -       BUG_ON(!packed);
> -
>         mqrq->cmd_type = MMC_PACKED_WRITE;
>         packed->blocks = 0;
>         packed->idx_failure = MMC_PACKED_NR_IDX;
> @@ -1886,8 +1896,6 @@ static int mmc_blk_end_packed_req(struct mmc_queue_req *mq_rq)
>         int idx = packed->idx_failure, i = 0;
>         int ret = 0;
>
> -       BUG_ON(!packed);
> -
>         while (!list_empty(&packed->list)) {
>                 prq = list_entry_rq(packed->list.next);
>                 if (idx == i) {
> --
> 2.3.7
>
>

This is step in the right direction, although there are still some
BUG_ONs() for the packed command path. Could you please have look and
remove all of them?

Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe linux-mmc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux