linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Satya Tangirala <satyat@google.com>
To: Bart Van Assche <bvanassche@acm.org>
Cc: linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jens Axboe <axboe@kernel.dk>, Eric Biggers <ebiggers@google.com>
Subject: Re: [PATCH v2 0/8] ensure bios aren't split in middle of crypto data unit
Date: Fri, 26 Mar 2021 01:39:58 +0000	[thread overview]
Message-ID: <YF07bqQ1NvPxPNrh@google.com> (raw)
In-Reply-To: <e0248d93-e880-6a6d-92d6-dfcfb6f9d661@acm.org>

On Thu, Mar 25, 2021 at 02:51:31PM -0700, Bart Van Assche wrote:
> On 3/25/21 2:26 PM, Satya Tangirala wrote:
> > When a bio has an encryption context, its size must be aligned to its
> > crypto data unit size. A bio must not be split in the middle of a data
> > unit. Currently, bios are split at logical block boundaries [...]
> 
> Hi Satya,
> 
> Are you sure that the block layer core splits bios at logical block
> boundaries? Commit 9cc5169cd478 ("block: Improve physical block
> alignment of split bios") should have changed the behavior from
> splitting at logical block boundaries into splitting at physical block
> boundaries.
Ah, what I really meant with that sentence was "Currently, if a bio is
split, the size of the resulting bio is guaranteed to be aligned to a
the lbs. The endpoint of the bio might also be aligned to a physical
block boundary (which 9cc5169cd478 tries to achieve if possible), but
the bio's size (and hence also its endpoint since the start of the bio
is always lbs aligned) is *at least* lbs aligned". Does that sound
accurate?

With inline encryption, that lbs alignment guarantee is no longer
enough - the bio now needs to have a size that's aligned to the bio's
data unit size (which may be larger than the logical block size, but
is also still a power of 2).
> Without having looked at this patch series, can the same
> effect be achieved by reporting the crypto data unit size as the
> physical block size?
That would've been awesome, but I don't think we can do that :(
1) There isn't only one crypto data unit size. A device can support,
   and upper layers are free to use, many different data unit sizes.
2) IIUC 9cc5169cd478 (or more accurately get_max_io_size() since the
   function has been changed slightly since your original patch)
   doesn't align the size of the bio to the pbs - it only aligns the
   endpoint of the bio to the pbs (and it may actually not even do
   that if it turns out to not be possible). Is that right? If so,
   that means that if the startpoint of the bio isn't pbs aligned, the
   size of the bio won't be pbs aligned either.

> 
> Thanks,
> 
> Bart.

  reply	other threads:[~2021-03-26  1:40 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-25 21:26 [PATCH v2 0/8] ensure bios aren't split in middle of crypto data unit Satya Tangirala
2021-03-25 21:26 ` [PATCH v2 1/8] block: introduce blk_ksm_is_empty() Satya Tangirala
2021-03-30 18:04   ` Christoph Hellwig
2021-04-15 19:21   ` Eric Biggers
2021-03-25 21:26 ` [PATCH v2 2/8] dm,mmc,ufshcd: handle error from blk_ksm_register() Satya Tangirala
2021-04-15 19:27   ` Eric Biggers
2021-03-25 21:26 ` [PATCH v2 3/8] block: blk-crypto: introduce blk_crypto_bio_sectors_alignment() Satya Tangirala
2021-04-15 19:33   ` Eric Biggers
2021-03-25 21:26 ` [PATCH v2 4/8] block: introduce bio_required_sector_alignment() Satya Tangirala
2021-03-30 18:06   ` Christoph Hellwig
2021-04-15 19:37     ` Eric Biggers
2021-04-15 19:44   ` Eric Biggers
2021-03-25 21:26 ` [PATCH v2 5/8] block: respect bio_required_sector_alignment() in blk-crypto-fallback Satya Tangirala
2021-04-15 19:45   ` Eric Biggers
2021-03-25 21:26 ` [PATCH v2 6/8] block: keyslot-manager: introduce blk_ksm_restrict_dus_to_queue_limits() Satya Tangirala
2021-03-26  3:50   ` kernel test robot
2021-03-26  6:28   ` kernel test robot
2021-04-15 19:55   ` Eric Biggers
2021-03-25 21:26 ` [PATCH v2 7/8] blk-merge: Ensure bios aren't split in middle of a crypto data unit Satya Tangirala
2021-03-25 21:26 ` [PATCH v2 8/8] block: add WARN() in bio_split() for sector alignment Satya Tangirala
2021-03-26  3:47   ` Bart Van Assche
2021-04-15 20:00   ` Eric Biggers
2021-03-25 21:51 ` [PATCH v2 0/8] ensure bios aren't split in middle of crypto data unit Bart Van Assche
2021-03-26  1:39   ` Satya Tangirala [this message]
2021-03-26  3:46     ` Bart Van Assche
2021-03-26  7:56       ` Satya Tangirala

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=YF07bqQ1NvPxPNrh@google.com \
    --to=satyat@google.com \
    --cc=axboe@kernel.dk \
    --cc=bvanassche@acm.org \
    --cc=ebiggers@google.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).