linux-kernel - Re: [PATCHv2] mmc: block: Add single read for 4k sector cards

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <e0cce326393645d3b4a163ce65c89fb9@hyperstone.com>
Date:   Tue, 28 Jun 2022 09:08:38 +0000
From:   Christian Löhle <CLoehle@...erstone.com>
To:     Adrian Hunter <adrian.hunter@...el.com>,
        Avri Altman <Avri.Altman@....com>,
        "ulf.hansson@...aro.org" <ulf.hansson@...aro.org>,
        "linux-mmc@...r.kernel.org" <linux-mmc@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv2] mmc: block: Add single read for 4k sector cards

> Cards with 4k native sector size may only be read 4k-aligned,
>> accommodate for this in the single read recovery and use it.
>
>Thanks for the patch.
>
>> 
>> Fixes: 81196976ed946 (mmc: block: Add blk-mq support)
>> Signed-off-by: Christian Loehle <cloehle@...erstone.com>
>
>FYI checkpatch says:
>
>WARNING: From:/Signed-off-by: email name mismatch: 'From: "Christian Löhle" <CLoehle@...erstone.com>' != 'Signed-off-by: Christian Loehle <cloehle@...erstone.com>'

Will be fixed in my future patches, thanks for the hint.

>
>> ---
>>  drivers/mmc/core/block.c | 25 ++++++++++++-------------
>>  1 file changed, 12 insertions(+), 13 deletions(-)
>> 
>> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
>> index f4a1281658db..a75a208ce203 100644
>> --- a/drivers/mmc/core/block.c
>> +++ b/drivers/mmc/core/block.c
>> @@ -176,7 +176,7 @@ static inline int mmc_blk_part_switch(struct mmc_card *card,
>>  				      unsigned int part_type);
>>  static void mmc_blk_rw_rq_prep(struct mmc_queue_req *mqrq,
>>  			       struct mmc_card *card,
>> -			       int disable_multi,
>> +			       int recovery_mode,
>>  			       struct mmc_queue *mq);
>>  static void mmc_blk_hsq_req_done(struct mmc_request *mrq);
>>  
>> @@ -1302,7 +1302,7 @@ static void mmc_blk_eval_resp_error(struct mmc_blk_request *brq)
>>  }
>>  
>>  static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
>> -			      int disable_multi, bool *do_rel_wr_p,
>> +			      int recovery_mode, bool *do_rel_wr_p,
>>  			      bool *do_data_tag_p)
>>  {
>>  	struct mmc_blk_data *md = mq->blkdata;
>> @@ -1372,8 +1372,8 @@ static void mmc_blk_data_prep(struct mmc_queue *mq, struct mmc_queue_req *mqrq,
>>  		 * at a time in order to accurately determine which
>>  		 * sectors can be read successfully.
>>  		 */
>> -		if (disable_multi)
>> -			brq->data.blocks = 1;
>> +		if (recovery_mode)
>> +			brq->data.blocks = mmc_large_sector(card) ? 8 : 1;
>
>I suggest changing to use queue_physical_block_size() here and further below

This part I'm impartial about, not sure if it makes it more readable, hopefully we never have to support another "native sector size" apart from the two.
Anyway I will send the next patch with queue_physical_block_size()

>
>			brq->data.blocks = queue_physical_block_size(req->q) >> SECTOR_SHIFT;

Do we want to switch to SECTOR_SHIFT instead of 9? So far SECTOR_SHIFT is not used at all in mmc core.
If so I would go ahead and change all the others in another patch:
queue.c:187:	q->limits.discard_granularity = card->pref_erase << 9;
core.c:103:	data->bytes_xfered = (prandom_u32() % (data->bytes_xfered >> 9)) << 9;
mmc.c:792:MMC_DEV_ATTR(erase_size, "%u\n", card->erase_size << 9);
mmc.c:793:MMC_DEV_ATTR(preferred_erase_size, "%u\n", card->pref_erase << 9);
mmc_test.c:1557:	sz = (unsigned long)test->card->pref_erase << 9;
mmc_test.c:1570:		t->max_tfr = test->card->host->max_blk_count << 9;
mmc_test.c:2461:	if (repeat_cmd && (t->blocks + 1) << 9 > t->max_tfr)
sd.c:707:MMC_DEV_ATTR(erase_size, "%u\n", card->erase_size << 9);
sd.c:708:MMC_DEV_ATTR(preferred_erase_size, "%u\n", card->pref_erase << 9);
block.c:1417:		int i, data_size = brq->data.blocks << 9;
block.c:1851:			brq->data.bytes_xfered = blocks << 9;




Hyperstone GmbH | Reichenaustr. 39a  | 78467 Konstanz
Managing Director: Dr. Jan Peter Berns.
Commercial register of local courts: Freiburg HRB381782