linux-kernel - Re: [PATCH 5/5] swap: avoid ra statistic lost when swapin races

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: <CAMgjq7AuY_oi9Rw2r8rnA_tr7NeEXAd0OgZ4sNz4CiVV+2i9Rg@mail.gmail.com>
Date:   Fri, 9 Dec 2022 09:54:31 +0800
From:   Kairui Song <ryncsn@...il.com>
To:     Matthew Wilcox <willy@...radead.org>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        Andrew Morton <akpm@...ux-foundation.org>,
        Miaohe Lin <linmiaohe@...wei.com>,
        David Hildenbrand <david@...hat.com>,
        "Huang, Ying" <ying.huang@...el.com>,
        Hugh Dickins <hughd@...gle.com>
Subject: Re: [PATCH 5/5] swap: avoid ra statistic lost when swapin races

Matthew Wilcox <willy@...radead.org> 于2022年12月9日周五 03:14写道：
>

Hi, thanks for the review.

> On Fri, Dec 09, 2022 at 02:02:09AM +0800, Kairui Song wrote:
> > From: Kairui Song <kasong@...cent.com>
> >
> > __read_swap_cache_async should just call swap_cache_get_folio for trying
> > to look up the swap cache. Because swap_cache_get_folio handles the
> > readahead statistic, and clears the RA flag, looking up the cache
> > directly will skip these parts.
> >
> > And the comment no longer applies after commit 442701e7058b
> > ("mm/swap: remove swap_cache_info statistics"), just remove them.
>
> But what about the readahead stats?
>

Shouldn't readahead stats be accounted here? __read_swap_cache_async
is called by swap read in path, if it hits the swap cache, and the
page have readahead page flag set, then accounting that readahead
should be just the right thing todo. And the readahead flag is checked
with folio_test_clear_readahead, so there should be no issue about
repeated accounting.

Only the addr info of the swap_readahead_info could be updated for
multiple times by racing readers, but I think that seems fine, since
we don't know which swap read comes later in case of race, just let
the last reader that hits the swap cache update the address info of
readahead makes sense to me.

Or do you mean I should update the comment about the readahead stat
instead of just drop the commnet?