Re: [PATCH v2 07/12] btrfs: make prepare_pages nowait compatible

[Filipe Manana <fdmanana@kernel.org>]

On Thu, Sep 8, 2022 at 1:26 AM Stefan Roesch <shr@fb.com> wrote:
>
> Add nowait parameter to the prepare_pages function. In case nowait is
> specified for an async buffered write request, do a nowait allocation or
> return -EAGAIN.
>
> Signed-off-by: Stefan Roesch <shr@fb.com>
> ---
>  fs/btrfs/file.c | 43 ++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/fs/btrfs/file.c b/fs/btrfs/file.c
> index cf19d381ead6..a154a3cec44b 100644
> --- a/fs/btrfs/file.c
> +++ b/fs/btrfs/file.c
> @@ -1339,26 +1339,55 @@ static int prepare_uptodate_page(struct inode *inode,
>         return 0;
>  }
>
> +static int get_prepare_fgp_flags(bool nowait)
> +{
> +       int fgp_flags;
> +
> +       fgp_flags = FGP_LOCK|FGP_ACCESSED|FGP_CREAT;

Please follow the existing code style and add a space before and after
each bitwise or operator.
Not only does it conform to the btrfs style, it's also easier to read.

The assignment could also be done when declaring the variable, since
it's short and simple.

Thanks.

> +       if (nowait)
> +               fgp_flags |= FGP_NOWAIT;
> +
> +       return fgp_flags;
> +}
> +
> +static gfp_t get_prepare_gfp_flags(struct inode *inode, bool nowait)
> +{
> +       gfp_t gfp;
> +
> +       gfp = btrfs_alloc_write_mask(inode->i_mapping);
> +       if (nowait) {
> +               gfp &= ~__GFP_DIRECT_RECLAIM;
> +               gfp |= GFP_NOWAIT;
> +       }
> +
> +       return gfp;
> +}
> +
>  /*
>   * this just gets pages into the page cache and locks them down.
>   */
>  static noinline int prepare_pages(struct inode *inode, struct page **pages,
>                                   size_t num_pages, loff_t pos,
> -                                 size_t write_bytes, bool force_uptodate)
> +                                 size_t write_bytes, bool force_uptodate,
> +                                 bool nowait)
>  {
>         int i;
>         unsigned long index = pos >> PAGE_SHIFT;
> -       gfp_t mask = btrfs_alloc_write_mask(inode->i_mapping);
> +       gfp_t mask = get_prepare_gfp_flags(inode, nowait);
> +       int fgp_flags = get_prepare_fgp_flags(nowait);
>         int err = 0;
>         int faili;
>
>         for (i = 0; i < num_pages; i++) {
>  again:
> -               pages[i] = find_or_create_page(inode->i_mapping, index + i,
> -                                              mask | __GFP_WRITE);
> +               pages[i] = pagecache_get_page(inode->i_mapping, index + i,
> +                                       fgp_flags, mask | __GFP_WRITE);
>                 if (!pages[i]) {
>                         faili = i - 1;
> -                       err = -ENOMEM;
> +                       if (nowait)
> +                               err = -EAGAIN;
> +                       else
> +                               err = -ENOMEM;
>                         goto fail;
>                 }
>
> @@ -1376,7 +1405,7 @@ static noinline int prepare_pages(struct inode *inode, struct page **pages,
>                                                     pos + write_bytes, false);
>                 if (err) {
>                         put_page(pages[i]);
> -                       if (err == -EAGAIN) {
> +                       if (!nowait && err == -EAGAIN) {
>                                 err = 0;
>                                 goto again;
>                         }
> @@ -1716,7 +1745,7 @@ static noinline ssize_t btrfs_buffered_write(struct kiocb *iocb,
>                  */
>                 ret = prepare_pages(inode, pages, num_pages,
>                                     pos, write_bytes,
> -                                   force_page_uptodate);
> +                                   force_page_uptodate, false);
>                 if (ret) {
>                         btrfs_delalloc_release_extents(BTRFS_I(inode),
>                                                        reserve_bytes);
> --
> 2.30.2
>