On Tue, Aug 20, 2024 at 1:28 PM Kundan Kumar <kundan.kumar@xxxxxxxxxxx> wrote: > > On 17/08/24 05:23AM, Matthew Wilcox wrote: > >On Thu, Jul 11, 2024 at 10:37:46AM +0530, Kundan Kumar wrote: > >> -bool bvec_try_merge_hw_page(struct request_queue *q, struct bio_vec *bv, > >> - struct page *page, unsigned len, unsigned offset, > >> +bool bvec_try_merge_hw_folio(struct request_queue *q, struct bio_vec *bv, > >> + struct folio *folio, size_t len, size_t offset, > >> bool *same_page) > >> { > >> + struct page *page = folio_page(folio, 0); > >> unsigned long mask = queue_segment_boundary(q); > >> phys_addr_t addr1 = bvec_phys(bv); > >> phys_addr_t addr2 = page_to_phys(page) + offset + len - 1; > >[...] > >> +bool bvec_try_merge_hw_page(struct request_queue *q, struct bio_vec *bv, > >> + struct page *page, unsigned int len, unsigned int offset, > >> + bool *same_page) > >> +{ > >> + struct folio *folio = page_folio(page); > >> + > >> + return bvec_try_merge_hw_folio(q, bv, folio, len, > >> + ((size_t)folio_page_idx(folio, page) << PAGE_SHIFT) + > >> + offset, same_page); > >> +} > > > >This is the wrong way to do it. bio_add_folio() does it correctly > >by being a wrapper around bio_add_page(). > > > >The reason is that in the future, not all pages will belong to folios. > >For those pages, page_folio() will return NULL, and this will crash. > > > > I can change in this fashion. page_folio is getting used at many other > places in kernel and currently there are no NULL checks. Will every place > need a change? > In this series we use page_folio to fetch folio for pages returned by > iov_iter_extract_pages. Then we iterate on the folios instead of pages. > We were progressing to change all the page related functions to accept > struct folio. > If page_folio may return NULL in future, it will require us to maintain > both page and folio versions. Do you see it differently ? > Gentle ping. Any feedback on this ?