linux-kernel - Re: Regression in NFS probably due to very large amounts of readahead

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <569d0df0-71d5-4227-aa28-e57cd60bc9f1@gmail.com>
Date: Tue, 26 Nov 2024 18:26:13 +0100
From: Anders Blomdell <anders.blomdell@...il.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: Jan Kara <jack@...e.cz>, Philippe Troin <phil@...i.org>,
 Andrew Morton <akpm@...ux-foundation.org>, linux-fsdevel@...r.kernel.org,
 linux-mm@...ck.org, linux-kernel@...r.kernel.org, NeilBrown <neilb@...e.de>
Subject: Re: Regression in NFS probably due to very large amounts of readahead



On 2024-11-26 17:55, Matthew Wilcox wrote:
> On Tue, Nov 26, 2024 at 04:28:04PM +0100, Anders Blomdell wrote:
>> On 2024-11-26 16:06, Jan Kara wrote:
>>> Hum, checking the history the update of ra->size has been added by Neil two
>>> years ago in 9fd472af84ab ("mm: improve cleanup when ->readpages doesn't
>>> process all pages"). Neil, the changelog seems as there was some real
>>> motivation behind updating of ra->size in read_pages(). What was it? Now I
>>> somewhat disagree with reducing ra->size in read_pages() because it seems
>>> like a wrong place to do that and if we do need something like that,
>>> readahead window sizing logic should rather be changed to take that into
>>> account? But it all depends on what was the real rationale behind reducing
>>> ra->size in read_pages()...
>>
>> My (rather limited) understanding of the patch is that it was intended to read those pages
>> that didn't get read because the allocation of a bigger folio failed, while not redoing what
>> readpages already did; how it was actually going to accomplish that is still unclear to me,
>> but I even don't even quite understand the comment...
>>
>> 	/*
>> 	 * If there were already pages in the page cache, then we may have
>> 	 * left some gaps.  Let the regular readahead code take care of this
>> 	 * situation.
>> 	 */
>>
>> the reason for an unchanged async_size is also beyond my understanding.
> 
> This isn't because we couldn't allocate a folio, this is when we
> allocated folios, tried to read them and we failed to submit the I/O.
> This is a pretty rare occurrence under normal conditions.

I beg to differ, the code is reached when there is
no folio support or ra->size < 4 (not considered in
this discussion) or falling throug when !err, err
is set by:

         err = ra_alloc_folio(ractl, index, mark, order, gfp);
                 if (err)
                         break;

isn't the reading done by:

         read_pages(ractl);

which does not set err!

/Anders