diff mbox series

[v2,8/8] block: add WARN() in bio_split() for sector alignment

Message ID 20210325212609.492188-9-satyat@google.com (mailing list archive)
State New, archived
Headers show
Series ensure bios aren't split in middle of crypto data unit | expand

Commit Message

Satya Tangirala March 25, 2021, 9:26 p.m. UTC
The number of sectors passed to bio_split() should be aligned to
bio_required_sector_alignment(). All callers (other than bounce.c) have
been updated to ensure this, so add a WARN() if the number of sectors is
not aligned. (bounce.c was not updated since it's legacy code that
won't interact with inline encryption).

Signed-off-by: Satya Tangirala <satyat@google.com>
---
 block/bio.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Bart Van Assche March 26, 2021, 3:47 a.m. UTC | #1
On 3/25/21 2:26 PM, Satya Tangirala wrote:
> @@ -1458,6 +1458,7 @@ struct bio *bio_split(struct bio *bio, int sectors,
>  
>  	BUG_ON(sectors <= 0);
>  	BUG_ON(sectors >= bio_sectors(bio));
> +	WARN_ON(!IS_ALIGNED(sectors, bio_required_sector_alignment(bio)));

Please change this WARN_ON() into WARN_ON_ONCE() to prevent a storm of
call traces in the kernel log if this warning statement would be hit
repeatedly.

Thanks,

Bart.
Eric Biggers April 15, 2021, 8 p.m. UTC | #2
On Thu, Mar 25, 2021 at 09:26:09PM +0000, Satya Tangirala wrote:
> The number of sectors passed to bio_split() should be aligned to
> bio_required_sector_alignment(). All callers (other than bounce.c) have
> been updated to ensure this, so add a WARN() if the number of sectors is
> not aligned. (bounce.c was not updated since it's legacy code that
> won't interact with inline encryption).

What does bounce.c "won't interact with inline encryption" mean, exactly?
Devices that enable bounce buffering won't declare inline encryption support, so
bounce.c will never see a bio that has an encryption context?

> diff --git a/block/bio.c b/block/bio.c
> index 26b7f721cda8..cb348f134a15 100644
> --- a/block/bio.c
> +++ b/block/bio.c
> @@ -1458,6 +1458,7 @@ struct bio *bio_split(struct bio *bio, int sectors,
>  
>  	BUG_ON(sectors <= 0);
>  	BUG_ON(sectors >= bio_sectors(bio));
> +	WARN_ON(!IS_ALIGNED(sectors, bio_required_sector_alignment(bio)));

If this warning triggers, shouldn't the function return NULL to indicate a
failure rather than proceeding on?

- Eric
diff mbox series

Patch

diff --git a/block/bio.c b/block/bio.c
index 26b7f721cda8..cb348f134a15 100644
--- a/block/bio.c
+++ b/block/bio.c
@@ -1458,6 +1458,7 @@  struct bio *bio_split(struct bio *bio, int sectors,
 
 	BUG_ON(sectors <= 0);
 	BUG_ON(sectors >= bio_sectors(bio));
+	WARN_ON(!IS_ALIGNED(sectors, bio_required_sector_alignment(bio)));
 
 	/* Zone append commands cannot be split */
 	if (WARN_ON_ONCE(bio_op(bio) == REQ_OP_ZONE_APPEND))