On Sat 29-06-19 20:24:34, Tetsuo Handa wrote:
> Since mpol_put_task_policy() in do_exit() sets mempolicy = NULL,
> mempolicy_nodemask_intersects() considers exited threads (e.g. a process
> with dying leader and live threads) as eligible. But it is possible that
> all of live threads are still ineligible.
>
> Since has_intersects_mems_allowed() returns true as soon as one of threads
> is considered eligible, mempolicy_nodemask_intersects() needs to consider
> exited threads as ineligible. Since exit_mm() in do_exit() sets mm = NULL
> before mpol_put_task_policy() sets mempolicy = NULL, we can exclude exited
> threads by checking whether mm is NULL.
Ok, this makes sense. For this change
Acked-by: Michal Hocko <mhocko@xxxxxxxx>
> While at it, since mempolicy_nodemask_intersects() is called by only
> has_intersects_mems_allowed(), it is guaranteed that mask != NULL.
Well, I am not really sure. It's true that mempolicy_nodemask_intersects
is currently only used by OOM path and never with mask == NULL but
the function seems to be more generic and hadnling NULL mask seems
reasonable to me. This is not a hot path that an additional check would
be harmful, right?
> BTW, are there processes where some of threads use MPOL_{BIND,INTERLEAVE}
> and the rest do not use MPOL_{BIND,INTERLEAVE} ? If no, we can use
> find_lock_task_mm() instead of for_each_thread() for mask != NULL case
> in has_intersects_mems_allowed().
I am afraid that mempolicy is allowed to be per thread which is quite
ugly and I am afraid we cannot change that right now.
> Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> ---
> mm/mempolicy.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/mm/mempolicy.c b/mm/mempolicy.c
> index 01600d8..938f0a0 100644
> --- a/mm/mempolicy.c
> +++ b/mm/mempolicy.c
> @@ -1974,11 +1974,10 @@ bool mempolicy_nodemask_intersects(struct task_struct *tsk,
> const nodemask_t *mask)
> {
> struct mempolicy *mempolicy;
> - bool ret = true;
> + bool ret;
>
> - if (!mask)
> - return ret;
> task_lock(tsk);
> + ret = tsk->mm;
> mempolicy = tsk->mempolicy;
> if (!mempolicy)
> goto out;
> --
> 1.8.3.1
--
Michal Hocko
SUSE Labs
