All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergey Senozhatsky <senozhatsky@chromium.org>
To: Nhat Pham <nphamcs@gmail.com>, Minchan Kim <minchan@kernel.org>
Cc: Johannes Weiner <hannes@cmpxchg.org>,
	Sergey Senozhatsky <senozhatsky@chromium.org>,
	akpm@linux-foundation.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, ngupta@vflare.org,
	sjenning@redhat.com, ddstreet@ieee.org, vitaly.wool@konsulko.com,
	kernel-team@meta.com
Subject: Re: [PATCH] zsmalloc: move LRU update from zs_map_object() to zs_malloc()
Date: Tue, 9 May 2023 12:00:30 +0900	[thread overview]
Message-ID: <20230509030030.GD11511@google.com> (raw)
In-Reply-To: <CAKEwX=MtunOe6A--SG3ud-gUFg3bXFJgG4csgwHeZFAEqjCgHg@mail.gmail.com>

On (23/05/08 09:00), Nhat Pham wrote:
> > The deeper bug here is that zs_map_object() tries to add the page to
> > the LRU list while the shrinker has it isolated for reclaim. This is
> > way too sutble and error prone. Even if it worked now, it'll cause
> > corruption issues down the line.
> >
> > For example, Nhat is adding a secondary entry point to reclaim.
> > Reclaim expects that a page that's on the LRU is also on the fullness
> > list, so this would lead to a double remove_zspage() and BUG_ON().
> >
> > This patch doesn't just fix the crash, it eliminates the deeper LRU
> > isolation issue and makes the code more robust and simple.
> 
> I agree. IMO, less unnecessary concurrent interaction is always a
> win for developers' and maintainers' cognitive load.

Thanks for all the explanations.

> As a side benefit - this also gets rid of the inelegant check
> (mm == ZS_MM_WO). The fact that we had to include a
> a multi-paragraph explanation for a 3-line piece of code
> should have been a red flag.

Minchan had some strong opinion on that, so we need to hear from him
before we decide how do we fix it.


  reply	other threads:[~2023-05-09  3:00 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-05 18:50 [PATCH] zsmalloc: move LRU update from zs_map_object() to zs_malloc() Nhat Pham
2023-05-05 19:14 ` Andrew Morton
2023-05-05 19:26   ` Nhat Pham
2023-05-05 19:29   ` Johannes Weiner
2023-05-06  3:01 ` Sergey Senozhatsky
2023-05-08 14:06   ` Johannes Weiner
2023-05-08 16:00     ` Nhat Pham
2023-05-09  3:00       ` Sergey Senozhatsky [this message]
2023-05-09 17:44         ` Johannes Weiner
2023-05-09 18:20           ` Minchan Kim
2023-05-09 19:24             ` Johannes Weiner
2023-05-09 22:04               ` Nhat Pham
2023-05-10  0:39 ` Sergey Senozhatsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230509030030.GD11511@google.com \
    --to=senozhatsky@chromium.org \
    --cc=akpm@linux-foundation.org \
    --cc=ddstreet@ieee.org \
    --cc=hannes@cmpxchg.org \
    --cc=kernel-team@meta.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=minchan@kernel.org \
    --cc=ngupta@vflare.org \
    --cc=nphamcs@gmail.com \
    --cc=sjenning@redhat.com \
    --cc=vitaly.wool@konsulko.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.