Re: [PATCH v2] mmc: core: Optimize the mmc erase size alignment

[Baolin Wang]

Hi Ulf and Shawn,

On 27 July 2016 at 18:02, Baolin Wang <baolin.wang@xxxxxxxxxx> wrote:
> On 27 July 2016 at 17:59, Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> wrote:
>> On 2016/7/27 15:17, Baolin Wang wrote:
>>>
>>> Before issuing mmc_erase() function, users always have checked if it can
>>> erase with mmc_can_erase/trim/discard() function, thus remove the
>>> redundant
>>> erase checking in mmc_erase() function.
>>>
>>> This patch also optimizes the erase start/end sector alignment with
>>> round_up()/round_down() function, when erase command is MMC_ERASE_ARG.
>>>
>>> Signed-off-by: Baolin Wang <baolin.wang@xxxxxxxxxx>
>>> ---
>>> Changes since v1:
>>>  - Add the alignment if card->erase_size is not power of 2.
>>> ---
>>>  drivers/mmc/core/core.c |   78
>>> +++++++++++++++++++++++++++++------------------
>>>  1 file changed, 48 insertions(+), 30 deletions(-)
>>>
>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c
>>> index b4c08d1a..303a917 100644
>>> --- a/drivers/mmc/core/core.c
>>> +++ b/drivers/mmc/core/core.c
>>> @@ -2195,6 +2195,51 @@ out:
>>>         return err;
>>>  }
>>>
>>> +static unsigned int mmc_align_erase_size(struct mmc_card *card,
>>> +                                        unsigned int *from,
>>> +                                        unsigned int *to,
>>> +                                        unsigned int nr)
>>> +{
>>> +       unsigned int from_new = *from, nr_new = nr, rem;
>>> +
>>> +       if (is_power_of_2(card->erase_size)) {
>>> +               unsigned int temp = from_new;
>>> +
>>> +               from_new = round_up(temp, card->erase_size);
>>> +               rem = from_new - temp;
>>> +
>>> +               if (nr_new > rem)
>>> +                       nr_new -= rem;
>>> +               else
>>> +                       return 0;
>>> +
>>> +               nr_new = round_down(nr_new, card->erase_size);
>>> +       } else {
>>> +               rem = from_new % card->erase_size;
>>> +               if (rem) {
>>> +                       rem = card->erase_size - rem;
>>> +                       from_new += rem;
>>> +                       if (nr_new > rem)
>>> +                               nr_new -= rem;
>>> +                       else
>>> +                               return 0;
>>> +               }
>>> +
>>> +               rem = nr_new % card->erase_size;
>>> +               if (rem)
>>> +                       nr_new -= rem;
>>> +       }
>>> +
>>> +       if (nr_new == 0)
>>> +               return 0;
>>> +
>>> +       /* 'from' and 'to' are inclusive */
>>> +       *to = from_new + nr_new - 1;
>>> +       *from = from_new;
>>> +
>>> +       return nr_new;
>>> +}
>>> +
>>>  /**
>>>   * mmc_erase - erase sectors.
>>>   * @card: card to erase
>>> @@ -2210,13 +2255,6 @@ int mmc_erase(struct mmc_card *card, unsigned int
>>> from, unsigned int nr,
>>>         unsigned int rem, to = from + nr;
>>>         int err;
>>>
>>> -       if (!(card->host->caps & MMC_CAP_ERASE) ||
>>> -           !(card->csd.cmdclass & CCC_ERASE))
>>
>>
>> Why remove the check , "!(card->csd.cmdclass & CCC_ERASE))"?
>
> Cause we always issue mmc_can_erase() function before strating to do
> mmc erase, so these looks like redundant.

Do you have any other comments about this patch? Thanks.

>
>>> -               return -EOPNOTSUPP;
>>> -
>>> -       if (!card->erase_size)
>>> -               return -EOPNOTSUPP;
>>> -
>>>         if (mmc_card_sd(card) && arg != MMC_ERASE_ARG)
>>>                 return -EOPNOTSUPP;
>>>
>>> @@ -2234,31 +2272,11 @@ int mmc_erase(struct mmc_card *card, unsigned int
>>> from, unsigned int nr,
>>>         }
>>>
>>>         if (arg == MMC_ERASE_ARG) {
>>> -               rem = from % card->erase_size;
>>> -               if (rem) {
>>> -                       rem = card->erase_size - rem;
>>> -                       from += rem;
>>> -                       if (nr > rem)
>>> -                               nr -= rem;
>>> -                       else
>>> -                               return 0;
>>> -               }
>>> -               rem = nr % card->erase_size;
>>> -               if (rem)
>>> -                       nr -= rem;
>>> +               rem = mmc_align_erase_size(card, &from, &to, nr);
>>> +               if (rem == 0)
>>> +                       return 0;
>>>         }
>>>
>>> -       if (nr == 0)
>>> -               return 0;
>>> -
>>> -       to = from + nr;
>>> -
>>> -       if (to <= from)
>>> -               return -EINVAL;
>>> -
>>> -       /* 'from' and 'to' are inclusive */
>>> -       to -= 1;
>>> -
>>>         /*
>>>          * Special case where only one erase-group fits in the timeout
>>> budget:
>>>          * If the region crosses an erase-group boundary on this
>>> particular
>>>
>>
>>
>> --
>> Best Regards
>> Shawn Lin
>>
>> --
>> 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
>
>
>
> --
> Baolin.wang
> Best Regards



-- 
Baolin.wang
Best Regards