On Thu, 28 Feb 2019 at 02:41, Jiong wu <lohengrin1024@xxxxxxxxx> wrote:
>
> In fact, it happened.
>
> For instance, a type of Toshiba 128G emmc(THGBMHT0C8LBAIG) has ext_csd that ERASE_TIMEOUT_MULT is 0x73, and TRIM_MULT
> is 0x01. So the card->ext_csd.hc_erase_timeout is 0x73 * 300ms(34.5s), which is larger than host->max_busy_timeout(30s) of kirin710. Consequently, it came out a condition that max_discard is calculated as 0.
>
> I suggest you to accept this fix into mainstream, firstly, it's exactly a logic mistake and can really be encountered. secondly, the change is small and clear, the function and correctness has been verified.
Thanks for clarifying! However, by looking at the code once more, I
suspect your analyze is in-correct. Please have a look at the below
comment.
>
> Thank you.
>
> Jiong Wu
>
>
> -------- 原始邮件 --------
> 主题：Re: [PATCH] mmc:fix a bug when max_discard is 0
> 发件人：Ulf Hansson
> 收件人：Jiong Wu
> 抄送：Shawn Lin ,Adrian Hunter ,Avri Altman ,Wolfram Sang ,Sergio Valverde ,Martin Hicks ,linux-mmc@xxxxxxxxxxxxxxx
>
> On Sun, 17 Feb 2019 at 09:26, Jiong Wu <lohengrin1024@xxxxxxxxx> wrote:
> >
> > The original purpose of the code I fix is to choose max_trim as the final
> > parameter instead of max_discard if max_trim is less than max_discard. But
> > there is a uncommon case that max_discard is 0 because max_discard is
> > overflow the maximum threshold. In this case, we should still choose
> > max_trim instead of max_discard, however, in the original code(if(max_trim
> > < max_discard)), the conditio is false when max_discard is 0. So I fix it
> > by add "|| max_discard == 0" to cover this case.
> >
> > Signed-off-by: Jiong Wu <Lohengrin1024@xxxxxxxxx>
>
> I am guessing this is more of a hypothetical problem rather than a
> real one, thus I am not queuing this a fix. If people want this for
> stable, they can always send a backport.
>
> So, applied for next, thanks!
>
> Kind regards
> Uffe
>
>
> > ---
> > drivers/mmc/core/core.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
> > index 5bd58b9..7b1bd95 100644
> > --- a/drivers/mmc/core/core.c
> > +++ b/drivers/mmc/core/core.c
> > @@ -2383,7 +2383,7 @@ unsigned int mmc_calc_max_discard(struct mmc_card *card)
> > max_discard = mmc_do_calc_max_discard(card, MMC_ERASE_ARG);
> > if (max_discard && mmc_can_trim(card)) {
> > max_trim = mmc_do_calc_max_discard(card, MMC_TRIM_ARG);
> > - if (max_trim < max_discard)
> > + if (max_trim < max_discard || max_discard == 0)
We have already checked that max_discard has a non-zero value, a few
lines above, so I think this change make no sense and has no impact.
> > max_discard = max_trim;
> > } else if (max_discard < card->erase_size) {
> > max_discard = 0;
> > --
> > 2.7.4
> >
So, I am dropping the patch until we have sorted this out. It seems
like there is a different problem.
Kind regards
Uffe
