From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id B377DC4332F for ; Mon, 14 Nov 2022 15:12:09 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E2D498E0001; Mon, 14 Nov 2022 10:12:08 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id DB60F6B0073; Mon, 14 Nov 2022 10:12:08 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C56A98E0001; Mon, 14 Nov 2022 10:12:08 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id B81DB6B0072 for ; Mon, 14 Nov 2022 10:12:08 -0500 (EST) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay01.hostedemail.com (Postfix) with ESMTP id 856661C1E18 for ; Mon, 14 Nov 2022 15:12:08 +0000 (UTC) X-FDA: 80132388336.22.348028B Received: from mail-pf1-f171.google.com (mail-pf1-f171.google.com [209.85.210.171]) by imf22.hostedemail.com (Postfix) with ESMTP id DA97FC0010 for ; Mon, 14 Nov 2022 15:12:07 +0000 (UTC) Received: by mail-pf1-f171.google.com with SMTP id k15so11278945pfg.2 for ; Mon, 14 Nov 2022 07:12:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to:subject :user-agent:mime-version:date:message-id:from:to:cc:subject:date :message-id:reply-to; bh=e/0liD4zfDdUwy8XG2a2Y/qRgR4CN54ZKUBk4lToHno=; b=riU8FvWGPfAZqyj8WmPJvGzPQ68RHcW8+D48NwSgZ3lNAnI2AXpGhtHVf3bl6q+qZ9 pbhT1osj/KzHcAgUo3UPhTPY+czk+QfGN190xCV7kRBhB1/yAJpiYZOwf/LMv+Xi/cwp WGJ2a1qhdA+ztabSdiNTnN9ZJFJF4npNykY/RWPvvkhMeSXFCO9HSAkD536ubLp41xwy MO2VuanYlZE3CMyuc9Cg2XRIiZ7Ga0L/+i6AFFYP4N/yBB04zjtXkL8kOsIKYL2/d6n7 +yiWi7psl/h2Wjs4VuexYYEpW+T1uEb6l8MoZi4/YJ0oVcv0BdpGGYgDEGssuM7/kCro aA0A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to:subject :user-agent:mime-version:date:message-id:x-gm-message-state:from:to :cc:subject:date:message-id:reply-to; bh=e/0liD4zfDdUwy8XG2a2Y/qRgR4CN54ZKUBk4lToHno=; b=203q8cuLqRP3CseQmWqrnxhNzPX/bS+PQgY1lbvyvBDO92czwTNj7g6/wqucYfXmuO mWFBC7AbbLh79QGcs2dayDEkb1ets+JvOc8NiBF+cg2dDjqLVy7UFNQvZcowE9tAMep5 Hh3xwu04THY264Lv35fsWjhAfrSeW3oEeJ+O9R/ikY5at/Xd/eD0p2UiT9+mpQGEPlEm XQcfDuKlCh6VKHgu1BePNxNEQwQqkiG2Iivkuewk0AbIctqxoGA9d9OtmENUU5ICudHd R67dCVrswj1Ir123VNp2q7di3WIXKSAw/15/RFd0IavSi1bNMxDHDtaZfFwZxsBtNWc7 WD6w== X-Gm-Message-State: ANoB5pmGXVC0el28vz9IPGD0GEE9Y4pRxigQHP8hHErQj2P7e5x5Mc91 H1FwzeK3U0SI8bYu0bw+LzyEVA== X-Google-Smtp-Source: AA0mqf5deRRQMkg46X2uMf/zlBQAB+/Exhx4ssc553jIiUej9t4Dih0k5QiaoT6krFeU5qMYoAQI5Q== X-Received: by 2002:a63:d143:0:b0:45c:5a74:9a92 with SMTP id c3-20020a63d143000000b0045c5a749a92mr12351490pgj.473.1668438726461; Mon, 14 Nov 2022 07:12:06 -0800 (PST) Received: from [10.68.76.92] ([139.177.225.229]) by smtp.gmail.com with ESMTPSA id 132-20020a62148a000000b0056c814a501dsm7100210pfu.10.2022.11.14.07.12.02 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 14 Nov 2022 07:12:05 -0800 (PST) Message-ID: <3a3b4f5b-14d1-27d8-7727-cf23da90988f@bytedance.com> Date: Mon, 14 Nov 2022 23:12:00 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.4.1 Subject: Re: [External] Re: [PATCH v2] mm: add new syscall pidfd_set_mempolicy(). To: Michal Hocko Cc: Andrew Morton , corbet@lwn.net, linux-mm@kvack.org, linux-kernel@vger.kernel.org, linux-api@vger.kernel.org, linux-doc@vger.kernel.org References: <20221111084051.2121029-1-hezhongkun.hzk@bytedance.com> <20221111112732.30e1696bcd0d5b711c188a9a@linux-foundation.org> From: Zhongkun He In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=bytedance-com.20210112.gappssmtp.com header.s=20210112 header.b=riU8FvWG; spf=pass (imf22.hostedemail.com: domain of hezhongkun.hzk@bytedance.com designates 209.85.210.171 as permitted sender) smtp.mailfrom=hezhongkun.hzk@bytedance.com; dmarc=pass (policy=none) header.from=bytedance.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1668438728; a=rsa-sha256; cv=none; b=UjL8rxn5R8KxWYqFMmaPX7D+taJF6WEx+k16UwZSjlIVN22amK1XIT840FFQXVhWa5kcth pEJHW/s+5zrWl98Yk4EQ37fEm89iI0leuUwvT10FcKzqGwvgG/XTUE7pdGFonT3G4VcJfi 0tVvzGBAPdm9DhLk3u9Tz7xdx+YJqP8= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1668438728; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=e/0liD4zfDdUwy8XG2a2Y/qRgR4CN54ZKUBk4lToHno=; b=PYfyVBSlG7lgt/QpVwWYWzeC34JgFhs4HiIsWNhQnwKAmOPeGZAO8OcKnh5w8HvxLPk5wf Udf+5dxJAgopIeF7du/hG/oQSEnJIqDVaBD1lLxu+djhrD1KEhHOHStT83z8HFdKhou3FQ swV02K460pjpybijOwFzqE+eKGxDdOY= X-Stat-Signature: xay934tz1rzf6pnyfwzqckbo3xx4esdf X-Rspamd-Queue-Id: DA97FC0010 X-Rspam-User: Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=bytedance-com.20210112.gappssmtp.com header.s=20210112 header.b=riU8FvWG; spf=pass (imf22.hostedemail.com: domain of hezhongkun.hzk@bytedance.com designates 209.85.210.171 as permitted sender) smtp.mailfrom=hezhongkun.hzk@bytedance.com; dmarc=pass (policy=none) header.from=bytedance.com X-Rspamd-Server: rspam11 X-HE-Tag: 1668438727-729216 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: 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). > 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. "Usage" here means one of the following: 1) querying of the policy, either by the task itself [using the get_mempolicy() API discussed below] or by another task using the /proc//numa_maps interface. 2) examination of the policy to determine the policy mode and associated node or node lists, if any, for page allocation. This is considered a "hot path". Note that for MPOL_BIND, the "usage" extends across the entire allocation process, which may sleep during page reclaimation, because the BIND policy nodemask is used, by reference, to filter ineligible nodes. > E.g. just looking at some > random places like allowed_mems_nr (relying on get_task_policy) is > completely lockless and some paths (like fadvise) do not use any of the > explicit (alloc_lock) or implicit (mmap_lock) locking. That means that > the task_work based approach cannot really work in this case, right? The task_work based approach (mpol_put_async()) allows mempolicy release to be transferred from the pidfd_set_mempolicy() context to the target process context.The old mempolicy droped by pidfd_set_mempolicy() will be freed by task_work(mpol_free_async) whenever the target task exit to user mode. At this point task->mempolicy will not be used, thus avoiding race conditions. pidfd process context: void mpol_put_async() {..... init_task_work(&p->w.cb_head, "mpol_free_async"); if (!task_work_add(task, &p->w.cb_head, TWA_SIGNAL_NO_IPI)) return;} target task: /* there is no lock and mempolicy may about to be freed by * pidfd_set_mempolicy(). */ pol = get_task_policy() page = __alloc_pages(..pol) ..... exit_to_user_mode task_work_run() /* It's safe to free old mempolicy * dropped by pidfd_set_mempolicy() at this time.*/ mpol_free_async() > Playing more tricks will not really help long term. So while your patch > tries to workaround the current state of the art I do not think we > really want that. As stated previously, I would much rather see proper > reference counting instead. I thought we have agreed this would be the > first approach unless the resulting performance is really bad. Have you > concluded that to be the case? I do not see any numbers or notes in the > changelog. I have tried it, but I found that using task_work to release the old policy on the target process can solve the problem, but I'm not sure. My expression may not be very clear, Looking forward to your reply. Thanks.