This patch series replaces find_get_pages_contig() with
filemap_get_folios_contig(). I've run xfstests on btrfs. I've also
tested the ramfs changes. I ran some xfstests on nilfs2, and its
seemingly fine although more testing may be beneficial.
---
v2:
- Removed an unused label in nilfs2
Vishal Moola (Oracle) (7):
filemap: Add filemap_get_folios_contig()
btrfs: Convert __process_pages_contig() to use
filemap_get_folios_contig()
btrfs: Convert end_compressed_writeback() to use filemap_get_folios()
btrfs: Convert process_page_range() to use filemap_get_folios_contig()
nilfs2: Convert nilfs_find_uncommited_extent() to use
filemap_get_folios_contig()
ramfs: Convert ramfs_nommu_get_unmapped_area() to use
filemap_get_folios_contig()
filemap: Remove find_get_pages_contig()
fs/btrfs/compression.c | 26 ++++++------
fs/btrfs/extent_io.c | 33 +++++++--------
fs/btrfs/subpage.c | 2 +-
fs/btrfs/tests/extent-io-tests.c | 31 +++++++-------
fs/nilfs2/page.c | 39 ++++++++----------
fs/ramfs/file-nommu.c | 50 ++++++++++++----------
include/linux/pagemap.h | 4 +-
mm/filemap.c | 71 +++++++++++++++++++-------------
8 files changed, 134 insertions(+), 122 deletions(-)
--
2.36.1
Convert to use folios throughout. This is in preparation for the removal of
find_get_pages_contig(). Now also supports large folios.
Since we may receive more than nr_pages pages, nr_pages may underflow.
Since nr_pages > 0 is equivalent to index <= end_index, we replaced it
with this check instead.
Signed-off-by: Vishal Moola (Oracle) <[email protected]>
---
fs/btrfs/extent_io.c | 33 +++++++++++++++------------------
1 file changed, 15 insertions(+), 18 deletions(-)
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 8f6b544ae616..f16929bc531b 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -1882,9 +1882,8 @@ static int __process_pages_contig(struct address_space *mapping,
pgoff_t start_index = start >> PAGE_SHIFT;
pgoff_t end_index = end >> PAGE_SHIFT;
pgoff_t index = start_index;
- unsigned long nr_pages = end_index - start_index + 1;
unsigned long pages_processed = 0;
- struct page *pages[16];
+ struct folio_batch fbatch;
int err = 0;
int i;
@@ -1893,16 +1892,17 @@ static int __process_pages_contig(struct address_space *mapping,
ASSERT(processed_end && *processed_end == start);
}
- if ((page_ops & PAGE_SET_ERROR) && nr_pages > 0)
+ if ((page_ops & PAGE_SET_ERROR) && start_index <= end_index)
mapping_set_error(mapping, -EIO);
- while (nr_pages > 0) {
- int found_pages;
+ folio_batch_init(&fbatch);
+ while (index <= end_index) {
+ int found_folios;
+
+ found_folios = filemap_get_folios_contig(mapping, &index,
+ end_index, &fbatch);
- found_pages = find_get_pages_contig(mapping, index,
- min_t(unsigned long,
- nr_pages, ARRAY_SIZE(pages)), pages);
- if (found_pages == 0) {
+ if (found_folios == 0) {
/*
* Only if we're going to lock these pages, we can find
* nothing at @index.
@@ -1912,23 +1912,20 @@ static int __process_pages_contig(struct address_space *mapping,
goto out;
}
- for (i = 0; i < found_pages; i++) {
+ for (i = 0; i < found_folios; i++) {
int process_ret;
-
+ struct folio *folio = fbatch.folios[i];
process_ret = process_one_page(fs_info, mapping,
- pages[i], locked_page, page_ops,
+ &folio->page, locked_page, page_ops,
start, end);
if (process_ret < 0) {
- for (; i < found_pages; i++)
- put_page(pages[i]);
err = -EAGAIN;
+ folio_batch_release(&fbatch);
goto out;
}
- put_page(pages[i]);
- pages_processed++;
+ pages_processed += folio_nr_pages(folio);
}
- nr_pages -= found_pages;
- index += found_pages;
+ folio_batch_release(&fbatch);
cond_resched();
}
out:
--
2.36.1
Converted function to use folios throughout. This is in preparation for
the removal of find_get_pages_contig(). Now also supports large folios.
Also cleaned up an unnecessary if statement - pvec.pages[0]->index > index
will always evaluate to false, and filemap_get_folios_contig() returns 0 if
there is no folio found at index.
Signed-off-by: Vishal Moola (Oracle) <[email protected]>
---
v2:
- Fixed a warning regarding a now unused label "out"
- Reported-by: kernel test robot <[email protected]>
---
fs/nilfs2/page.c | 39 +++++++++++++++++----------------------
1 file changed, 17 insertions(+), 22 deletions(-)
diff --git a/fs/nilfs2/page.c b/fs/nilfs2/page.c
index 3267e96c256c..14629e03d0da 100644
--- a/fs/nilfs2/page.c
+++ b/fs/nilfs2/page.c
@@ -480,13 +480,13 @@ unsigned long nilfs_find_uncommitted_extent(struct inode *inode,
sector_t start_blk,
sector_t *blkoff)
{
- unsigned int i;
+ unsigned int i, nr;
pgoff_t index;
unsigned int nblocks_in_page;
unsigned long length = 0;
sector_t b;
- struct pagevec pvec;
- struct page *page;
+ struct folio_batch fbatch;
+ struct folio *folio;
if (inode->i_mapping->nrpages == 0)
return 0;
@@ -494,27 +494,24 @@ unsigned long nilfs_find_uncommitted_extent(struct inode *inode,
index = start_blk >> (PAGE_SHIFT - inode->i_blkbits);
nblocks_in_page = 1U << (PAGE_SHIFT - inode->i_blkbits);
- pagevec_init(&pvec);
+ folio_batch_init(&fbatch);
repeat:
- pvec.nr = find_get_pages_contig(inode->i_mapping, index, PAGEVEC_SIZE,
- pvec.pages);
- if (pvec.nr == 0)
+ nr = filemap_get_folios_contig(inode->i_mapping, &index, ULONG_MAX,
+ &fbatch);
+ if (nr == 0)
return length;
- if (length > 0 && pvec.pages[0]->index > index)
- goto out;
-
- b = pvec.pages[0]->index << (PAGE_SHIFT - inode->i_blkbits);
+ b = fbatch.folios[0]->index << (PAGE_SHIFT - inode->i_blkbits);
i = 0;
do {
- page = pvec.pages[i];
+ folio = fbatch.folios[i];
- lock_page(page);
- if (page_has_buffers(page)) {
+ folio_lock(folio);
+ if (folio_buffers(folio)) {
struct buffer_head *bh, *head;
- bh = head = page_buffers(page);
+ bh = head = folio_buffers(folio);
do {
if (b < start_blk)
continue;
@@ -532,18 +529,16 @@ unsigned long nilfs_find_uncommitted_extent(struct inode *inode,
b += nblocks_in_page;
}
- unlock_page(page);
+ folio_unlock(folio);
- } while (++i < pagevec_count(&pvec));
+ } while (++i < nr);
- index = page->index + 1;
- pagevec_release(&pvec);
+ folio_batch_release(&fbatch);
cond_resched();
goto repeat;
out_locked:
- unlock_page(page);
-out:
- pagevec_release(&pvec);
+ folio_unlock(folio);
+ folio_batch_release(&fbatch);
return length;
}
--
2.36.1
This function is meant to replace find_get_pages_contig().
Unlike find_get_pages_contig(), filemap_get_folios_contig() no
longer takes in a target number of pages to find - It returns up to 15
contiguous folios.
To be more consistent with filemap_get_folios(), filemap_get_folios_contig()
now also updates the start index passed in, and takes an end index.
Signed-off-by: Vishal Moola (Oracle) <[email protected]>
---
include/linux/pagemap.h | 2 ++
mm/filemap.c | 73 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 75 insertions(+)
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index cc9adbaddb59..951936a2be1d 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -720,6 +720,8 @@ static inline struct page *find_subpage(struct page *head, pgoff_t index)
unsigned filemap_get_folios(struct address_space *mapping, pgoff_t *start,
pgoff_t end, struct folio_batch *fbatch);
+unsigned filemap_get_folios_contig(struct address_space *mapping,
+ pgoff_t *start, pgoff_t end, struct folio_batch *fbatch);
unsigned find_get_pages_contig(struct address_space *mapping, pgoff_t start,
unsigned int nr_pages, struct page **pages);
unsigned find_get_pages_range_tag(struct address_space *mapping, pgoff_t *index,
diff --git a/mm/filemap.c b/mm/filemap.c
index 8ccb868c3d95..8167bcc96e37 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2196,6 +2196,79 @@ bool folio_more_pages(struct folio *folio, pgoff_t index, pgoff_t max)
return index < folio->index + folio_nr_pages(folio) - 1;
}
+/**
+ * filemap_get_folios_contig - Get a batch of contiguous folios
+ * @mapping: The address_space to search
+ * @start: The starting page index
+ * @end: The final page index (inclusive)
+ * @fbatch: The batch to fill
+ *
+ * filemap_get_folios_contig() works exactly like filemap_get_folios(),
+ * except the returned folios are guaranteed to be contiguous. This may
+ * not return all contiguous folios if the batch gets filled up.
+ *
+ * Return: The number of folios found.
+ * Also update @start to be positioned for traversal of the next folio.
+ */
+
+unsigned filemap_get_folios_contig(struct address_space *mapping,
+ pgoff_t *start, pgoff_t end, struct folio_batch *fbatch)
+{
+ XA_STATE(xas, &mapping->i_pages, *start);
+ unsigned long nr;
+ struct folio *folio;
+
+ rcu_read_lock();
+
+ for (folio = xas_load(&xas); folio && xas.xa_index <= end;
+ folio = xas_next(&xas)) {
+ if (xas_retry(&xas, folio))
+ continue;
+ /*
+ * If the entry has been swapped out, we can stop looking.
+ * No current caller is looking for DAX entries.
+ */
+ if (xa_is_value(folio))
+ goto update_start;
+
+ if (!folio_try_get_rcu(folio))
+ goto retry;
+
+ if (unlikely(folio != xas_reload(&xas)))
+ goto put_folio;
+
+ if (!folio_batch_add(fbatch, folio)) {
+ nr = folio_nr_pages(folio);
+
+ if (folio_test_hugetlb(folio))
+ nr = 1;
+ *start = folio->index + nr;
+ goto out;
+ }
+ continue;
+put_folio:
+ folio_put(folio);
+
+retry:
+ xas_reset(&xas);
+ }
+
+update_start:
+ nr = folio_batch_count(fbatch);
+
+ if (nr) {
+ folio = fbatch->folios[nr - 1];
+ if (folio_test_hugetlb(folio))
+ *start = folio->index + 1;
+ else
+ *start = folio->index + folio_nr_pages(folio);
+ }
+out:
+ rcu_read_unlock();
+ return folio_batch_count(fbatch);
+}
+EXPORT_SYMBOL(filemap_get_folios_contig);
+
/**
* find_get_pages_contig - gang contiguous pagecache lookup
* @mapping: The address_space to search
--
2.36.1
Converted function to use folios throughout. This is in preparation for
the removal of find_get_pages_contig(). Now also supports large folios.
Since we may receive more than nr_pages pages, nr_pages may underflow.
Since nr_pages > 0 is equivalent to index <= end_index, we replaced it
with this check instead.
Also this function does not care about the pages being contiguous so we
can just use filemap_get_folios() to be more efficient.
Signed-off-by: Vishal Moola (Oracle) <[email protected]>
---
fs/btrfs/compression.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)
diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index f4564f32f6d9..3dcd5006cfb4 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -8,6 +8,7 @@
#include <linux/file.h>
#include <linux/fs.h>
#include <linux/pagemap.h>
+#include <linux/pagevec.h>
#include <linux/highmem.h>
#include <linux/kthread.h>
#include <linux/time.h>
@@ -339,8 +340,7 @@ static noinline void end_compressed_writeback(struct inode *inode,
struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
unsigned long index = cb->start >> PAGE_SHIFT;
unsigned long end_index = (cb->start + cb->len - 1) >> PAGE_SHIFT;
- struct page *pages[16];
- unsigned long nr_pages = end_index - index + 1;
+ struct folio_batch fbatch;
const int errno = blk_status_to_errno(cb->status);
int i;
int ret;
@@ -348,24 +348,22 @@ static noinline void end_compressed_writeback(struct inode *inode,
if (errno)
mapping_set_error(inode->i_mapping, errno);
- while (nr_pages > 0) {
- ret = find_get_pages_contig(inode->i_mapping, index,
- min_t(unsigned long,
- nr_pages, ARRAY_SIZE(pages)), pages);
+ folio_batch_init(&fbatch);
+ while (index <= end_index) {
+ ret = filemap_get_folios(inode->i_mapping, &index, end_index,
+ &fbatch);
+
if (ret == 0) {
- nr_pages -= 1;
- index += 1;
- continue;
+ return;
}
for (i = 0; i < ret; i++) {
+ struct folio *folio = fbatch.folios[i];
if (errno)
- SetPageError(pages[i]);
- btrfs_page_clamp_clear_writeback(fs_info, pages[i],
+ folio_set_error(folio);
+ btrfs_page_clamp_clear_writeback(fs_info, &folio->page,
cb->start, cb->len);
- put_page(pages[i]);
}
- nr_pages -= ret;
- index += ret;
+ folio_batch_release(&fbatch);
}
/* the inode may be gone now */
}
--
2.36.1
All callers of find_get_pages_contig() have been removed, so it is no
longer needed.
Signed-off-by: Vishal Moola (Oracle) <[email protected]>
---
include/linux/pagemap.h | 2 --
mm/filemap.c | 60 -----------------------------------------
2 files changed, 62 deletions(-)
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 951936a2be1d..a04a645b64e9 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -722,8 +722,6 @@ unsigned filemap_get_folios(struct address_space *mapping, pgoff_t *start,
pgoff_t end, struct folio_batch *fbatch);
unsigned filemap_get_folios_contig(struct address_space *mapping,
pgoff_t *start, pgoff_t end, struct folio_batch *fbatch);
-unsigned find_get_pages_contig(struct address_space *mapping, pgoff_t start,
- unsigned int nr_pages, struct page **pages);
unsigned find_get_pages_range_tag(struct address_space *mapping, pgoff_t *index,
pgoff_t end, xa_mark_t tag, unsigned int nr_pages,
struct page **pages);
diff --git a/mm/filemap.c b/mm/filemap.c
index 8167bcc96e37..af5a4b5f866d 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2269,66 +2269,6 @@ unsigned filemap_get_folios_contig(struct address_space *mapping,
}
EXPORT_SYMBOL(filemap_get_folios_contig);
-/**
- * find_get_pages_contig - gang contiguous pagecache lookup
- * @mapping: The address_space to search
- * @index: The starting page index
- * @nr_pages: The maximum number of pages
- * @pages: Where the resulting pages are placed
- *
- * find_get_pages_contig() works exactly like find_get_pages_range(),
- * except that the returned number of pages are guaranteed to be
- * contiguous.
- *
- * Return: the number of pages which were found.
- */
-unsigned find_get_pages_contig(struct address_space *mapping, pgoff_t index,
- unsigned int nr_pages, struct page **pages)
-{
- XA_STATE(xas, &mapping->i_pages, index);
- struct folio *folio;
- unsigned int ret = 0;
-
- if (unlikely(!nr_pages))
- return 0;
-
- rcu_read_lock();
- for (folio = xas_load(&xas); folio; folio = xas_next(&xas)) {
- if (xas_retry(&xas, folio))
- continue;
- /*
- * If the entry has been swapped out, we can stop looking.
- * No current caller is looking for DAX entries.
- */
- if (xa_is_value(folio))
- break;
-
- if (!folio_try_get_rcu(folio))
- goto retry;
-
- if (unlikely(folio != xas_reload(&xas)))
- goto put_page;
-
-again:
- pages[ret] = folio_file_page(folio, xas.xa_index);
- if (++ret == nr_pages)
- break;
- if (folio_more_pages(folio, xas.xa_index, ULONG_MAX)) {
- xas.xa_index++;
- folio_ref_inc(folio);
- goto again;
- }
- continue;
-put_page:
- folio_put(folio);
-retry:
- xas_reset(&xas);
- }
- rcu_read_unlock();
- return ret;
-}
-EXPORT_SYMBOL(find_get_pages_contig);
-
/**
* find_get_pages_range_tag - Find and return head pages matching @tag.
* @mapping: the address_space to search
--
2.36.1
Converted to use folios throughout. This is in preparation for the
removal for find_get_pages_contig(). Now also supports large folios.
The initial version of this function set the page_address to be returned
after finishing all the checks. Since folio_batches have a maximum of 15
folios, the function had to be modified to support getting and checking up
to lpages, 15 pages at a time while still returning the initial page address.
Now the function sets ret as soon as the first batch arrives, and updates it
only if a check fails.
The physical adjacency check utilizes the page frame numbers. The page frame
number of each folio must be nr_pages away from the first folio.
Signed-off-by: Vishal Moola (Oracle) <[email protected]>
---
fs/ramfs/file-nommu.c | 50 +++++++++++++++++++++++++------------------
1 file changed, 29 insertions(+), 21 deletions(-)
diff --git a/fs/ramfs/file-nommu.c b/fs/ramfs/file-nommu.c
index ba3525ccc27e..81817f301e17 100644
--- a/fs/ramfs/file-nommu.c
+++ b/fs/ramfs/file-nommu.c
@@ -203,9 +203,9 @@ static unsigned long ramfs_nommu_get_unmapped_area(struct file *file,
unsigned long addr, unsigned long len,
unsigned long pgoff, unsigned long flags)
{
- unsigned long maxpages, lpages, nr, loop, ret;
+ unsigned long maxpages, lpages, nr, loop, ret, nr_pages, pfn;
struct inode *inode = file_inode(file);
- struct page **pages = NULL, **ptr, *page;
+ struct folio_batch fbatch;
loff_t isize;
/* the mapping mustn't extend beyond the EOF */
@@ -221,31 +221,39 @@ static unsigned long ramfs_nommu_get_unmapped_area(struct file *file,
goto out;
/* gang-find the pages */
- pages = kcalloc(lpages, sizeof(struct page *), GFP_KERNEL);
- if (!pages)
- goto out_free;
-
- nr = find_get_pages_contig(inode->i_mapping, pgoff, lpages, pages);
- if (nr != lpages)
- goto out_free_pages; /* leave if some pages were missing */
+ folio_batch_init(&fbatch);
+ nr_pages = 0;
+repeat:
+ nr = filemap_get_folios_contig(inode->i_mapping, &pgoff,
+ ULONG_MAX, &fbatch);
+ if (!nr) {
+ ret = -ENOSYS;
+ return ret;
+ }
+ if (ret == -ENOSYS) {
+ ret = (unsigned long) folio_address(fbatch.folios[0]);
+ pfn = folio_pfn(fbatch.folios[0]);
+ }
/* check the pages for physical adjacency */
- ptr = pages;
- page = *ptr++;
- page++;
- for (loop = lpages; loop > 1; loop--)
- if (*ptr++ != page++)
- goto out_free_pages;
+ for (loop = 0; loop < nr; loop++) {
+ if (pfn + nr_pages != folio_pfn(fbatch.folios[loop])) {
+ ret = -ENOSYS;
+ goto out_free; /* leave if not physical adjacent */
+ }
+ nr_pages += folio_nr_pages(fbatch.folios[loop]);
+ if (nr_pages >= lpages)
+ goto out_free; /* successfully found desired pages*/
+ }
+ if (nr_pages < lpages) {
+ folio_batch_release(&fbatch);
+ goto repeat; /* loop if pages are missing */
+ }
/* okay - all conditions fulfilled */
- ret = (unsigned long) page_address(pages[0]);
-out_free_pages:
- ptr = pages;
- for (loop = nr; loop > 0; loop--)
- put_page(*ptr++);
out_free:
- kfree(pages);
+ folio_batch_release(&fbatch);
out:
return ret;
}
--
2.36.1
Converted function to use folios throughout. This is in preparation for
the removal of find_get_pages_contig(). Now also supports large folios.
Since we may receive more than nr_pages pages, nr_pages may underflow.
Since nr_pages > 0 is equivalent to index <= end_index, we replaced it
with this check instead.
Also minor comment renaming for consistency in subpage.
Signed-off-by: Vishal Moola (Oracle) <[email protected]>
---
fs/btrfs/subpage.c | 2 +-
fs/btrfs/tests/extent-io-tests.c | 31 ++++++++++++++++---------------
2 files changed, 17 insertions(+), 16 deletions(-)
diff --git a/fs/btrfs/subpage.c b/fs/btrfs/subpage.c
index a105b291444f..6418c38c4b30 100644
--- a/fs/btrfs/subpage.c
+++ b/fs/btrfs/subpage.c
@@ -337,7 +337,7 @@ bool btrfs_subpage_end_and_test_writer(const struct btrfs_fs_info *fs_info,
*
* Even with 0 returned, the page still need extra check to make sure
* it's really the correct page, as the caller is using
- * find_get_pages_contig(), which can race with page invalidating.
+ * filemap_get_folios_contig(), which can race with page invalidating.
*/
int btrfs_page_start_writer_lock(const struct btrfs_fs_info *fs_info,
struct page *page, u64 start, u32 len)
diff --git a/fs/btrfs/tests/extent-io-tests.c b/fs/btrfs/tests/extent-io-tests.c
index a232b15b8021..530073868916 100644
--- a/fs/btrfs/tests/extent-io-tests.c
+++ b/fs/btrfs/tests/extent-io-tests.c
@@ -4,6 +4,7 @@
*/
#include <linux/pagemap.h>
+#include <linux/pagevec.h>
#include <linux/sched.h>
#include <linux/slab.h>
#include <linux/sizes.h>
@@ -20,39 +21,39 @@ static noinline int process_page_range(struct inode *inode, u64 start, u64 end,
unsigned long flags)
{
int ret;
- struct page *pages[16];
+ struct folio_batch fbatch;
unsigned long index = start >> PAGE_SHIFT;
unsigned long end_index = end >> PAGE_SHIFT;
- unsigned long nr_pages = end_index - index + 1;
int i;
int count = 0;
int loops = 0;
- while (nr_pages > 0) {
- ret = find_get_pages_contig(inode->i_mapping, index,
- min_t(unsigned long, nr_pages,
- ARRAY_SIZE(pages)), pages);
+ folio_batch_init(&fbatch);
+
+ while (index <= end_index) {
+ ret = filemap_get_folios_contig(inode->i_mapping, &index,
+ end_index, &fbatch);
for (i = 0; i < ret; i++) {
+ struct folio *folio = fbatch.folios[i];
if (flags & PROCESS_TEST_LOCKED &&
- !PageLocked(pages[i]))
+ !folio_test_locked(folio))
count++;
- if (flags & PROCESS_UNLOCK && PageLocked(pages[i]))
- unlock_page(pages[i]);
- put_page(pages[i]);
+ if (flags & PROCESS_UNLOCK && folio_test_locked(folio))
+ folio_unlock(folio);
if (flags & PROCESS_RELEASE)
- put_page(pages[i]);
+ folio_put(folio);
}
- nr_pages -= ret;
- index += ret;
+ folio_batch_release(&fbatch);
cond_resched();
loops++;
if (loops > 100000) {
printk(KERN_ERR
- "stuck in a loop, start %llu, end %llu, nr_pages %lu, ret %d\n",
- start, end, nr_pages, ret);
+ "stuck in a loop, start %llu, end %llu, ret %d\n",
+ start, end, ret);
break;
}
}
+
return count;
}
--
2.36.1
On Wed, Aug 17, 2022 at 2:54 AM Vishal Moola (Oracle) wrote:
>
> Converted function to use folios throughout. This is in preparation for
> the removal of find_get_pages_contig(). Now also supports large folios.
>
> Also cleaned up an unnecessary if statement - pvec.pages[0]->index > index
> will always evaluate to false, and filemap_get_folios_contig() returns 0 if
> there is no folio found at index.
>
> Signed-off-by: Vishal Moola (Oracle) <[email protected]>
> ---
>
> v2:
> - Fixed a warning regarding a now unused label "out"
> - Reported-by: kernel test robot <[email protected]>
> ---
> fs/nilfs2/page.c | 39 +++++++++++++++++----------------------
> 1 file changed, 17 insertions(+), 22 deletions(-)
>
> diff --git a/fs/nilfs2/page.c b/fs/nilfs2/page.c
> index 3267e96c256c..14629e03d0da 100644
> --- a/fs/nilfs2/page.c
> +++ b/fs/nilfs2/page.c
> @@ -480,13 +480,13 @@ unsigned long nilfs_find_uncommitted_extent(struct inode *inode,
> sector_t start_blk,
> sector_t *blkoff)
> {
> - unsigned int i;
> + unsigned int i, nr;
> pgoff_t index;
> unsigned int nblocks_in_page;
> unsigned long length = 0;
> sector_t b;
> - struct pagevec pvec;
> - struct page *page;
> + struct folio_batch fbatch;
> + struct folio *folio;
>
> if (inode->i_mapping->nrpages == 0)
> return 0;
> @@ -494,27 +494,24 @@ unsigned long nilfs_find_uncommitted_extent(struct inode *inode,
> index = start_blk >> (PAGE_SHIFT - inode->i_blkbits);
> nblocks_in_page = 1U << (PAGE_SHIFT - inode->i_blkbits);
>
> - pagevec_init(&pvec);
> + folio_batch_init(&fbatch);
>
> repeat:
> - pvec.nr = find_get_pages_contig(inode->i_mapping, index, PAGEVEC_SIZE,
> - pvec.pages);
> - if (pvec.nr == 0)
> + nr = filemap_get_folios_contig(inode->i_mapping, &index, ULONG_MAX,
> + &fbatch);
> + if (nr == 0)
> return length;
>
> - if (length > 0 && pvec.pages[0]->index > index)
> - goto out;
> -
> - b = pvec.pages[0]->index << (PAGE_SHIFT - inode->i_blkbits);
> + b = fbatch.folios[0]->index << (PAGE_SHIFT - inode->i_blkbits);
> i = 0;
> do {
> - page = pvec.pages[i];
> + folio = fbatch.folios[i];
>
> - lock_page(page);
> - if (page_has_buffers(page)) {
> + folio_lock(folio);
> + if (folio_buffers(folio)) {
> struct buffer_head *bh, *head;
>
> - bh = head = page_buffers(page);
> + bh = head = folio_buffers(folio);
> do {
> if (b < start_blk)
> continue;
> @@ -532,18 +529,16 @@ unsigned long nilfs_find_uncommitted_extent(struct inode *inode,
>
> b += nblocks_in_page;
Here, It looks like the block index "b" should be updated with the
number of blocks in the
folio because the loop is now per folio, not per page.
Instead of replacing it with a calculation that uses folio_size(folio)
or folio_shift(folio),
I think it would be better to move the calculation of "b" inside the
branch where the folio
has buffers as follows:
if (folio_buffers(folio)) {
struct buffer_head *bh, *head;
sector_t b;
b = folio->index << (PAGE_SHIFT - inode->i_blkbits);
bh = head = folio_buffers(folio);
...
} else if (length > 0) {
goto out_locked;
}
This way, we can remove calculations for the block index "b" outside
the above part
and the variable "nblocks_in_page" as well.
Thanks,
Ryusuke Konishi
> }
> - unlock_page(page);
> + folio_unlock(folio);
>
> - } while (++i < pagevec_count(&pvec));
> + } while (++i < nr);
>
> - index = page->index + 1;
> - pagevec_release(&pvec);
> + folio_batch_release(&fbatch);
> cond_resched();
> goto repeat;
>
> out_locked:
> - unlock_page(page);
> -out:
> - pagevec_release(&pvec);
> + folio_unlock(folio);
> + folio_batch_release(&fbatch);
> return length;
> }
> --
> 2.36.1
>
On Tue, Aug 16, 2022 at 9:33 PM Ryusuke Konishi
<[email protected]> wrote:
>
> On Wed, Aug 17, 2022 at 2:54 AM Vishal Moola (Oracle) wrote:
> >
> > Converted function to use folios throughout. This is in preparation for
> > the removal of find_get_pages_contig(). Now also supports large folios.
> >
> > Also cleaned up an unnecessary if statement - pvec.pages[0]->index > index
> > will always evaluate to false, and filemap_get_folios_contig() returns 0 if
> > there is no folio found at index.
> >
> > Signed-off-by: Vishal Moola (Oracle) <[email protected]>
> > ---
> >
> > v2:
> > - Fixed a warning regarding a now unused label "out"
> > - Reported-by: kernel test robot <[email protected]>
> > ---
> > fs/nilfs2/page.c | 39 +++++++++++++++++----------------------
> > 1 file changed, 17 insertions(+), 22 deletions(-)
> >
> > diff --git a/fs/nilfs2/page.c b/fs/nilfs2/page.c
> > index 3267e96c256c..14629e03d0da 100644
> > --- a/fs/nilfs2/page.c
> > +++ b/fs/nilfs2/page.c
> > @@ -480,13 +480,13 @@ unsigned long nilfs_find_uncommitted_extent(struct inode *inode,
> > sector_t start_blk,
> > sector_t *blkoff)
> > {
> > - unsigned int i;
> > + unsigned int i, nr;
> > pgoff_t index;
> > unsigned int nblocks_in_page;
> > unsigned long length = 0;
> > sector_t b;
> > - struct pagevec pvec;
> > - struct page *page;
> > + struct folio_batch fbatch;
> > + struct folio *folio;
> >
> > if (inode->i_mapping->nrpages == 0)
> > return 0;
> > @@ -494,27 +494,24 @@ unsigned long nilfs_find_uncommitted_extent(struct inode *inode,
> > index = start_blk >> (PAGE_SHIFT - inode->i_blkbits);
> > nblocks_in_page = 1U << (PAGE_SHIFT - inode->i_blkbits);
> >
> > - pagevec_init(&pvec);
> > + folio_batch_init(&fbatch);
> >
> > repeat:
> > - pvec.nr = find_get_pages_contig(inode->i_mapping, index, PAGEVEC_SIZE,
> > - pvec.pages);
> > - if (pvec.nr == 0)
> > + nr = filemap_get_folios_contig(inode->i_mapping, &index, ULONG_MAX,
> > + &fbatch);
> > + if (nr == 0)
> > return length;
> >
> > - if (length > 0 && pvec.pages[0]->index > index)
> > - goto out;
> > -
> > - b = pvec.pages[0]->index << (PAGE_SHIFT - inode->i_blkbits);
> > + b = fbatch.folios[0]->index << (PAGE_SHIFT - inode->i_blkbits);
> > i = 0;
> > do {
> > - page = pvec.pages[i];
> > + folio = fbatch.folios[i];
> >
> > - lock_page(page);
> > - if (page_has_buffers(page)) {
> > + folio_lock(folio);
> > + if (folio_buffers(folio)) {
> > struct buffer_head *bh, *head;
> >
> > - bh = head = page_buffers(page);
> > + bh = head = folio_buffers(folio);
> > do {
> > if (b < start_blk)
> > continue;
> > @@ -532,18 +529,16 @@ unsigned long nilfs_find_uncommitted_extent(struct inode *inode,
> >
>
> > b += nblocks_in_page;
>
> Here, It looks like the block index "b" should be updated with the
> number of blocks in the
> folio because the loop is now per folio, not per page.
Yup, thanks for catching that.
> Instead of replacing it with a calculation that uses folio_size(folio)
> or folio_shift(folio),
> I think it would be better to move the calculation of "b" inside the
> branch where the folio
> has buffers as follows:
>
> if (folio_buffers(folio)) {
> struct buffer_head *bh, *head;
> sector_t b;
>
> b = folio->index << (PAGE_SHIFT - inode->i_blkbits);
> bh = head = folio_buffers(folio);
> ...
> } else if (length > 0) {
> goto out_locked;
> }
>
> This way, we can remove calculations for the block index "b" outside
> the above part
> and the variable "nblocks_in_page" as well.
Sounds good, I'll do that.
> Thanks,
> Ryusuke Konishi
>
> > }
> > - unlock_page(page);
> > + folio_unlock(folio);
> >
> > - } while (++i < pagevec_count(&pvec));
> > + } while (++i < nr);
> >
> > - index = page->index + 1;
> > - pagevec_release(&pvec);
> > + folio_batch_release(&fbatch);
> > cond_resched();
> > goto repeat;
> >
> > out_locked:
> > - unlock_page(page);
> > -out:
> > - pagevec_release(&pvec);
> > + folio_unlock(folio);
> > + folio_batch_release(&fbatch);
> > return length;
> > }
> > --
> > 2.36.1
> >
On Tue, Aug 16, 2022 at 10:53 AM Vishal Moola (Oracle)
<[email protected]> wrote:
>
> Convert to use folios throughout. This is in preparation for the removal of
> find_get_pages_contig(). Now also supports large folios.
>
> Since we may receive more than nr_pages pages, nr_pages may underflow.
> Since nr_pages > 0 is equivalent to index <= end_index, we replaced it
> with this check instead.
>
> Signed-off-by: Vishal Moola (Oracle) <[email protected]>
> ---
> fs/btrfs/extent_io.c | 33 +++++++++++++++------------------
> 1 file changed, 15 insertions(+), 18 deletions(-)
>
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 8f6b544ae616..f16929bc531b 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -1882,9 +1882,8 @@ static int __process_pages_contig(struct address_space *mapping,
> pgoff_t start_index = start >> PAGE_SHIFT;
> pgoff_t end_index = end >> PAGE_SHIFT;
> pgoff_t index = start_index;
> - unsigned long nr_pages = end_index - start_index + 1;
> unsigned long pages_processed = 0;
> - struct page *pages[16];
> + struct folio_batch fbatch;
> int err = 0;
> int i;
>
> @@ -1893,16 +1892,17 @@ static int __process_pages_contig(struct address_space *mapping,
> ASSERT(processed_end && *processed_end == start);
> }
>
> - if ((page_ops & PAGE_SET_ERROR) && nr_pages > 0)
> + if ((page_ops & PAGE_SET_ERROR) && start_index <= end_index)
> mapping_set_error(mapping, -EIO);
>
> - while (nr_pages > 0) {
> - int found_pages;
> + folio_batch_init(&fbatch);
> + while (index <= end_index) {
> + int found_folios;
> +
> + found_folios = filemap_get_folios_contig(mapping, &index,
> + end_index, &fbatch);
>
> - found_pages = find_get_pages_contig(mapping, index,
> - min_t(unsigned long,
> - nr_pages, ARRAY_SIZE(pages)), pages);
> - if (found_pages == 0) {
> + if (found_folios == 0) {
> /*
> * Only if we're going to lock these pages, we can find
> * nothing at @index.
> @@ -1912,23 +1912,20 @@ static int __process_pages_contig(struct address_space *mapping,
> goto out;
> }
>
> - for (i = 0; i < found_pages; i++) {
> + for (i = 0; i < found_folios; i++) {
> int process_ret;
> -
> + struct folio *folio = fbatch.folios[i];
> process_ret = process_one_page(fs_info, mapping,
> - pages[i], locked_page, page_ops,
> + &folio->page, locked_page, page_ops,
> start, end);
> if (process_ret < 0) {
> - for (; i < found_pages; i++)
> - put_page(pages[i]);
> err = -EAGAIN;
> + folio_batch_release(&fbatch);
> goto out;
> }
> - put_page(pages[i]);
> - pages_processed++;
> + pages_processed += folio_nr_pages(folio);
> }
> - nr_pages -= found_pages;
> - index += found_pages;
> + folio_batch_release(&fbatch);
> cond_resched();
> }
> out:
> --
> 2.36.1
>
Just following up on these btrfs patches (2/7, 3/7, 4/7). Does anyone have
time to review them this week? Feedback would be appreciated.
On Tue, Aug 16, 2022 at 10:52:39AM -0700, Vishal Moola (Oracle) wrote:
> This patch series replaces find_get_pages_contig() with
> filemap_get_folios_contig(). I've run xfstests on btrfs. I've also
> tested the ramfs changes. I ran some xfstests on nilfs2, and its
> seemingly fine although more testing may be beneficial.
> ---
>
> v2:
> - Removed an unused label in nilfs2
>
> Vishal Moola (Oracle) (7):
> filemap: Add filemap_get_folios_contig()
> btrfs: Convert __process_pages_contig() to use
> filemap_get_folios_contig()
> btrfs: Convert end_compressed_writeback() to use filemap_get_folios()
> btrfs: Convert process_page_range() to use filemap_get_folios_contig()
> nilfs2: Convert nilfs_find_uncommited_extent() to use
> filemap_get_folios_contig()
> ramfs: Convert ramfs_nommu_get_unmapped_area() to use
> filemap_get_folios_contig()
> filemap: Remove find_get_pages_contig()
>
> fs/btrfs/compression.c | 26 ++++++------
> fs/btrfs/extent_io.c | 33 +++++++--------
> fs/btrfs/subpage.c | 2 +-
> fs/btrfs/tests/extent-io-tests.c | 31 +++++++-------
I've tested the whole branch in my fstest setup, no issues found and
did a light review of the code changes, looks ok as well.
How do you want get the patches merged? As it's an API conversion I can
ack it and let it go via the mm tree. So far there are no conflicts with
our btrfs development patches, I assume it'll be a clean merge in the
future too.
> fs/nilfs2/page.c | 39 ++++++++----------
> fs/ramfs/file-nommu.c | 50 ++++++++++++----------
> include/linux/pagemap.h | 4 +-
> mm/filemap.c | 71 +++++++++++++++++++-------------
> 8 files changed, 134 insertions(+), 122 deletions(-)
>
> --
> 2.36.1
On Tue, Aug 23, 2022 at 07:26:42PM +0200, David Sterba wrote:
> I've tested the whole branch in my fstest setup, no issues found and
> did a light review of the code changes, looks ok as well.
Thanks!
> How do you want get the patches merged? As it's an API conversion I can
> ack it and let it go via the mm tree. So far there are no conflicts with
> our btrfs development patches, I assume it'll be a clean merge in the
> future too.
I was planning on taking this patch series through the pagecache tree;
it's in -next, so any problems will show up as conflicts there.
On Tue, Aug 16, 2022 at 10:52:41AM -0700, Vishal Moola (Oracle) wrote:
> Convert to use folios throughout. This is in preparation for the removal of
> find_get_pages_contig(). Now also supports large folios.
>
> Since we may receive more than nr_pages pages, nr_pages may underflow.
> Since nr_pages > 0 is equivalent to index <= end_index, we replaced it
> with this check instead.
>
> Signed-off-by: Vishal Moola (Oracle) <[email protected]>
Acked-by: David Sterba <[email protected]>
On Tue, Aug 16, 2022 at 10:52:42AM -0700, Vishal Moola (Oracle) wrote:
> Converted function to use folios throughout. This is in preparation for
> the removal of find_get_pages_contig(). Now also supports large folios.
>
> Since we may receive more than nr_pages pages, nr_pages may underflow.
> Since nr_pages > 0 is equivalent to index <= end_index, we replaced it
> with this check instead.
>
> Also this function does not care about the pages being contiguous so we
> can just use filemap_get_folios() to be more efficient.
>
> Signed-off-by: Vishal Moola (Oracle) <[email protected]>
With minor comments
Acked-by: David Sterba <[email protected]>
> --- a/fs/btrfs/compression.c
> +++ b/fs/btrfs/compression.c
> @@ -8,6 +8,7 @@
> #include <linux/file.h>
> #include <linux/fs.h>
> #include <linux/pagemap.h>
> +#include <linux/pagevec.h>
> #include <linux/highmem.h>
> #include <linux/kthread.h>
> #include <linux/time.h>
> @@ -339,8 +340,7 @@ static noinline void end_compressed_writeback(struct inode *inode,
> struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
> unsigned long index = cb->start >> PAGE_SHIFT;
> unsigned long end_index = (cb->start + cb->len - 1) >> PAGE_SHIFT;
> - struct page *pages[16];
> - unsigned long nr_pages = end_index - index + 1;
> + struct folio_batch fbatch;
> const int errno = blk_status_to_errno(cb->status);
> int i;
> int ret;
> @@ -348,24 +348,22 @@ static noinline void end_compressed_writeback(struct inode *inode,
> if (errno)
> mapping_set_error(inode->i_mapping, errno);
>
> - while (nr_pages > 0) {
> - ret = find_get_pages_contig(inode->i_mapping, index,
> - min_t(unsigned long,
> - nr_pages, ARRAY_SIZE(pages)), pages);
> + folio_batch_init(&fbatch);
> + while (index <= end_index) {
> + ret = filemap_get_folios(inode->i_mapping, &index, end_index,
> + &fbatch);
> +
> if (ret == 0) {
> - nr_pages -= 1;
> - index += 1;
> - continue;
> + return;
> }
Please drop { } around single statement
> for (i = 0; i < ret; i++) {
> + struct folio *folio = fbatch.folios[i];
And add a newline after declaration.
> if (errno)
> - SetPageError(pages[i]);
> - btrfs_page_clamp_clear_writeback(fs_info, pages[i],
> + folio_set_error(folio);
> + btrfs_page_clamp_clear_writeback(fs_info, &folio->page,
> cb->start, cb->len);
> - put_page(pages[i]);
> }
> - nr_pages -= ret;
> - index += ret;
> + folio_batch_release(&fbatch);
> }
> /* the inode may be gone now */
> }
> --
> 2.36.1
On Tue, Aug 16, 2022 at 10:52:43AM -0700, Vishal Moola (Oracle) wrote:
> Converted function to use folios throughout. This is in preparation for
> the removal of find_get_pages_contig(). Now also supports large folios.
>
> Since we may receive more than nr_pages pages, nr_pages may underflow.
> Since nr_pages > 0 is equivalent to index <= end_index, we replaced it
> with this check instead.
>
> Also minor comment renaming for consistency in subpage.
>
> Signed-off-by: Vishal Moola (Oracle) <[email protected]>
Acked-by: David Sterba <[email protected]>
> --- a/fs/btrfs/tests/extent-io-tests.c
> +++ b/fs/btrfs/tests/extent-io-tests.c
> @@ -4,6 +4,7 @@
> */
>
> #include <linux/pagemap.h>
> +#include <linux/pagevec.h>
> #include <linux/sched.h>
> #include <linux/slab.h>
> #include <linux/sizes.h>
> @@ -20,39 +21,39 @@ static noinline int process_page_range(struct inode *inode, u64 start, u64 end,
> unsigned long flags)
> {
> int ret;
> - struct page *pages[16];
> + struct folio_batch fbatch;
> unsigned long index = start >> PAGE_SHIFT;
> unsigned long end_index = end >> PAGE_SHIFT;
> - unsigned long nr_pages = end_index - index + 1;
> int i;
> int count = 0;
> int loops = 0;
>
> - while (nr_pages > 0) {
> - ret = find_get_pages_contig(inode->i_mapping, index,
> - min_t(unsigned long, nr_pages,
> - ARRAY_SIZE(pages)), pages);
> + folio_batch_init(&fbatch);
> +
> + while (index <= end_index) {
> + ret = filemap_get_folios_contig(inode->i_mapping, &index,
> + end_index, &fbatch);
> for (i = 0; i < ret; i++) {
> + struct folio *folio = fbatch.folios[i];
Add a newline please
> if (flags & PROCESS_TEST_LOCKED &&
> - !PageLocked(pages[i]))
> + !folio_test_locked(folio))
> count++;
> - if (flags & PROCESS_UNLOCK && PageLocked(pages[i]))
> - unlock_page(pages[i]);
> - put_page(pages[i]);
> + if (flags & PROCESS_UNLOCK && folio_test_locked(folio))
> + folio_unlock(folio);
> if (flags & PROCESS_RELEASE)
> - put_page(pages[i]);
> + folio_put(folio);
> }
> - nr_pages -= ret;
> - index += ret;
> + folio_batch_release(&fbatch);
> cond_resched();
> loops++;
> if (loops > 100000) {
> printk(KERN_ERR
> - "stuck in a loop, start %llu, end %llu, nr_pages %lu, ret %d\n",
> - start, end, nr_pages, ret);
> + "stuck in a loop, start %llu, end %llu, ret %d\n",
> + start, end, ret);
> break;
> }
> }
> +
> return count;
> }
>
> --
> 2.36.1