[v2,00/19] bio: check return values of bio_add_page

Message

Johannes Thumshirn March 30, 2023, 10:43 a.m. UTC

We have two functions for adding a page to a bio, __bio_add_page() which is
used to add a single page to a freshly created bio and bio_add_page() which is
used to add a page to an existing bio.

While __bio_add_page() is expected to succeed, bio_add_page() can fail.

This series converts the callers of bio_add_page() which can easily use
__bio_add_page() to using it and checks the return of bio_add_page() for
callers that don't work on a freshly created bio.

Lastly it marks bio_add_page() as __must_check so we don't have to go again
and audit all callers.

Changes to v1:
- Removed pointless comment pointed out by Willy
- Changed commit messages pointed out by Damien
- Colledted Damien's Reviews and Acks

Johannes Thumshirn (19):
  swap: use __bio_add_page to add page to bio
  drbd: use __bio_add_page to add page to bio
  dm: dm-zoned: use __bio_add_page for adding single metadata page
  fs: buffer: use __bio_add_page to add single page to bio
  md: use __bio_add_page to add single page
  md: raid5-log: use __bio_add_page to add single page
  md: raid5: use __bio_add_page to add single page to new bio
  btrfs: repair: use __bio_add_page for adding single page
  btrfs: raid56: use __bio_add_page to add single page
  jfs: logmgr: use __bio_add_page to add single page to bio
  gfs: use __bio_add_page for adding single page to bio
  zonefs: use __bio_add_page for adding single page to bio
  zram: use __bio_add_page for adding single page to bio
  floppy: use __bio_add_page for adding single page to bio
  md: check for failure when adding pages in alloc_behind_master_bio
  md: raid1: use __bio_add_page for adding single page to bio
  md: raid1: check if adding pages to resync bio fails
  dm-crypt: check if adding pages to clone bio fails
  block: mark bio_add_page as __must_check

 drivers/block/drbd/drbd_bitmap.c |  4 +---
 drivers/block/floppy.c           |  2 +-
 drivers/block/zram/zram_drv.c    |  2 +-
 drivers/md/dm-crypt.c            |  9 ++++++++-
 drivers/md/dm-zoned-metadata.c   |  6 +++---
 drivers/md/md.c                  |  4 ++--
 drivers/md/raid1-10.c            |  7 ++++++-
 drivers/md/raid1.c               |  5 +++--
 drivers/md/raid10.c              | 12 ++++++++++--
 drivers/md/raid5-cache.c         |  2 +-
 drivers/md/raid5-ppl.c           |  4 ++--
 fs/btrfs/bio.c                   |  2 +-
 fs/btrfs/raid56.c                |  2 +-
 fs/buffer.c                      |  2 +-
 fs/gfs2/ops_fstype.c             |  2 +-
 fs/jfs/jfs_logmgr.c              |  4 ++--
 fs/zonefs/super.c                |  2 +-
 include/linux/bio.h              |  2 +-
 mm/page_io.c                     |  8 ++++----
 19 files changed, 50 insertions(+), 31 deletions(-)

Comments

David Sterba March 30, 2023, 3:45 p.m. UTC | #1

On Thu, Mar 30, 2023 at 03:43:42AM -0700, Johannes Thumshirn wrote:
> We have two functions for adding a page to a bio, __bio_add_page() which is
> used to add a single page to a freshly created bio and bio_add_page() which is
> used to add a page to an existing bio.
> 
> While __bio_add_page() is expected to succeed, bio_add_page() can fail.
> 
> This series converts the callers of bio_add_page() which can easily use
> __bio_add_page() to using it and checks the return of bio_add_page() for
> callers that don't work on a freshly created bio.
> 
> Lastly it marks bio_add_page() as __must_check so we don't have to go again
> and audit all callers.
> 
> Changes to v1:
> - Removed pointless comment pointed out by Willy
> - Changed commit messages pointed out by Damien
> - Colledted Damien's Reviews and Acks
> 
> Johannes Thumshirn (19):

>   btrfs: repair: use __bio_add_page for adding single page
>   btrfs: raid56: use __bio_add_page to add single page

The btrfs patches added to misc-next, thanks.

Johannes Thumshirn March 30, 2023, 4:41 p.m. UTC | #2

On 30.03.23 17:52, David Sterba wrote:
> On Thu, Mar 30, 2023 at 03:43:42AM -0700, Johannes Thumshirn wrote:
>> We have two functions for adding a page to a bio, __bio_add_page() which is
>> used to add a single page to a freshly created bio and bio_add_page() which is
>> used to add a page to an existing bio.
>>
>> While __bio_add_page() is expected to succeed, bio_add_page() can fail.
>>
>> This series converts the callers of bio_add_page() which can easily use
>> __bio_add_page() to using it and checks the return of bio_add_page() for
>> callers that don't work on a freshly created bio.
>>
>> Lastly it marks bio_add_page() as __must_check so we don't have to go again
>> and audit all callers.
>>
>> Changes to v1:
>> - Removed pointless comment pointed out by Willy
>> - Changed commit messages pointed out by Damien
>> - Colledted Damien's Reviews and Acks
>>
>> Johannes Thumshirn (19):
> 
>>   btrfs: repair: use __bio_add_page for adding single page
>>   btrfs: raid56: use __bio_add_page to add single page
> 
> The btrfs patches added to misc-next, thanks.
> 

Thanks but wouldn't it make more sense for Jens to pick up all of them?
The last patch in the series flips bio_add_pages() over to
__must_check and so it'll create an interdependency between the
btrfs and the block tree.

David Sterba March 30, 2023, 7:25 p.m. UTC | #3

On Thu, Mar 30, 2023 at 04:41:58PM +0000, Johannes Thumshirn wrote:
> On 30.03.23 17:52, David Sterba wrote:
> > On Thu, Mar 30, 2023 at 03:43:42AM -0700, Johannes Thumshirn wrote:
> >> We have two functions for adding a page to a bio, __bio_add_page() which is
> >> used to add a single page to a freshly created bio and bio_add_page() which is
> >> used to add a page to an existing bio.
> >>
> >> While __bio_add_page() is expected to succeed, bio_add_page() can fail.
> >>
> >> This series converts the callers of bio_add_page() which can easily use
> >> __bio_add_page() to using it and checks the return of bio_add_page() for
> >> callers that don't work on a freshly created bio.
> >>
> >> Lastly it marks bio_add_page() as __must_check so we don't have to go again
> >> and audit all callers.
> >>
> >> Changes to v1:
> >> - Removed pointless comment pointed out by Willy
> >> - Changed commit messages pointed out by Damien
> >> - Colledted Damien's Reviews and Acks
> >>
> >> Johannes Thumshirn (19):
> > 
> >>   btrfs: repair: use __bio_add_page for adding single page
> >>   btrfs: raid56: use __bio_add_page to add single page
> > 
> > The btrfs patches added to misc-next, thanks.
> > 
> 
> Thanks but wouldn't it make more sense for Jens to pick up all of them?
> The last patch in the series flips bio_add_pages() over to
> __must_check and so it'll create an interdependency between the
> btrfs and the block tree.

I'd rather take it via btrfs tree, this avoids future merge conflicts.