| Message ID | 20210224072407.46363-5-hch@lst.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] block-crypto-fallback: use a bio_set for splitting bios | expand |
On Wed, Feb 24, 2021 at 08:24:07AM +0100, Christoph Hellwig wrote: > The caller can't cope with a failure from bounce_clone_bio, so > use __GFP_NOFAIL for the passthrough case. bio_alloc_bioset already > won't fail due to the use of mempools. > > And yes, we need to get rid of this bock layer bouncing code entirely > sooner or later.. > > Signed-off-by: Christoph Hellwig <hch@lst.de> > --- > block/bounce.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/block/bounce.c b/block/bounce.c > index 417faaac36b691..87983a35079c22 100644 > --- a/block/bounce.c > +++ b/block/bounce.c > @@ -242,12 +242,11 @@ static struct bio *bounce_clone_bio(struct bio *bio_src) > * __bio_clone_fast() anyways. > */ > if (bio_is_passthrough(bio_src)) > - bio = bio_kmalloc(GFP_NOIO, bio_segments(bio_src)); > + bio = bio_kmalloc(GFP_NOIO | __GFP_NOFAIL, > + bio_segments(bio_src)); bio_kmalloc() still may fail if bio_segments(bio_src) is > UIO_MAXIOV.
On Wed, Feb 24, 2021 at 07:19:52PM +0800, Ming Lei wrote: > > if (bio_is_passthrough(bio_src)) > > - bio = bio_kmalloc(GFP_NOIO, bio_segments(bio_src)); > > + bio = bio_kmalloc(GFP_NOIO | __GFP_NOFAIL, > > + bio_segments(bio_src)); > > bio_kmalloc() still may fail if bio_segments(bio_src) is > UIO_MAXIOV. Yes, but bio_kmalloc is what is used to allocate the passthrough requests to start with, so we'd not even make it here.
On 2/23/21 23:29, Christoph Hellwig wrote: > The caller can't cope with a failure from bounce_clone_bio, so > use __GFP_NOFAIL for the passthrough case. bio_alloc_bioset already > won't fail due to the use of mempools. > > And yes, we need to get rid of this bock layer bouncing code entirely > sooner or later.. > > Signed-off-by: Christoph Hellwig <hch@lst.de> Looks good. Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com>
On Wed, Feb 24, 2021 at 05:12:36PM +0100, Christoph Hellwig wrote: > On Wed, Feb 24, 2021 at 07:19:52PM +0800, Ming Lei wrote: > > > if (bio_is_passthrough(bio_src)) > > > - bio = bio_kmalloc(GFP_NOIO, bio_segments(bio_src)); > > > + bio = bio_kmalloc(GFP_NOIO | __GFP_NOFAIL, > > > + bio_segments(bio_src)); > > > > bio_kmalloc() still may fail if bio_segments(bio_src) is > UIO_MAXIOV. > > Yes, but bio_kmalloc is what is used to allocate the passthrough > requests to start with, so we'd not even make it here. The original bio_kmalloc() may start with allowed nr_iovecs , but later more pages are retrieved from iov_iter and added to the bio, see bio_map_user_iov(). Then bounce_clone_bio() will see too big bio_segments(bio_src) to be held in UIO_MAXIOV vecs. This behavior is similar with blkdev_direct_IO().
diff --git a/block/bounce.c b/block/bounce.c index 417faaac36b691..87983a35079c22 100644 --- a/block/bounce.c +++ b/block/bounce.c @@ -242,12 +242,11 @@ static struct bio *bounce_clone_bio(struct bio *bio_src) * __bio_clone_fast() anyways. */ if (bio_is_passthrough(bio_src)) - bio = bio_kmalloc(GFP_NOIO, bio_segments(bio_src)); + bio = bio_kmalloc(GFP_NOIO | __GFP_NOFAIL, + bio_segments(bio_src)); else bio = bio_alloc_bioset(GFP_NOIO, bio_segments(bio_src), &bounce_bio_set); - if (!bio) - return NULL; bio->bi_bdev = bio_src->bi_bdev; if (bio_flagged(bio_src, BIO_REMAPPED)) bio_set_flag(bio, BIO_REMAPPED);
The caller can't cope with a failure from bounce_clone_bio, so use __GFP_NOFAIL for the passthrough case. bio_alloc_bioset already won't fail due to the use of mempools. And yes, we need to get rid of this bock layer bouncing code entirely sooner or later.. Signed-off-by: Christoph Hellwig <hch@lst.de> --- block/bounce.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)