| Message ID | 20230315123233.121593-3-p.raghav@samsung.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC,1/3] filemap: convert page_endio to folio_endio | expand |
On Wed, Mar 15, 2023 at 01:32:32PM +0100, Pankaj Raghav wrote: > Use bio_for_each_folio_all to iterate through folios in a bio so that > the folios can be directly passed to the folio_endio() function. > > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com> Reviewed-by: Luis Chamberlain <mcgrof@kernel.org> Luis
On 3/15/23 13:32, Pankaj Raghav wrote: > Use bio_for_each_folio_all to iterate through folios in a bio so that > the folios can be directly passed to the folio_endio() function. > > Signed-off-by: Pankaj Raghav <p.raghav@samsung.com> > --- > fs/mpage.c | 11 ++++------- > 1 file changed, 4 insertions(+), 7 deletions(-) > > diff --git a/fs/mpage.c b/fs/mpage.c > index 40e86e839e77..bfcc139938a8 100644 > --- a/fs/mpage.c > +++ b/fs/mpage.c > @@ -45,14 +45,11 @@ > */ > static void mpage_end_io(struct bio *bio) > { > - struct bio_vec *bv; > - struct bvec_iter_all iter_all; > + struct folio_iter fi; > > - bio_for_each_segment_all(bv, bio, iter_all) { > - struct page *page = bv->bv_page; > - folio_endio(page_folio(page), bio_op(bio), > - blk_status_to_errno(bio->bi_status)); > - } > + bio_for_each_folio_all(fi, bio) > + folio_endio(fi.folio, bio_op(bio), > + blk_status_to_errno(bio->bi_status)); > > bio_put(bio); > } Ah. Here it is. I would suggest merge these two patches. Cheers, Hannes
On Wed, Mar 15, 2023 at 03:52:15PM +0100, Hannes Reinecke wrote: > On 3/15/23 13:32, Pankaj Raghav wrote: > > Use bio_for_each_folio_all to iterate through folios in a bio so that > > the folios can be directly passed to the folio_endio() function. > > + bio_for_each_folio_all(fi, bio) > > + folio_endio(fi.folio, bio_op(bio), > > + blk_status_to_errno(bio->bi_status)); > > bio_put(bio); > > } > > Ah. Here it is. > > I would suggest merge these two patches. The right way to have handled this patch series was: 1. Introduce a new folio_endio() [but see Christoph's mail on why we shouldn't do that] 2-n convert callers to use folios directly n+1 remove page_endio() entirely. Note that patch n+1 might not be part of this patch series; sometimes it takes a while to convert all callers to use folios. I very much dislike the way this was done by pushing the page_folio() call into each of the callers because it makes the entire series hard to review.
On 2023-03-15 17:08, Matthew Wilcox wrote: > On Wed, Mar 15, 2023 at 03:52:15PM +0100, Hannes Reinecke wrote: >> On 3/15/23 13:32, Pankaj Raghav wrote: >>> Use bio_for_each_folio_all to iterate through folios in a bio so that >>> the folios can be directly passed to the folio_endio() function. >>> + bio_for_each_folio_all(fi, bio) >>> + folio_endio(fi.folio, bio_op(bio), >>> + blk_status_to_errno(bio->bi_status)); >>> bio_put(bio); >>> } >> >> Ah. Here it is. >> >> I would suggest merge these two patches. > > The right way to have handled this patch series was: > > 1. Introduce a new folio_endio() [but see Christoph's mail on why we > shouldn't do that] > 2-n convert callers to use folios directly > n+1 remove page_endio() entirely. > > Note that patch n+1 might not be part of this patch series; sometimes > it takes a while to convert all callers to use folios. > This is definitely a much better way to handle these changes. Thanks willy. > I very much dislike the way this was done by pushing the page_folio() > call into each of the callers because it makes the entire series hard to > review.
diff --git a/fs/mpage.c b/fs/mpage.c index 40e86e839e77..bfcc139938a8 100644 --- a/fs/mpage.c +++ b/fs/mpage.c @@ -45,14 +45,11 @@ */ static void mpage_end_io(struct bio *bio) { - struct bio_vec *bv; - struct bvec_iter_all iter_all; + struct folio_iter fi; - bio_for_each_segment_all(bv, bio, iter_all) { - struct page *page = bv->bv_page; - folio_endio(page_folio(page), bio_op(bio), - blk_status_to_errno(bio->bi_status)); - } + bio_for_each_folio_all(fi, bio) + folio_endio(fi.folio, bio_op(bio), + blk_status_to_errno(bio->bi_status)); bio_put(bio); }
Use bio_for_each_folio_all to iterate through folios in a bio so that the folios can be directly passed to the folio_endio() function. Signed-off-by: Pankaj Raghav <p.raghav@samsung.com> --- fs/mpage.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-)