Re: [Cluster-devel] [PATCH v5 17/23] gfs2: Convert gfs2_write_cache_jdata() to use filemap_get_folios_tag()

Andreas Gruenbacher
2023-01-06T07:58:19Z

On Wed, Jan 4, 2023 at 10:15 PM Vishal Moola (Oracle)
<vishal.moola@gmail.com> wrote:
> Converted function to use folios throughout. This is in preparation for
> the removal of find_get_pgaes_range_tag(). This change removes 8 calls
> to compound_head().
>
> Also had to modify and rename gfs2_write_jdata_pagevec() to take in
> and utilize folio_batch rather than pagevec and use folios rather
> than pages. gfs2_write_jdata_batch() now supports large folios.
>
> Signed-off-by: Vishal Moola (Oracle) <vishal.moola@gmail.com>
> ---
> fs/gfs2/aops.c | 64 +++++++++++++++++++++++++++-----------------------
> 1 file changed, 35 insertions(+), 29 deletions(-)
>
> diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
> index e782b4f1d104..0a47068f9acc 100644
> --- a/fs/gfs2/aops.c
> +++ b/fs/gfs2/aops.c
> @@ -195,67 +195,71 @@ static int gfs2_writepages(struct address_space *mapping,
> }
>
> /**
> - * gfs2_write_jdata_pagevec - Write back a pagevec's worth of pages
> + * gfs2_write_jdata_batch - Write back a folio batch's worth of folios
> * @mapping: The mapping
> * @wbc: The writeback control
> - * @pvec: The vector of pages
> - * @nr_pages: The number of pages to write
> + * @fbatch: The batch of folios
> * @done_index: Page index
> *
> * Returns: non-zero if loop should terminate, zero otherwise
> */
>
> -static int gfs2_write_jdata_pagevec(struct address_space *mapping,
> +static int gfs2_write_jdata_batch(struct address_space *mapping,
> struct writeback_control *wbc,
> - struct pagevec *pvec,
> - int nr_pages,
> + struct folio_batch *fbatch,
> pgoff_t *done_index)
> {
> struct inode *inode = mapping->host;
> struct gfs2_sbd *sdp = GFS2_SB(inode);
> - unsigned nrblocks = nr_pages * (PAGE_SIZE >> inode->i_blkbits);
> + unsigned nrblocks;
> int i;
> int ret;
> + int nr_pages = 0;
> + int nr_folios = folio_batch_count(fbatch);
> +
> + for (i = 0; i < nr_folios; i++)
> + nr_pages += folio_nr_pages(fbatch->folios[i]);
> + nrblocks = nr_pages * (PAGE_SIZE >> inode->i_blkbits);
>
> ret = gfs2_trans_begin(sdp, nrblocks, nrblocks);
> if (ret < 0)
> return ret;
>
> - for(i = 0; i < nr_pages; i++) {
> - struct page *page = pvec->pages[i];
> + for (i = 0; i < nr_folios; i++) {
> + struct folio *folio = fbatch->folios[i];
>
> - *done_index = page->index;
> + *done_index = folio->index;
>
> - lock_page(page);
> + folio_lock(folio);
>
> - if (unlikely(page->mapping != mapping)) {
> + if (unlikely(folio->mapping != mapping)) {
> continue_unlock:
> - unlock_page(page);
> + folio_unlock(folio);
> continue;
> }
>
> - if (!PageDirty(page)) {
> + if (!folio_test_dirty(folio)) {
> /* someone wrote it for us */
> goto continue_unlock;
> }
>
> - if (PageWriteback(page)) {
> + if (folio_test_writeback(folio)) {
> if (wbc->sync_mode != WB_SYNC_NONE)
> - wait_on_page_writeback(page);
> + folio_wait_writeback(folio);
> else
> goto continue_unlock;
> }
>
> - BUG_ON(PageWriteback(page));
> - if (!clear_page_dirty_for_io(page))
> + BUG_ON(folio_test_writeback(folio));
> + if (!folio_clear_dirty_for_io(folio))
> goto continue_unlock;
>
> trace_wbc_writepage(wbc, inode_to_bdi(inode));
>
> - ret = __gfs2_jdata_writepage(page, wbc);
> + ret = __gfs2_jdata_writepage(&folio->page, wbc);
> if (unlikely(ret)) {
> if (ret == AOP_WRITEPAGE_ACTIVATE) {
> - unlock_page(page);
> + folio_unlock(folio);
> ret = 0;
> } else {
>
> @@ -268,7 +272,8 @@ static int gfs2_write_jdata_pagevec(struct address_space *mapping,
> * not be suitable for data integrity
> * writeout).
> */
> - *done_index = page->index + 1;
> + *done_index = folio->index +
> + folio_nr_pages(folio);
> ret = 1;
> break;
> }
> @@ -305,8 +310,8 @@ static int gfs2_write_cache_jdata(struct address_space *mapping,
> {
> int ret = 0;
> int done = 0;
> - struct pagevec pvec;
> - int nr_pages;
> + struct folio_batch fbatch;
> + int nr_folios;
> pgoff_t writeback_index;
> pgoff_t index;
> pgoff_t end;
> @@ -315,7 +320,7 @@ static int gfs2_write_cache_jdata(struct address_space *mapping,
> int range_whole = 0;
> xa_mark_t tag;
>
> - pagevec_init(&pvec);
> + folio_batch_init(&fbatch);
> if (wbc->range_cyclic) {
> writeback_index = mapping->writeback_index; /* prev offset */
> index = writeback_index;
> @@ -341,17 +346,18 @@ static int gfs2_write_cache_jdata(struct address_space *mapping,
> tag_pages_for_writeback(mapping, index, end);
> done_index = index;
> while (!done && (index <= end)) {
> - nr_pages = pagevec_lookup_range_tag(&pvec, mapping, &index, end,
> - tag);
> - if (nr_pages == 0)
> + nr_folios = filemap_get_folios_tag(mapping, &index, end,
> + tag, &fbatch);
> + if (nr_folios == 0)
> break;
>
> - ret = gfs2_write_jdata_pagevec(mapping, wbc, &pvec, nr_pages, &done_index);
> + ret = gfs2_write_jdata_batch(mapping, wbc, &fbatch,
> + &done_index);
> if (ret)
> done = 1;
> if (ret > 0)
> ret = 0;
> - pagevec_release(&pvec);
> + folio_batch_release(&fbatch);
> cond_resched();
> }
>
> --
> 2.38.1
>

Reviewed-by: Andreas Gruenbacher <agruenba@redhat.com>

Thanks,
Andreas