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

Stefan Roesch
2022-09-08T18:33:53Z

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
>>