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

[Stefan Roesch <shr@fb.com>]

On 9/8/22 3:17 AM, Filipe Manana wrote:
> > 
> 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.

I added the space and moved the assignment to the definition.
> 
>> +       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
>>