Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230107173635.2025233-1-trondmy@kernel.org> <20230107173635.2025233-2-trondmy@kernel.org>
 <20230107173635.2025233-3-trondmy@kernel.org> <20230107173635.2025233-4-trondmy@kernel.org>
 <20230107173635.2025233-5-trondmy@kernel.org> <20230107173635.2025233-6-trondmy@kernel.org>
 <20230107173635.2025233-7-trondmy@kernel.org> <20230107173635.2025233-8-trondmy@kernel.org>
 <20230107173635.2025233-9-trondmy@kernel.org> <20230107173635.2025233-10-trondmy@kernel.org>
In-Reply-To: <20230107173635.2025233-10-trondmy@kernel.org>
From:   Anna Schumaker <schumaker.anna@gmail.com>
Date:   Wed, 11 Jan 2023 11:18:19 -0500
Message-ID: <CAFX2Jf=HxgJmuE0CjUcFbw1pekcpYqm7aigeb_XY0rG5Gbje_Q@mail.gmail.com>
Subject: Re: [PATCH 09/17] NFS: Convert the function nfs_wb_page() to use folios
To:     trondmy@kernel.org
Cc:     linux-nfs@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Trond,

On Sat, Jan 7, 2023 at 12:43 PM <trondmy@kernel.org> wrote:
>
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
>
> Signed-off-by: Trond Myklebust <trond.myklebust@hammerspace.com>
> ---
>  fs/nfs/write.c | 35 ++++++++++++++++-------------------
>  1 file changed, 16 insertions(+), 19 deletions(-)
>
> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
> index 0fbb119022d9..14f98c452595 100644
> --- a/fs/nfs/write.c
> +++ b/fs/nfs/write.c
> @@ -2069,13 +2069,18 @@ int nfs_wb_folio_cancel(struct inode *inode, struct folio *folio)
>         return ret;
>  }
>
> -/*
> - * Write back all requests on one page - we do this before reading it.
> +/**
> + * nfs_wb_folio - Write back all requests on one page
> + * @inode: pointer to page
> + * @folio: pointer to folio
> + *
> + * Assumes that the folio has been locked by the caller, and will
> + * not unlock it.
>   */
> -int nfs_wb_page(struct inode *inode, struct page *page)
> +int nfs_wb_folio(struct inode *inode, struct folio *folio)
>  {
> -       loff_t range_start = page_file_offset(page);
> -       loff_t range_end = range_start + (loff_t)(PAGE_SIZE - 1);
> +       loff_t range_start = folio_file_pos(folio);
> +       loff_t range_end = range_start + (loff_t)folio_size(folio) - 1;
>         struct writeback_control wbc = {
>                 .sync_mode = WB_SYNC_ALL,
>                 .nr_to_write = 0,
> @@ -2087,15 +2092,15 @@ int nfs_wb_page(struct inode *inode, struct page *page)
>         trace_nfs_writeback_page_enter(inode);
>
>         for (;;) {
> -               wait_on_page_writeback(page);
> -               if (clear_page_dirty_for_io(page)) {
> -                       ret = nfs_writepage_locked(page, &wbc);
> +               folio_wait_writeback(folio);
> +               if (folio_clear_dirty_for_io(folio)) {
> +                       ret = nfs_writepage_locked(folio, &wbc);

nfs_writepage_locked() still takes a "struct page *" until the next
patch (NFS: Convert buffered writes to use folios), so I'm seeing this
when I try to compile:

fs/nfs/write.c:2097:31: error: incompatible pointer types passing
'struct folio *' to parameter of type 'struct page *'
[-Werror,-Wincompatible-pointer-types]
                        ret = nfs_writepage_locked(folio, &wbc);
                                                   ^~~~~
fs/nfs/write.c:661:46: note: passing argument to parameter 'page' here
static int nfs_writepage_locked(struct page *page,
                                             ^
1 error generated.
make[3]: *** [scripts/Makefile.build:252: fs/nfs/write.o] Error 1
make[2]: *** [scripts/Makefile.build:504: fs/nfs] Error 2
make[1]: *** [scripts/Makefile.build:504: fs] Error 2
make: *** [Makefile:2008: .] Error 2


>                         if (ret < 0)
>                                 goto out_error;
>                         continue;
>                 }
>                 ret = 0;
> -               if (!PagePrivate(page))
> +               if (!folio_test_private(folio))
>                         break;
>                 ret = nfs_commit_inode(inode, FLUSH_SYNC);
>                 if (ret < 0)
> @@ -2106,17 +2111,9 @@ int nfs_wb_page(struct inode *inode, struct page *page)
>         return ret;
>  }
>
> -/**
> - * nfs_wb_folio - Write back all requests on one page
> - * @inode: pointer to page
> - * @folio: pointer to folio
> - *
> - * Assumes that the folio has been locked by the caller, and will
> - * not unlock it.
> - */
> -int nfs_wb_folio(struct inode *inode, struct folio *folio)
> +int nfs_wb_page(struct inode *inode, struct page *page)
>  {
> -       return nfs_wb_page(inode, &folio->page);
> +       return nfs_wb_folio(inode, page_folio(page));
>  }
>
>  #ifdef CONFIG_MIGRATION
> --
> 2.39.0
>