| Message ID | 20210120144550.697303-1-fengnanchang@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] mmc: limit the time of retries when analyse tuples failed | expand |
On Wed, 20 Jan 2021 at 15:47, Fengnan Chang <fengnanchang@gmail.com> wrote: > > when analyse tuples failed, may enter an endless loop，so limit the time of retries. Since this is fixing a real bug for you, it looks like we should tag this for stable kernels as well, right? > > Signed-off-by: Fengnan Chang <fengnanchang@gmail.com> > --- > drivers/mmc/core/sdio_cis.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/drivers/mmc/core/sdio_cis.c b/drivers/mmc/core/sdio_cis.c > index dcb3dee59fa5..47423a6d13fb 100644 > --- a/drivers/mmc/core/sdio_cis.c > +++ b/drivers/mmc/core/sdio_cis.c > @@ -266,6 +266,7 @@ static int sdio_read_cis(struct mmc_card *card, struct sdio_func *func) > > do { > unsigned char tpl_code, tpl_link; > + u64 timeout = get_jiffies_64() + 10 * HZ; To be consistent with how we do time based polling (see __mmc_poll_for_busy() in mmc_ops.c for example) I would prefer if you use a define for the timeout, rather than a magic value as here. #define SDIO_READ_CIS_TIMEOUT_MS (10 * 1000) /* 10s */ Additionally, regular jiffies should be sufficient I think. Thus I prefer if you could specify the timeout along the lines like the below: timeout = jiffies + msecs_to_jiffies(SDIO_READ_CIS_TIMEOUT_MS); > > ret = mmc_io_rw_direct(card, 0, 0, ptr++, 0, &tpl_code); > if (ret) > @@ -318,6 +319,8 @@ static int sdio_read_cis(struct mmc_card *card, struct sdio_func *func) > prev = &this->next; > > if (ret == -ENOENT) { > + if (time_after64(get_jiffies_64(), timeout)) > + break; > /* warn about unknown tuples */ > pr_warn_ratelimited("%s: queuing unknown" > " CIS tuple 0x%02x (%u bytes)\n", > -- > 2.25.1 > Other than that, this looks okay to me. Kind regards Uffe
yeah, I think it should tag this for stable kernels as well. I will send a new version later. Ulf Hansson <ulf.hansson@linaro.org> 于2021年1月20日周三 下午11:29写道： > > On Wed, 20 Jan 2021 at 15:47, Fengnan Chang <fengnanchang@gmail.com> wrote: > > > > when analyse tuples failed, may enter an endless loop，so limit the time of retries. > > Since this is fixing a real bug for you, it looks like we should tag > this for stable kernels as well, right? > > > > > Signed-off-by: Fengnan Chang <fengnanchang@gmail.com> > > --- > > drivers/mmc/core/sdio_cis.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/mmc/core/sdio_cis.c b/drivers/mmc/core/sdio_cis.c > > index dcb3dee59fa5..47423a6d13fb 100644 > > --- a/drivers/mmc/core/sdio_cis.c > > +++ b/drivers/mmc/core/sdio_cis.c > > @@ -266,6 +266,7 @@ static int sdio_read_cis(struct mmc_card *card, struct sdio_func *func) > > > > do { > > unsigned char tpl_code, tpl_link; > > + u64 timeout = get_jiffies_64() + 10 * HZ; > > To be consistent with how we do time based polling (see > __mmc_poll_for_busy() in mmc_ops.c for example) I would prefer if you > use a define for the timeout, rather than a magic value as here. > > #define SDIO_READ_CIS_TIMEOUT_MS (10 * 1000) /* 10s */ > > Additionally, regular jiffies should be sufficient I think. Thus I > prefer if you could specify the timeout along the lines like the > below: > timeout = jiffies + msecs_to_jiffies(SDIO_READ_CIS_TIMEOUT_MS); > > > > > ret = mmc_io_rw_direct(card, 0, 0, ptr++, 0, &tpl_code); > > if (ret) > > @@ -318,6 +319,8 @@ static int sdio_read_cis(struct mmc_card *card, struct sdio_func *func) > > prev = &this->next; > > > > if (ret == -ENOENT) { > > + if (time_after64(get_jiffies_64(), timeout)) > > + break; > > /* warn about unknown tuples */ > > pr_warn_ratelimited("%s: queuing unknown" > > " CIS tuple 0x%02x (%u bytes)\n", > > -- > > 2.25.1 > > > > Other than that, this looks okay to me. > > Kind regards > Uffe
diff --git a/drivers/mmc/core/sdio_cis.c b/drivers/mmc/core/sdio_cis.c index dcb3dee59fa5..47423a6d13fb 100644 --- a/drivers/mmc/core/sdio_cis.c +++ b/drivers/mmc/core/sdio_cis.c @@ -266,6 +266,7 @@ static int sdio_read_cis(struct mmc_card *card, struct sdio_func *func) do { unsigned char tpl_code, tpl_link; + u64 timeout = get_jiffies_64() + 10 * HZ; ret = mmc_io_rw_direct(card, 0, 0, ptr++, 0, &tpl_code); if (ret) @@ -318,6 +319,8 @@ static int sdio_read_cis(struct mmc_card *card, struct sdio_func *func) prev = &this->next; if (ret == -ENOENT) { + if (time_after64(get_jiffies_64(), timeout)) + break; /* warn about unknown tuples */ pr_warn_ratelimited("%s: queuing unknown" " CIS tuple 0x%02x (%u bytes)\n",
when analyse tuples failed, may enter an endless loop，so limit the time of retries. Signed-off-by: Fengnan Chang <fengnanchang@gmail.com> --- drivers/mmc/core/sdio_cis.c | 3 +++ 1 file changed, 3 insertions(+) -- 2.25.1