Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Mon, 20 Sep 2021 08:55:15 -0700
From:   Sultan Alsawaf <sultan@kerneltoast.com>
To:     Michal Hocko <mhocko@suse.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>,
        David Rientjes <rientjes@google.com>,
        Mel Gorman <mgorman@suse.de>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mm: Mark the OOM reaper thread as freezable
Message-ID: <YUiu42krQjSTVPnc@sultan-box.localdomain>
References: <20210918233920.9174-1-sultan@kerneltoast.com>
 <YUiBRdrkjIdB/rSN@dhcp22.suse.cz>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <YUiBRdrkjIdB/rSN@dhcp22.suse.cz>
Precedence: bulk

On Mon, Sep 20, 2021 at 02:40:37PM +0200, Michal Hocko wrote:
> What is the actual problem you are trying to solve here.

There isn't any specific problem I'm trying to solve here; simply that, it
appeared as though you intended for the reaper thread to be freezable when it
actually isn't. The OOM killer is disabled after processes are frozen though so
I guess it could be considered a matter of consistency to freeze the reaper
thread too.

Do you remember why you used wait_event_freezable()?

> Freezer details are hairy and I have to re-learn them each time again and
> again but from what I remember wait_event_freezable doesn't really depend on
> tyask being freezable. It tells the freezer that the task is OK to exclude
> while it is sleeping and that should be just the case for the oom reaper. Or
> am I missing something?

The task indeed doesn't need to be freezable, but the rest of what you remember
isn't quite true. It tells the freezer to exclude the task only because the task
will handle entering the freezer on its own. When a task sleeps on
wait_event_freezable(), it will be woken up when system-wide freezing starts,
and then it will try to freeze itself (see freezable_schedule() and
freezer_count()).

If the freezer bits here are undesired then I think wait_event_interruptible()
should be used instead.

Sultan