| Message ID | 20230502101934.24901-1-johannes.thumshirn@wdc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | bio: check return values of bio_add_page | expand |
On 02.05.23 12:20, 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 v4: > - Rebased onto latest Linus' master > - Dropped already merged patches > - Added Sergey's Reviewed-by > > Changes to v3: > - Added __bio_add_folio and use it in iomap (Willy) > - Mark bio_add_folio must check (Willy) > - s/GFS/GFS2/ (Andreas) > > Changes to v2: > - Removed 'wont fail' comments pointed out by Song > > Changes to v1: > - Removed pointless comment pointed out by Willy > - Changed commit messages pointed out by Damien > - Colledted Damien's Reviews and Acks Jens any comments on this?
On 5/5/23 2:09?AM, Johannes Thumshirn wrote: > On 02.05.23 12:20, 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 v4: >> - Rebased onto latest Linus' master >> - Dropped already merged patches >> - Added Sergey's Reviewed-by >> >> Changes to v3: >> - Added __bio_add_folio and use it in iomap (Willy) >> - Mark bio_add_folio must check (Willy) >> - s/GFS/GFS2/ (Andreas) >> >> Changes to v2: >> - Removed 'wont fail' comments pointed out by Song >> >> Changes to v1: >> - Removed pointless comment pointed out by Willy >> - Changed commit messages pointed out by Damien >> - Colledted Damien's Reviews and Acks > > Jens any comments on this? I'll take a look post -rc1.
On 05.05.23 16:12, Jens Axboe wrote: > On 5/5/23 2:09?AM, Johannes Thumshirn wrote: >> On 02.05.23 12:20, 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 v4: >>> - Rebased onto latest Linus' master >>> - Dropped already merged patches >>> - Added Sergey's Reviewed-by >>> >>> Changes to v3: >>> - Added __bio_add_folio and use it in iomap (Willy) >>> - Mark bio_add_folio must check (Willy) >>> - s/GFS/GFS2/ (Andreas) >>> >>> Changes to v2: >>> - Removed 'wont fail' comments pointed out by Song >>> >>> Changes to v1: >>> - Removed pointless comment pointed out by Willy >>> - Changed commit messages pointed out by Damien >>> - Colledted Damien's Reviews and Acks >> >> Jens any comments on this? > > I'll take a look post -rc1. > Ping again?
On 5/2/23 4:19?AM, 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. Looks fine to me, though it would be nice if the fs and dm people could give this a quick look. Should not take long, any empty bio addition should, by definition, be able to use a non-checked page addition for the first page.
On 24.05.23 17:02, Jens Axboe wrote: > On 5/2/23 4:19?AM, 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. > > Looks fine to me, though it would be nice if the fs and dm people could > give this a quick look. Should not take long, any empty bio addition > should, by definition, be able to use a non-checked page addition for > the first page. > I think the FS side is all covered. @Mike could you have a look at the dm patches?
On 5/26/23 12:37 AM, Johannes Thumshirn wrote: > On 24.05.23 17:02, Jens Axboe wrote: >> On 5/2/23 4:19?AM, 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. >> >> Looks fine to me, though it would be nice if the fs and dm people could >> give this a quick look. Should not take long, any empty bio addition >> should, by definition, be able to use a non-checked page addition for >> the first page. >> > > I think the FS side is all covered. @Mike could you have a look at the dm > patches? Not the iomap one, that was my main concern. Not that this is tricky stuff, but still...