On Mon, Jan 10, 2022 at 11:31:03AM -0400, Jason Gunthorpe wrote: > > On Mon, Jan 10, 2022 at 04:23:38AM +0000, Matthew Wilcox (Oracle) wrote: > > This patch series is against my current folio for-next branch. I know > > it won't apply to sfr's next tree, and it's not for-next material yet. > > I intend to submit it for 5.18 after I've rebased it to one of the > > 5.17-rc releases. > > > > The overall effect of this (ignoring the primary "preparing for folios > > that are not PAGE or PMD sized" purpose) is to reduce the size of gup.o > > by ~700 bytes in the config I normally test with. > > > > This patchset just converts existing implementations to use folios. > > There's no new API for consumers here to provide information in a more > > efficient (address, length) format. That will be a separate patchset. > > > > In v2, I've tried to address all the comments from the reviews I got > > on v1. Apologies if I missed anything; I got a lot of good feedback. > > Primarily I separated out the folio changes (later) from the "While > > I'm looking at this ..." changes (earlier). I'm not sure the story > > of the patchset is necessarily coherent this way, but it should be > > easier to review. > > > > Another big change is that compound_pincount is now available to all > > compound pages, not just those that are order-2-or-higher. Patch 11. > > > > I did notice one bug in my original patchset which I'm disappointed GCC > > didn't diagnose: > > > > pages[nr++] = nth_page(page, nr); > > > > Given the massive reorg of the patchset, I didn't feel right using > > anyone's SoB from v1 on any of the patches. But, despite the increased > > number of patches, I hope it's easier to review than v1. > > > > And I'd dearly love a better name than 'folio_nth'. page_nth() is > > a temporary construct, so doesn't need a better name. If you need > > context, it's in the gup_folio_range_next() patch and its job > > is to tell you, given a page and a folio, what # page it is within > > a folio (so a number between [0 and folio_nr_pages())). > > folio_tail_index() ? I'm a little wary of "index" because folios are used to cache file data, and folio->index means offset of the folio within the file. We could make the argument for folio_page_index() (since it might be the head page, not a tail page), and argue this is the index into the folio, not the index into the file. It's better than folio_nth ;-) > Series still looks good to me, though I checked each patch > carefully than the prior series: > > Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx> Thanks!
