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 BF2F6C433EF for ; Wed, 9 Mar 2022 01:03:22 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3F7648D0002; Tue, 8 Mar 2022 20:03:22 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 37F968D0001; Tue, 8 Mar 2022 20:03:22 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1F8D98D0002; Tue, 8 Mar 2022 20:03:22 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0232.hostedemail.com [216.40.44.232]) by kanga.kvack.org (Postfix) with ESMTP id 099878D0001 for ; Tue, 8 Mar 2022 20:03:22 -0500 (EST) Received: from smtpin26.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id BA9DB1828D801 for ; Wed, 9 Mar 2022 01:03:21 +0000 (UTC) X-FDA: 79223049402.26.4724DCD Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by imf20.hostedemail.com (Postfix) with ESMTP id 038311C0008 for ; Wed, 9 Mar 2022 01:03:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1646787801; x=1678323801; h=from:to:cc:subject:references:date:in-reply-to: message-id:mime-version; bh=WqYjw0riMyrcnpYGzd3LwwF0KxaWZQvCdtVqi7q+5xA=; b=MYE5c5nSf5SHxt78Fw2WxxrmESeTI60BR1NL0edLLS7WgFndPIjZBOR1 JoMQg3Die9HWVOo90jjGR5UCbwMDChP0O8xHqOcv5lg9xg7b19YLNJuGT 9Fa+G39k3YlrmjEDd03jOQlLM/aGtbHrGjNLiIwzZXzufliyKLAdr/peH v/8239/E+1KWx4TpbVOww7wn3DMWDgnwxTIr0/iuu6kVDxGWDSwRIb+XT JlEoaM178cTpRG6RLSx7QLNMN6nQ93EnvSX6bSEdTkV6oPXJgMuuFR8HM RHd4+JnkNVUFNAF70ibjhmrqYb4xMgySpGjSN/cnZT72aqNvvuch+K11S g==; X-IronPort-AV: E=McAfee;i="6200,9189,10280"; a="255045773" X-IronPort-AV: E=Sophos;i="5.90,165,1643702400"; d="scan'208";a="255045773" Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga103.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Mar 2022 17:02:52 -0800 X-IronPort-AV: E=Sophos;i="5.90,165,1643702400"; d="scan'208";a="513330922" Received: from yhuang6-desk2.sh.intel.com (HELO yhuang6-desk2.ccr.corp.intel.com) ([10.239.13.94]) by orsmga006-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 08 Mar 2022 17:02:47 -0800 From: "Huang, Ying" To: Miaohe Lin Cc: , , , , , , , , , , , , , , , , , , Christoph Lameter , "David Howells" , Linus Torvalds Subject: Re: [PATCH 04/16] mm/migration: reduce the rcu lock duration References: <20220304093409.25829-1-linmiaohe@huawei.com> <20220304093409.25829-5-linmiaohe@huawei.com> <8735ju7as9.fsf@yhuang6-desk2.ccr.corp.intel.com> <2eb3fc34-3c81-394f-3bca-8eb00027afcf@huawei.com> Date: Wed, 09 Mar 2022 09:02:45 +0800 In-Reply-To: <2eb3fc34-3c81-394f-3bca-8eb00027afcf@huawei.com> (Miaohe Lin's message of "Tue, 8 Mar 2022 20:09:15 +0800") Message-ID: <87y21key4q.fsf@yhuang6-desk2.ccr.corp.intel.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=ascii X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: 038311C0008 X-Stat-Signature: 7p4ocyc4qcrpmabd1enzcim6qi516a5z Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=intel.com header.s=Intel header.b=MYE5c5nS; spf=none (imf20.hostedemail.com: domain of ying.huang@intel.com has no SPF policy when checking 192.55.52.115) smtp.mailfrom=ying.huang@intel.com; dmarc=pass (policy=none) header.from=intel.com X-Rspam-User: X-HE-Tag: 1646787800-64789 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: Miaohe Lin writes: > On 2022/3/7 10:32, Huang, Ying wrote: >> Miaohe Lin writes: >> >>> rcu_read_lock is required by grabbing the task refcount but it's not >>> needed for ptrace_may_access. So we could release the rcu lock after >>> task refcount is successfully grabbed to reduce the rcu holding time. >>> >>> Signed-off-by: Miaohe Lin >>> --- >>> mm/migrate.c | 3 +-- >>> 1 file changed, 1 insertion(+), 2 deletions(-) >>> >>> diff --git a/mm/migrate.c b/mm/migrate.c >>> index da5a81052468..26943bd819e8 100644 >>> --- a/mm/migrate.c >>> +++ b/mm/migrate.c >>> @@ -1907,17 +1907,16 @@ static struct mm_struct *find_mm_struct(pid_t pid, nodemask_t *mem_nodes) >>> return ERR_PTR(-ESRCH); >>> } >>> get_task_struct(task); >>> + rcu_read_unlock(); >>> >>> /* >>> * Check if this process has the right to modify the specified >>> * process. Use the regular "ptrace_may_access()" checks. >>> */ >>> if (!ptrace_may_access(task, PTRACE_MODE_READ_REALCREDS)) { >>> - rcu_read_unlock(); >>> mm = ERR_PTR(-EPERM); >>> goto out; >>> } >>> - rcu_read_unlock(); >>> >>> mm = ERR_PTR(security_task_movememory(task)); >>> if (IS_ERR(mm)) >> >> Digged some history via `git blame`, found that the RCU read lock is >> extended in the following commit, >> >> " >> 3268c63eded4612a3d07b56d1e02ce7731e6608e >> Author: Christoph Lameter >> AuthorDate: Wed Mar 21 16:34:06 2012 -0700 >> Commit: Linus Torvalds >> CommitDate: Wed Mar 21 17:54:58 2012 -0700 >> >> mm: fix move/migrate_pages() race on task struct >> >> Migration functions perform the rcu_read_unlock too early. As a result >> the task pointed to may change from under us. This can result in an oops, >> as reported by Dave Hansen in https://lkml.org/lkml/2012/2/23/302. >> >> The following patch extend the period of the rcu_read_lock until after the >> permissions checks are done. We also take a refcount so that the task >> reference is stable when calling security check functions and performing >> cpuset node validation (which takes a mutex). >> >> The refcount is dropped before actual page migration occurs so there is no >> change to the refcounts held during page migration. >> >> Also move the determination of the mm of the task struct to immediately >> before the do_migrate*() calls so that it is clear that we switch from >> handling the task during permission checks to the mm for the actual >> migration. Since the determination is only done once and we then no >> longer use the task_struct we can be sure that we operate on a specific >> address space that will not change from under us. >> " >> >> After that, the permission checking has been changed from __task_cred() >> to ptrace_may_access(). So the situation may change somewhat. Cced > > In ptrace_may_access, __task_cred is access while holding the rcu read lock. > It seems this is ensured by the ptrace_may_access itself. Please read the patch above. Before extending rcu_read_lock protected region, __task_cred() is protected by rcu_read_lock already. The patch above combines 2 regions into 1. Best Regards, Huang, Ying >> some names found in git history to verify. > > Thanks for your carefulness. > >> >> Best Regards, >> Huang, Ying >> . >>