| Message ID | e1c7ed59e2d2b69567ef2d9925fa997ecb7b4822.1685461490.git.johannes.thumshirn@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | bio: check return values of bio_add_page | expand |
On Tue, May 30 2023 at 11:49P -0400, Johannes Thumshirn <johannes.thumshirn@wdc.com> wrote: > Check if adding pages to clone bio fails and if it does retry with > reclaim. This mirrors the behaviour of page allocation in > crypt_alloc_buffer(). Nope. > This way we can mark bio_add_pages as __must_check. > > Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> > Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> The above patch header doesn't reflect the code. I also think __bio_add_page should be used, like my racey reply to Mikulas vs your v6 patchbomb said, please see: https://listman.redhat.com/archives/dm-devel/2023-May/054388.html Thanks, Mike > --- > drivers/md/dm-crypt.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c > index 8b47b913ee83..0dd231e61757 100644 > --- a/drivers/md/dm-crypt.c > +++ b/drivers/md/dm-crypt.c > @@ -1693,7 +1693,10 @@ static struct bio *crypt_alloc_buffer(struct dm_crypt_io *io, unsigned int size) > > len = (remaining_size > PAGE_SIZE) ? PAGE_SIZE : remaining_size; > > - bio_add_page(clone, page, len, 0); > + if (!bio_add_page(clone, page, len, 0)) { > + WARN_ONCE(1, "Adding page to bio failed\n"); > + return NULL; > + } > > remaining_size -= len; > } > -- > 2.40.1 > > -- > dm-devel mailing list > dm-devel@redhat.com > https://listman.redhat.com/mailman/listinfo/dm-devel >
On 30.05.23 18:10, Mike Snitzer wrote: > On Tue, May 30 2023 at 11:49P -0400, > Johannes Thumshirn <johannes.thumshirn@wdc.com> wrote: > >> Check if adding pages to clone bio fails and if it does retry with >> reclaim. This mirrors the behaviour of page allocation in >> crypt_alloc_buffer(). > > Nope. > >> This way we can mark bio_add_pages as __must_check. >> >> Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> >> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com> > > The above patch header doesn't reflect the code. > > I also think __bio_add_page should be used, like my racey reply to > Mikulas vs your v6 patchbomb said, please see: > https://listman.redhat.com/archives/dm-devel/2023-May/054388.html Yep that mail was racing with my send of v6. I can send out a v7 of the series tomorrow or just that one patch updated. Whatever Jens preferes.
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c index 8b47b913ee83..0dd231e61757 100644 --- a/drivers/md/dm-crypt.c +++ b/drivers/md/dm-crypt.c @@ -1693,7 +1693,10 @@ static struct bio *crypt_alloc_buffer(struct dm_crypt_io *io, unsigned int size) len = (remaining_size > PAGE_SIZE) ? PAGE_SIZE : remaining_size; - bio_add_page(clone, page, len, 0); + if (!bio_add_page(clone, page, len, 0)) { + WARN_ONCE(1, "Adding page to bio failed\n"); + return NULL; + } remaining_size -= len; }