Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 14 Nov 2022 19:12:37 +0100
From:   Michal Hocko <mhocko@suse.com>
To:     Zhongkun He <hezhongkun.hzk@bytedance.com>
Cc:     Andrew Morton <akpm@linux-foundation.org>, corbet@lwn.net,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        linux-api@vger.kernel.org, linux-doc@vger.kernel.org
Subject: Re: [External] Re: [PATCH v2] mm: add new syscall
 pidfd_set_mempolicy().
Message-ID: <Y3KFFfMFE55lVdNZ@dhcp22.suse.cz>
References: <20221111084051.2121029-1-hezhongkun.hzk@bytedance.com>
 <20221111112732.30e1696bcd0d5b711c188a9a@linux-foundation.org>
 <a44f794e-fe60-e261-3631-9107822d5c36@bytedance.com>
 <Y3IqLzvduM6HqPJV@dhcp22.suse.cz>
 <3a3b4f5b-14d1-27d8-7727-cf23da90988f@bytedance.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <3a3b4f5b-14d1-27d8-7727-cf23da90988f@bytedance.com>
Precedence: bulk

On Mon 14-11-22 23:12:00, Zhongkun He wrote:
> Sorry,michal. I dont know if my expression is accurate.
> > 
> > We shouldn't really rely on mmap_sem for this IMO.
> 
>  Yes, We should rely on mmap_sem for vma->vm_policy,but not for
>  process context policy(task->mempolicy).

But the caller has no way to know which kind of policy is returned so
the locking cannot be conditional on the policy type.

> > There is alloc_lock
> > (aka task lock) that makes sure the policy is stable so that caller can
> > atomically take a reference and hold on the policy. And we do not do
> > that consistently and this should be fixed.
> 
> I saw some explanations in the doc("numa_memory_policy.rst") and
> comments(mempolcy.h) why not use locks and reference in page
> allocation:
> 
> In process context there is no locking because only the process accesses
> its own state.
> 
> During run-time "usage" of the policy, we attempt to minimize atomic
> operations on the reference count, as this can lead to cache lines
> bouncing between cpus and NUMA nodes.

Yes this is all understood but the level of the overhead is not really
clear. So the question is whether this will induce a visible overhead.
Because from the maintainability point of view it is much less costly to
have a clear life time model. Right now we have a mix of reference
counting and per-task requirements which is rather subtle and easy to
get wrong. In an ideal world we would have get_vma_policy always
returning a reference counted policy or NULL. If we really need to
optimize for cache line bouncing we can go with per cpu reference
counters (something that was not available at the time the mempolicy
code has been introduced).

So I am not saying that the task_work based solution is not possible I
just think that this looks like a good opportunity to get from the
existing subtle model.
-- 
Michal Hocko
SUSE Labs