All of lore.kernel.org
 help / color / mirror / Atom feed
From: Damien Le Moal <Damien.LeMoal@wdc.com>
To: "jejb@linux.ibm.com" <jejb@linux.ibm.com>,
	Bart Van Assche <bvanassche@acm.org>,
	"Martin K . Petersen" <martin.petersen@oracle.com>
Cc: Jaegeuk Kim <jaegeuk@kernel.org>, Bean Huo <beanhuo@micron.com>,
	Avri Altman <Avri.Altman@wdc.com>,
	Asutosh Das <asutoshd@codeaurora.org>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Can Guo <cang@codeaurora.org>,
	"linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>
Subject: Re: [PATCH v2 0/7] Rename scsi_get_lba() into scsi_get_pos()
Date: Thu, 13 May 2021 02:18:27 +0000	[thread overview]
Message-ID: <DM6PR04MB708186DD35EB12B26BC9CE60E7519@DM6PR04MB7081.namprd04.prod.outlook.com> (raw)
In-Reply-To: 0c2d87fde65e40f34914e7555d3971f7b2c8f28b.camel@linux.ibm.com

On 2021/05/13 9:14, James Bottomley wrote:
> On Wed, 2021-05-12 at 17:00 -0700, Bart Van Assche wrote:
>> On 5/12/21 4:23 PM, James Bottomley wrote:
>>> No, we support physical sector sizes up to 4k.  The logical block
>>> size internal to the kernel and the block layer is always 512.  I
>>> can see the utility in using consistent naming to the block layer,
>>> but I can't see that logical block address is confusing ...
>>> especially now manufacturers seem all to have aligned on 512 for
>>> the logical block size even when it's usually 4k physical.
>>
>> Are we talking about the same? Just below the code that I included in
>> my previous email there is the following line:
>>
>> 	blk_queue_logical_block_size(sdp->request_queue, sector_size);
>>
>> where sector_size is the logical block size reported by the READ 
>> CAPACITY command and has a value between 512 and 4096.
> 
> That was for devices from before the industry standardised, which are
> getting harder and harder to find (In fact I'm thinking of making a NFT
> out of my last 4k logical/physical disk).  But it didn't alter the fact
> that the kernel internal block size is 512.

struct bio and struct request use 512B sector_t unit addressing. So does the
entire block layer, file systems device mapper etc. SAll users of block devices
use this unit. Yes, that is fixed to 512B, regardless of the characteristics of
the target device. But to avoid confusion, we never refer to this as the
"logical block size" or "block size". We use the term "sector" and reserve the
term "block" for the device layer.

The logical block size (the unit used for command addressing) may or may not be
512B (it may or may not be equal to the block layer sector size). These days,
most HDDs are 512e, that is, 512B logical block size and 4K physical block size.
Lots of SSDs are still 512/512. 4K/4K HDDs and SSDs are gaining ground and
spreading.

I agree with Bart's cleanup patches. They correct a non-standard use of the term
LBA to refer to a value using the block layer sector unit.

Bart suggested scsi_get_pos() as the new function name to solve the confusion. I
think that using scsi_get_sector() as a name would be even clearer about the
unit of the values being handled.

-- 
Damien Le Moal
Western Digital Research

  reply	other threads:[~2021-05-13  2:18 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-12 20:08 [PATCH v2 0/7] Rename scsi_get_lba() into scsi_get_pos() Bart Van Assche
2021-05-12 20:08 ` [PATCH v2 1/7] Introduce scsi_get_pos() Bart Van Assche
2021-05-12 20:08 ` [PATCH v2 2/7] iser: Use scsi_get_pos() instead of scsi_get_lba() Bart Van Assche
2021-05-12 20:46   ` Sagi Grimberg
2021-05-12 20:08 ` [PATCH v2 3/7] zfcp: " Bart Van Assche
2021-05-12 20:08 ` [PATCH v2 4/7] isci: " Bart Van Assche
2021-05-12 20:08 ` [PATCH v2 5/7] qla2xxx: " Bart Van Assche
2021-05-12 20:08 ` [PATCH v2 6/7] ufs: Fix the tracing code Bart Van Assche
2021-05-12 20:08 ` [PATCH v2 7/7] Remove scsi_get_lba() Bart Van Assche
2021-05-12 22:10 ` [PATCH v2 0/7] Rename scsi_get_lba() into scsi_get_pos() James Bottomley
2021-05-12 22:20   ` Bart Van Assche
2021-05-12 23:23     ` James Bottomley
2021-05-13  0:00       ` Bart Van Assche
2021-05-13  0:11         ` James Bottomley
2021-05-13  2:18           ` Damien Le Moal [this message]
2021-05-13  5:42             ` James Bottomley
2021-05-13  6:10               ` Damien Le Moal

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DM6PR04MB708186DD35EB12B26BC9CE60E7519@DM6PR04MB7081.namprd04.prod.outlook.com \
    --to=damien.lemoal@wdc.com \
    --cc=Avri.Altman@wdc.com \
    --cc=asutoshd@codeaurora.org \
    --cc=beanhuo@micron.com \
    --cc=bvanassche@acm.org \
    --cc=cang@codeaurora.org \
    --cc=jaegeuk@kernel.org \
    --cc=jejb@linux.ibm.com \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.