Thread (16 messages) 16 messages, 3 authors, 2021-04-20

Re: [PATCH v7 1/3] bio: limit bio max size

From: Changheun Lee <hidden>
Date: 2021-04-19 06:07:53
Also in: lkml

quoted
@@ -167,6 +168,7 @@ void blk_queue_max_hw_sectors(struct request_queue *q, unsigned int max_hw_secto
 	max_sectors = round_down(max_sectors,
 				 limits->logical_block_size >> SECTOR_SHIFT);
 	limits->max_sectors = max_sectors;
+	limits->bio_max_bytes = max_sectors << SECTOR_SHIFT;
 
 	q->backing_dev_info->io_pages = max_sectors >> (PAGE_SHIFT - 9);
 }
Can the new shift operation overflow? If so, how about using
check_shl_overflow()?
Actually, overflow might be not heppen in case of physical device.
But I modified as below. feedback about this.
@@ -168,6 +169,9 @@ void blk_queue_max_hw_sectors(struct request_queue *q, unsigned int max_hw_secto
 				 limits->logical_block_size >> SECTOR_SHIFT);
 	limits->max_sectors = max_sectors;
 
+	limits->bio_max_bytes = check_shl_overflow(max_sectors, SECTOR_SHIFT,
+		&limits->bio_max_bytes) ? UINT_MAX : max_sectors << SECTOR_SHIFT;
+
 	q->backing_dev_info->io_pages = max_sectors >> (PAGE_SHIFT - 9);
 }
 EXPORT_SYMBOL(blk_queue_max_hw_sectors);
quoted
quoted
quoted
@@ -538,6 +540,8 @@ int blk_stack_limits(struct queue_limits *t, struct queue_limits *b,
 {
 	unsigned int top, bottom, alignment, ret = 0;
 
+	t->bio_max_bytes = min_not_zero(t->bio_max_bytes, b->bio_max_bytes);
+
 	t->max_sectors = min_not_zero(t->max_sectors, b->max_sectors);
 	t->max_hw_sectors = min_not_zero(t->max_hw_sectors, b->max_hw_sectors);
 	t->max_dev_sectors = min_not_zero(t->max_dev_sectors, b->max_dev_sectors);
The above will limit bio_max_bytes for all stacked block devices, which
is something we do not want. I propose to set t->bio_max_bytes to
UINT_MAX in blk_stack_limits() and to let the stacked driver (e.g.
dm-crypt) decide whether or not to lower that value.
Actually, bio size should be limited in dm-crypt too. Because almost I/O
from user space will be gone to dm-crypt first. I/O issue timing will be
delayed if bio size is not limited in dm-crypt.
Do you have any idea to decide whether takes lower bio max size, or not
in the stacked driver?
Add a flag to decide this in driver layer like before?
Or insert code manually in each stacked driver if it is needed?
There will be fewer stacked drivers for which the bio size has to be
limited than for which the bio size has not to be limited. Hence the
proposal to set t->bio_max_bytes to UINT_MAX in blk_stack_limits() and
to let the stacked driver (e.g. dm-crypt) decide whether or not to lower
that value.
I see what you said. I'll set t->bio_max_bytes to UINT_MAX in
blk_stack_limits() as you mentioned.
quoted
diff --git a/include/linux/bio.h b/include/linux/bio.h
index d0246c92a6e8..e5add63da3af 100644
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -106,6 +106,8 @@ static inline void *bio_data(struct bio *bio)
 	return NULL;
 }
 
+extern unsigned int bio_max_size(struct bio *bio);
You may want to define bio_max_size() as an inline function in bio.h
such that no additional function calls are introduced in the hot path.
I tried, but it is not easy. because request_queue structure of blkdev.h
should be referred in bio.h. I think it's not good to apply as a inline function.


Thanks,

Changheun Lee.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help