| Message ID | 20220605193854.2371230-4-willy@infradead.org |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | Convert to filemap_get_folios() | expand |
On Sun, Jun 05, 2022 at 08:38:47PM +0100, Matthew Wilcox (Oracle) wrote: > If the folio is large, it may overlap the beginning or end of the > unused range. If it does, we need to avoid invalidating it. It's never going to be larger for ext4, is it? But either way, those precautions looks fine. Reviewed-by: Christoph Hellwig <hch@lst.de>
On Wed, Jun 08, 2022 at 01:02:22AM -0700, Christoph Hellwig wrote: > On Sun, Jun 05, 2022 at 08:38:47PM +0100, Matthew Wilcox (Oracle) wrote: > > If the folio is large, it may overlap the beginning or end of the > > unused range. If it does, we need to avoid invalidating it. > > It's never going to be larger for ext4, is it? But either way, > those precautions looks fine. I don't want to say "never". Today, it's not, but if ext4 ever does gain support for large folios, then this is a precaution it will need to take. I'm trying not to leave traps when I do conversions. > Reviewed-by: Christoph Hellwig <hch@lst.de>
On Wed, Jun 08, 2022 at 05:02:40PM +0100, Matthew Wilcox wrote: > On Wed, Jun 08, 2022 at 01:02:22AM -0700, Christoph Hellwig wrote: > > On Sun, Jun 05, 2022 at 08:38:47PM +0100, Matthew Wilcox (Oracle) wrote: > > > If the folio is large, it may overlap the beginning or end of the > > > unused range. If it does, we need to avoid invalidating it. > > > > It's never going to be larger for ext4, is it? But either way, > > those precautions looks fine. > > I don't want to say "never". Today, it's not, but if ext4 ever does > gain support for large folios, then this is a precaution it will need > to take. I'm trying not to leave traps when I do conversions. FYI, this wasn't an objection to the patch, just a hint that the commit log could spell this out a bit better.
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 3dce7d058985..32a7f5e024d6 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -1554,9 +1554,9 @@ struct mpage_da_data { static void mpage_release_unused_pages(struct mpage_da_data *mpd, bool invalidate) { - int nr_pages, i; + unsigned nr, i; pgoff_t index, end; - struct pagevec pvec; + struct folio_batch fbatch; struct inode *inode = mpd->inode; struct address_space *mapping = inode->i_mapping; @@ -1574,15 +1574,18 @@ static void mpage_release_unused_pages(struct mpage_da_data *mpd, ext4_es_remove_extent(inode, start, last - start + 1); } - pagevec_init(&pvec); + folio_batch_init(&fbatch); while (index <= end) { - nr_pages = pagevec_lookup_range(&pvec, mapping, &index, end); - if (nr_pages == 0) + nr = filemap_get_folios(mapping, &index, end, &fbatch); + if (nr == 0) break; - for (i = 0; i < nr_pages; i++) { - struct page *page = pvec.pages[i]; - struct folio *folio = page_folio(page); + for (i = 0; i < nr; i++) { + struct folio *folio = fbatch.folios[i]; + if (folio->index < mpd->first_page) + continue; + if (folio->index + folio_nr_pages(folio) - 1 > end) + continue; BUG_ON(!folio_test_locked(folio)); BUG_ON(folio_test_writeback(folio)); if (invalidate) { @@ -1594,7 +1597,7 @@ static void mpage_release_unused_pages(struct mpage_da_data *mpd, } folio_unlock(folio); } - pagevec_release(&pvec); + folio_batch_release(&fbatch); } }
If the folio is large, it may overlap the beginning or end of the unused range. If it does, we need to avoid invalidating it. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- fs/ext4/inode.c | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-)