diff mbox series

[v2] mmc: core: Use GFP_NOIO in ACMD22

Message ID 20241018052901.446638-1-avri.altman@wdc.com
State Superseded
Headers show
Series [v2] mmc: core: Use GFP_NOIO in ACMD22 | expand

Commit Message

Avri Altman Oct. 18, 2024, 5:29 a.m. UTC
While reviewing the SDUC series, Adrian made a comment concerning the
memory allocation code in mmc_sd_num_wr_blocks() - see [1].
Prevent memory allocations from triggering I/O operations while ACMD22
is in progress.

[1] https://www.spinics.net/lists/linux-mmc/msg82199.html

Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
Signed-off-by: Avri Altman <avri.altman@wdc.com>
Cc: stable@vger.kernel.org

---
Changes since v1:
 - Move memalloc_noio_restore around (Adrian)
---
 drivers/mmc/core/block.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Adrian Hunter Oct. 21, 2024, 10:47 a.m. UTC | #1
On 18/10/24 08:29, Avri Altman wrote:
> While reviewing the SDUC series, Adrian made a comment concerning the
> memory allocation code in mmc_sd_num_wr_blocks() - see [1].
> Prevent memory allocations from triggering I/O operations while ACMD22
> is in progress.
> 
> [1] https://www.spinics.net/lists/linux-mmc/msg82199.html
> 
> Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
> Signed-off-by: Avri Altman <avri.altman@wdc.com>
> Cc: stable@vger.kernel.org

Some checkpatch warnings:

  WARNING: Use lore.kernel.org archive links when possible - see https://lore.kernel.org/lists.html
  #12: 
  [1] https://www.spinics.net/lists/linux-mmc/msg82199.html

  WARNING: The commit message has 'stable@', perhaps it also needs a 'Fixes:' tag?

  total: 0 errors, 2 warnings, 17 lines checked

Otherwise:

Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>

> 
> ---
> Changes since v1:
>  - Move memalloc_noio_restore around (Adrian)
> ---
>  drivers/mmc/core/block.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 04f3165cf9ae..a813fd7f39cc 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -995,6 +995,8 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
>  	u32 result;
>  	__be32 *blocks;
>  	u8 resp_sz = mmc_card_ult_capacity(card) ? 8 : 4;
> +	unsigned int noio_flag;
> +
>  	struct mmc_request mrq = {};
>  	struct mmc_command cmd = {};
>  	struct mmc_data data = {};
> @@ -1018,7 +1020,9 @@ static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
>  	mrq.cmd = &cmd;
>  	mrq.data = &data;
>  
> +	noio_flag = memalloc_noio_save();
>  	blocks = kmalloc(resp_sz, GFP_KERNEL);
> +	memalloc_noio_restore(noio_flag);
>  	if (!blocks)
>  		return -ENOMEM;
>
Avri Altman Oct. 21, 2024, 12:24 p.m. UTC | #2
> 
> On 18/10/24 08:29, Avri Altman wrote:
> > While reviewing the SDUC series, Adrian made a comment concerning the
> > memory allocation code in mmc_sd_num_wr_blocks() - see [1].
> > Prevent memory allocations from triggering I/O operations while ACMD22
> > is in progress.
> >
> > [1] https://www.spinics.net/lists/linux-mmc/msg82199.html
> >
> > Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
> > Signed-off-by: Avri Altman <avri.altman@wdc.com>
> > Cc: stable@vger.kernel.org
> 
> Some checkpatch warnings:
> 
>   WARNING: Use lore.kernel.org archive links when possible - see
> https://lore.kernel.org/lists.html
>   #12:
>   [1] https://www.spinics.net/lists/linux-mmc/msg82199.html
Done.

> 
>   WARNING: The commit message has 'stable@', perhaps it also needs a
> 'Fixes:' tag?
I tried to look for the patch that introduced mmc_sd_num_wr_blocks but couldn't find it in Ulf's tree.

Thanks,
Avri
> 
>   total: 0 errors, 2 warnings, 17 lines checked
> 
> Otherwise:
> 
> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
> 
> >
> > ---
> > Changes since v1:
> >  - Move memalloc_noio_restore around (Adrian)
> > ---
> >  drivers/mmc/core/block.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
> > 04f3165cf9ae..a813fd7f39cc 100644
> > --- a/drivers/mmc/core/block.c
> > +++ b/drivers/mmc/core/block.c
> > @@ -995,6 +995,8 @@ static int mmc_sd_num_wr_blocks(struct mmc_card
> *card, u32 *written_blocks)
> >       u32 result;
> >       __be32 *blocks;
> >       u8 resp_sz = mmc_card_ult_capacity(card) ? 8 : 4;
> > +     unsigned int noio_flag;
> > +
> >       struct mmc_request mrq = {};
> >       struct mmc_command cmd = {};
> >       struct mmc_data data = {};
> > @@ -1018,7 +1020,9 @@ static int mmc_sd_num_wr_blocks(struct
> mmc_card *card, u32 *written_blocks)
> >       mrq.cmd = &cmd;
> >       mrq.data = &data;
> >
> > +     noio_flag = memalloc_noio_save();
> >       blocks = kmalloc(resp_sz, GFP_KERNEL);
> > +     memalloc_noio_restore(noio_flag);
> >       if (!blocks)
> >               return -ENOMEM;
> >
Adrian Hunter Oct. 21, 2024, 2:33 p.m. UTC | #3
On 21/10/24 15:24, Avri Altman wrote:
>>
>> On 18/10/24 08:29, Avri Altman wrote:
>>> While reviewing the SDUC series, Adrian made a comment concerning the
>>> memory allocation code in mmc_sd_num_wr_blocks() - see [1].
>>> Prevent memory allocations from triggering I/O operations while ACMD22
>>> is in progress.
>>>
>>> [1] https://www.spinics.net/lists/linux-mmc/msg82199.html
>>>
>>> Suggested-by: Adrian Hunter <adrian.hunter@intel.com>
>>> Signed-off-by: Avri Altman <avri.altman@wdc.com>
>>> Cc: stable@vger.kernel.org
>>
>> Some checkpatch warnings:
>>
>>   WARNING: Use lore.kernel.org archive links when possible - see
>> https://lore.kernel.org/lists.html
>>   #12:
>>   [1] https://www.spinics.net/lists/linux-mmc/msg82199.html
> Done.
> 
>>
>>   WARNING: The commit message has 'stable@', perhaps it also needs a
>> 'Fixes:' tag?
> I tried to look for the patch that introduced mmc_sd_num_wr_blocks but couldn't find it in Ulf's tree.

Seems like the following introduced the kmalloc()

	commit 051913dada046ac948eb6f48c0717fc25de2a917
	Author: Ben Dooks <ben@simtec.co.uk>
	Date:   Mon Jun 8 23:33:57 2009 +0100

	    mmc_block: do not DMA to stack

> 
> Thanks,
> Avri
>>
>>   total: 0 errors, 2 warnings, 17 lines checked
>>
>> Otherwise:
>>
>> Reviewed-by: Adrian Hunter <adrian.hunter@intel.com>
>>
>>>
>>> ---
>>> Changes since v1:
>>>  - Move memalloc_noio_restore around (Adrian)
>>> ---
>>>  drivers/mmc/core/block.c | 4 ++++
>>>  1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c index
>>> 04f3165cf9ae..a813fd7f39cc 100644
>>> --- a/drivers/mmc/core/block.c
>>> +++ b/drivers/mmc/core/block.c
>>> @@ -995,6 +995,8 @@ static int mmc_sd_num_wr_blocks(struct mmc_card
>> *card, u32 *written_blocks)
>>>       u32 result;
>>>       __be32 *blocks;
>>>       u8 resp_sz = mmc_card_ult_capacity(card) ? 8 : 4;
>>> +     unsigned int noio_flag;
>>> +
>>>       struct mmc_request mrq = {};
>>>       struct mmc_command cmd = {};
>>>       struct mmc_data data = {};
>>> @@ -1018,7 +1020,9 @@ static int mmc_sd_num_wr_blocks(struct
>> mmc_card *card, u32 *written_blocks)
>>>       mrq.cmd = &cmd;
>>>       mrq.data = &data;
>>>
>>> +     noio_flag = memalloc_noio_save();
>>>       blocks = kmalloc(resp_sz, GFP_KERNEL);
>>> +     memalloc_noio_restore(noio_flag);
>>>       if (!blocks)
>>>               return -ENOMEM;
>>>
>
Avri Altman Oct. 21, 2024, 3:26 p.m. UTC | #4
> >>   WARNING: The commit message has 'stable@', perhaps it also needs a
> >> 'Fixes:' tag?
> > I tried to look for the patch that introduced mmc_sd_num_wr_blocks but
> couldn't find it in Ulf's tree.
> 
> Seems like the following introduced the kmalloc()
> 
>         commit 051913dada046ac948eb6f48c0717fc25de2a917
>         Author: Ben Dooks <ben@simtec.co.uk>
>         Date:   Mon Jun 8 23:33:57 2009 +0100
> 
>             mmc_block: do not DMA to stack
Got it.

Thanks,
Avri
diff mbox series

Patch

diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
index 04f3165cf9ae..a813fd7f39cc 100644
--- a/drivers/mmc/core/block.c
+++ b/drivers/mmc/core/block.c
@@ -995,6 +995,8 @@  static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
 	u32 result;
 	__be32 *blocks;
 	u8 resp_sz = mmc_card_ult_capacity(card) ? 8 : 4;
+	unsigned int noio_flag;
+
 	struct mmc_request mrq = {};
 	struct mmc_command cmd = {};
 	struct mmc_data data = {};
@@ -1018,7 +1020,9 @@  static int mmc_sd_num_wr_blocks(struct mmc_card *card, u32 *written_blocks)
 	mrq.cmd = &cmd;
 	mrq.data = &data;
 
+	noio_flag = memalloc_noio_save();
 	blocks = kmalloc(resp_sz, GFP_KERNEL);
+	memalloc_noio_restore(noio_flag);
 	if (!blocks)
 		return -ENOMEM;