Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <b57c76b8-aaf5-d245-6c0e-a3afdfd96643@samsung.com>
Date:   Wed, 21 Jun 2023 12:25:45 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
        Thunderbird/102.11.0
Subject: Re: [RFC 2/4] filemap: use minimum order while allocating folios
To:     Hannes Reinecke <hare@suse.de>, <willy@infradead.org>,
        <david@fromorbit.com>
CC:     <gost.dev@samsung.com>, <mcgrof@kernel.org>, <hch@lst.de>,
        <jwong@kernel.org>, <linux-fsdevel@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>
Content-Language: en-US
From:   Pankaj Raghav <p.raghav@samsung.com>
In-Reply-To: <d0b77326-e93f-c1dc-c46c-1213bfafd7ee@suse.de>
Content-Transfer-Encoding: 8bit
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 201P
References: <20230621083823.1724337-1-p.raghav@samsung.com>
        <CGME20230621083827eucas1p2948b4efaf55064c3761c924b5b049219@eucas1p2.samsung.com>
        <20230621083823.1724337-3-p.raghav@samsung.com>
        <d0b77326-e93f-c1dc-c46c-1213bfafd7ee@suse.de>
Precedence: bulk

>> index 47afbca1d122..090b810ddeed 100644
>> --- a/mm/readahead.c
>> +++ b/mm/readahead.c
>> @@ -245,7 +245,8 @@ void page_cache_ra_unbounded(struct readahead_control *ractl,
>>               continue;
>>           }
>>   -        folio = filemap_alloc_folio(gfp_mask, 0);
>> +        folio = filemap_alloc_folio(gfp_mask,
>> +                        mapping_min_folio_order(mapping));
>>           if (!folio)
>>               break;
>>           if (filemap_add_folio(mapping, folio, index + i,
>> @@ -259,7 +260,8 @@ void page_cache_ra_unbounded(struct readahead_control *ractl,
>>           if (i == nr_to_read - lookahead_size)
>>               folio_set_readahead(folio);
>>           ractl->_workingset |= folio_test_workingset(folio);
>> -        ractl->_nr_pages++;
>> +        ractl->_nr_pages += folio_nr_pages(folio);
>> +        i += folio_nr_pages(folio) - 1;
>>       }
>>         /*
> This is incomplete, as the loop above has some exit statements which blindly step backwards by one
> page.
> 
> I found it better to rework the 'for' into a 'while' loop; please check the attached patch.
> 
Taken from your patch:

@@ -240,8 +240,8 @@ void page_cache_ra_unbounded(struct readahead_control *ractl,
 			 * not worth getting one just for that.
 			 */
 			read_pages(ractl);
-			ractl->_index++;
-			i = ractl->_index + ractl->_nr_pages - index - 1;
+			ractl->_index += folio_nr_pages(folio);
+			i = ractl->_index + ractl->_nr_pages - index;

IIUC, we don't need to update the _index after read_pages() as it already modifies it. We just need
to move ractl->_index by 1 to move to the next index.


> Cheers,
> 
> Hannes