linux-kernel - Re: [PATCH] mm: zswap: fix the lack of page lru flag in zswap_writeback

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]

Date: Fri, 29 Dec 2023 18:09:07 -0800
From: Nhat Pham <nphamcs@...il.com>
To: Zhongkun He <hezhongkun.hzk@...edance.com>
Cc: akpm@...ux-foundation.org, hannes@...xchg.org, yosryahmed@...gle.com, 
	sjenning@...hat.com, ddstreet@...e.org, vitaly.wool@...sulko.com, 
	linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: zswap: fix the lack of page lru flag in zswap_writeback_entry

On Tue, Oct 24, 2023 at 7:27 AM Zhongkun He
<hezhongkun.hzk@...edance.com> wrote:
>

My apologies for the delayed response. I have a couple of questions.

> The zswap_writeback_entry() will add a page to the swap cache, decompress
> the entry data into the page, and issue a bio write to write the page back
> to the swap device. Move the page to the tail of lru list  through
> SetPageReclaim(page) and folio_rotate_reclaimable().
>
> Currently, about half of the pages will fail to move to the tail of lru

May I ask what's the downstream effect of this? i.e so what if it
fails? And yes, as Andrew pointed out, it'd be nice if the patch
changelog spells out any observable or measurable change from the
user's POV.

> list because there is no LRU flag in page which is not in the LRU list but
> the cpu_fbatches. So fix it.

This sentence is a bit confusing to me. Does this mean the page
currently being processed for writeback is not in the LRU list
(!PageLRU(page)), but IN one of the cpu folio batches? Which makes
folio_rotate_reclaimable() fails on this page later on in the
_swap_writepage() path? (hence the necessity of lru_add_drain()?)

Let me know if I'm misunderstanding the intention of this patch. I
know it's a bit pedantic, but spelling things out (ideally in the
changelog itself) will help the reviewers, as well as future
contributors who want to study the codebase and make changes to it.

>
> Signed-off-by: Zhongkun He <hezhongkun.hzk@...edance.com>

Thanks and look forward to your response,
Nhat

P/S: Have a nice holiday season and happy new year!