[PATCH V5 2/6] nvmet: add lba to sect coversion helpers

[Chaitanya Kulkarni]

On 12/10/20 16:50, Damien Le Moal wrote:
> On 2020/12/10 15:27, Chaitanya Kulkarni wrote:
>> In this preparation patch we add helpers to convert lbas to sectors and
>> sectors to lba. This is needed to eliminate code duplication in the ZBD
>> backend.
>>
>> Use these helpers in the block device backennd.
> s/backennd/backend
>
> And in the title:
>
> s/coversion/conversion
>
> scripts/checkpatch.pl does spell checking...
I'll double check, it did not spit any warningand I rely on checkpatch.

zbd-nvmeof/v5/0002-nvmet-add-lba-to-sect-coversion-helpers.patch
----------------------------------------------------------------
total: 0 errors, 0 warnings, 40 lines checked

zbd-nvmeof/v5/0002-nvmet-add-lba-to-sect-coversion-helpers.patch has no
obvious style problems and is ready for submission.

>
>> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni at wdc.com>
>> ---
>>  drivers/nvme/target/io-cmd-bdev.c |  8 +++-----
>>  drivers/nvme/target/nvmet.h       | 10 ++++++++++
>>  2 files changed, 13 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/nvme/target/io-cmd-bdev.c b/drivers/nvme/target/io-cmd-bdev.c
>> index 125dde3f410e..23095bdfce06 100644
>> --- a/drivers/nvme/target/io-cmd-bdev.c
>> +++ b/drivers/nvme/target/io-cmd-bdev.c
>> @@ -256,8 +256,7 @@ static void nvmet_bdev_execute_rw(struct nvmet_req *req)
>>  	if (is_pci_p2pdma_page(sg_page(req->sg)))
>>  		op |= REQ_NOMERGE;
>>  
>> -	sector = le64_to_cpu(req->cmd->rw.slba);
>> -	sector <<= (req->ns->blksize_shift - 9);
>> +	sector = nvmet_lba_to_sect(req->ns, req->cmd->rw.slba);
>>  
>>  	if (req->transfer_len <= NVMET_MAX_INLINE_DATA_LEN) {
>>  		bio = &req->b.inline_bio;
>> @@ -345,7 +344,7 @@ static u16 nvmet_bdev_discard_range(struct nvmet_req *req,
>>  	int ret;
>>  
>>  	ret = __blkdev_issue_discard(ns->bdev,
>> -			le64_to_cpu(range->slba) << (ns->blksize_shift - 9),
>> +			nvmet_lba_to_sect(ns, range->slba),
>>  			le32_to_cpu(range->nlb) << (ns->blksize_shift - 9),
>>  			GFP_KERNEL, 0, bio);
>>  	if (ret && ret != -EOPNOTSUPP) {
>> @@ -414,8 +413,7 @@ static void nvmet_bdev_execute_write_zeroes(struct nvmet_req *req)
>>  	if (!nvmet_check_transfer_len(req, 0))
>>  		return;
>>  
>> -	sector = le64_to_cpu(write_zeroes->slba) <<
>> -		(req->ns->blksize_shift - 9);
>> +	sector = nvmet_lba_to_sect(req->ns, write_zeroes->slba);
>>  	nr_sector = (((sector_t)le16_to_cpu(write_zeroes->length) + 1) <<
>>  		(req->ns->blksize_shift - 9));
>>  
>> diff --git a/drivers/nvme/target/nvmet.h b/drivers/nvme/target/nvmet.h
>> index 592763732065..4cb4cdae858c 100644
>> --- a/drivers/nvme/target/nvmet.h
>> +++ b/drivers/nvme/target/nvmet.h
>> @@ -603,4 +603,14 @@ static inline bool nvmet_ns_has_pi(struct nvmet_ns *ns)
>>  	return ns->pi_type && ns->metadata_size == sizeof(struct t10_pi_tuple);
>>  }
>>  
>> +static inline u64 nvmet_sect_to_lba(struct nvmet_ns *ns, sector_t sect)
>> +{
>> +	return sect >> (ns->blksize_shift - SECTOR_SHIFT);
>> +}
>> +
>> +static inline sector_t nvmet_lba_to_sect(struct nvmet_ns *ns, __le64 lba)
>> +{
>> +	return le64_to_cpu(lba) << (ns->blksize_shift - SECTOR_SHIFT);
>> +}
>> +
> One thing here that I am not a fan of is the inconsistency between the 2 helpers
> regarding the LBA endianess: nvmet_lba_to_sect() takes LE lba value, but
> nvmet_sect_to_lba() returns a CPU endian lba value. Can't we unify them to work
> on CPU endian values, and then if needed add another helper like:
>
> static inline sector_t nvmet_lelba_to_sect(struct nvmet_ns *ns, __le64 lba)
> {
> 	return nvmet_lba_to_sect(ns, le64_to_cpu(lba));
> }
>
I'll add the endian version of the nvmet_sect_to_lba() which matches the
nvmet_lba_to_sect() that is anyways needed to get rid of the
cpu_to_le64() calls
in the zns patch.
>>  #endif /* _NVMET_H */
>>
>