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 416DAC47258 for ; Fri, 26 Jan 2024 00:35:47 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 919F96B0083; Thu, 25 Jan 2024 19:35:46 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 8C9D76B0087; Thu, 25 Jan 2024 19:35:46 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 791326B0088; Thu, 25 Jan 2024 19:35:46 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 6991E6B0083 for ; Thu, 25 Jan 2024 19:35:46 -0500 (EST) Received: from smtpin19.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 0B1411405C4 for ; Fri, 26 Jan 2024 00:35:46 +0000 (UTC) X-FDA: 81719594292.19.1593BFF Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by imf13.hostedemail.com (Postfix) with ESMTP id 3151A20010 for ; Fri, 26 Jan 2024 00:35:43 +0000 (UTC) Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=R5yN7LCd; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf13.hostedemail.com: domain of "SRS0=zAkC=JE=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" designates 139.178.84.217 as permitted sender) smtp.mailfrom="SRS0=zAkC=JE=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706229344; h=from:from:sender:reply-to: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=AGdApbOBn8lcxRFg285Po55+KmETUlx4QttGkVUQWX8=; b=4HXeKux8UElR0evjfdmRtxHsVY7C5N96/VZbHmRh57vALUz7Smp+DQq9YLSTOijuImxra0 dQ+R2DWr+uzmJHOVaM6jFh7nA0gBPKdp21DIN9EHkRRAEvlYKKOrduDSnhnGuPbEuixBgv DPXzLH+3GbMM1G+ckzAvUX3D7iMy6Iw= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=kernel.org header.s=k20201202 header.b=R5yN7LCd; dmarc=pass (policy=none) header.from=kernel.org; spf=pass (imf13.hostedemail.com: domain of "SRS0=zAkC=JE=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" designates 139.178.84.217 as permitted sender) smtp.mailfrom="SRS0=zAkC=JE=paulmck-ThinkPad-P17-Gen-1.home=paulmck@kernel.org" ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706229344; a=rsa-sha256; cv=none; b=g9zY37mIqfY27E36/UZJMpr4of268Rq5LqQyZxO8F6F4/hR+jkEyZsHUUAK9f+gN7Svc0J pwi3rtOX7DpWfxeX0cb8qD7TCIXN4hCV6AFBQuODL8Dnysp14FOV1ir5TasPKWuV25mQ76 h6lcHiS40e3uhRT+kwRntVU3wz6yCgg= Received: from smtp.kernel.org (transwarp.subspace.kernel.org [100.75.92.58]) by dfw.source.kernel.org (Postfix) with ESMTP id 1165A62384; Fri, 26 Jan 2024 00:35:38 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BCDF2C433F1; Fri, 26 Jan 2024 00:35:36 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1706229336; bh=aiwzMlQcwKusi2WD5mp/I9f4iHR1B+oJpqL8BoebmQ8=; h=Date:From:To:Cc:Subject:Reply-To:References:In-Reply-To:From; b=R5yN7LCdE6du4H1Lv+od9LQxc2y/qNm/Xk1SF1yM0M/fo/STgxQ7qNJhirectSr5Q PEp38quC/2Ufabd8xTE4J7bTrIf5oL9E5RRanPI3aoLQ7yqrFCjrZo0wo8QTdyEQaz g7b/QXMaRNPeKvE8hWPqbcztAjpH79i7r4dLcJNeeCZ1YyU7gJWhfoc6ab/bFkzxGr zfzPbm5f7lgsgKmFp+uQ+WITPIRFvmVgb668yAM+jlWQOPxB1MbELzhWAk5i1xVyJO SqehbQAtHdkt7Ib5Cztdcm+ESwaTC7/T4HZCaGTt48o0VE/5W4/M10jLPAbLw8aNaZ VumfjyTPiISPg== Received: by paulmck-ThinkPad-P17-Gen-1.home (Postfix, from userid 1000) id 5F702CE1473; Thu, 25 Jan 2024 16:35:36 -0800 (PST) Date: Thu, 25 Jan 2024 16:35:36 -0800 From: "Paul E. McKenney" To: Suren Baghdasaryan Cc: kernel test robot , Matthew Wilcox , oe-kbuild-all@lists.linux.dev, Linux Memory Management List , Andrew Morton Subject: Re: [linux-next:master 1589/1892] fs/proc/task_mmu.c:143:45: sparse: sparse: incorrect type in argument 1 (different address spaces) Message-ID: Reply-To: paulmck@kernel.org References: <202401251829.0m6Eo4LI-lkp@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Server: rspam09 X-Rspamd-Queue-Id: 3151A20010 X-Stat-Signature: yssm3xgyrprrtmahu9qij5cdxysr8hg6 X-Rspam-User: X-HE-Tag: 1706229343-947656 X-HE-Meta: U2FsdGVkX1+XYdvTSxldff6GDaN6+6wqZI3j+rrGMm4vCCb0PPo7XlPCnBtRAOoIeJmDghPRAAGNj8tvFTZuZOTqHCJGqCFw2B73URrsqIFFxNpLMN81olagrxNlCUqvHxGPUbZYZSnCmQ0OYzDWdPObnYB1H3m0mT/SPIxgfbdYiX0UuoeOc8ygSXHHVPifdcaKZZZ4Q+Rub8M8p4Sd60n9O0HIH/DRfEKDt9YsuiIqgfsib8cJ+gxsn0ZLAzhq/L34KY2dJ9j3Xzk8y7BbRQ/RHK3CLwOgDOEfErqrCWtRF6nz2oTzbs9xzo91hkVEDDDWyt8gwEJd1W9lYOd+ShVbsDOaCcF9+lBqYIilLgZcl8fAUATFF0EHorDC/Fa0yKIgamGUIkYo8x1+wzIbuK0o8CNENofAUt4D6ZUC9hG44mUHfHiAOUjyrvtFmEYj/jFt6g5IEcrik/rU7Nb3yY7NphhgeYI8hx3iIPJ59rp3xmnm6rBqLizNvN3o14r87cxxnaQQ2DQJd0VmtQEJL1hATy/3BYIT8OtP3KTeLuXtTmLbTVS8eMM+6aVBWrCO580lryKzMqrwxBbZWwnNfbUVtNmGwYqxtLQgCQOkwEaYwQbIlVUq0+/PSzh+Bt4/A0L86Tv0QGDetsrJg3iqiCN3zKJ82H7N7ENj73Jdga1MceSR6LV4tb9Mld0imw2nEyM77LBx+UKh4ilhhnyYBYKjcICatJjtq3CrPr5qsI/nCPTmwP69SIFSd0f+iDzTlfTpM5JFfDJCUuXXV7EBfPYGven3I1pAUUEd2i5stC90RrD2q+Y8rsKy6/lYNx4fEAQ17jmbc3Q8vWE08c3hrHk7POtonSu9Ivv/IXAtxzXhNciMlBPIoxnmPxMi8OMomhPTtMOk4Q6eNptOPpe4uOgLgkJ9tnF+jR+FabN+ozR6zVWMM3E/YIopHUvxkSrteeCZU+GM3djxRPRaq/i IrPleR0g DHUzdobEXyvwM+boriBZAShXSnm+9xrrxVS3fysqvEVCK9gb4Dka0c0xQVttsqDm1KslhdJjDgNTCYPsXM942IOrh+CylMqW1njLJkfDWfbpkcSoC5dEouuwgzGfzZLfso5Z6e0NDpanf+xxF32zgi+qpHTpDSdtgP8Ni0WNxhOvT1eXXvopWcdKLBuXIq8O1HSrXQXPPekk/nXPkSEuANkZE1va8GCOStJZk2xIWG8DiTyl5vCRd+5IdeCR2FmHazevAXk35AM5wGmHfBriQu8PBgI3PzdLEmpaLNVu5HO6pgHPoMGxjBmx5n9PrTGunO6a53h4JwEwazv91U/ZuhOnjPFX+BJJaGcu4aUBXVG+yRNfYtXzsTTkcyKpt69Za2LRC7LjJT2FlCXTVSfbH5dEpGtI+O+30RJei0CiEt7+vNBIwNgnwrUe15gmDaGb4HrEHZlfUEe0egg9kDrFLKdKqOCutcB/ml/wgLduGhFMLVn05fhqb1toKgk+VYiK4vcfSueBIdK9nsTqXEHfUqq09k6W/Fd5MeMtdlQciheQ8q0VvInk5hKThO+50qbDvaUXHbB4bpvSjoc7AGVKDAAIJ1um9Z2iaC0asgw4OWFzj6C866GfD+ySFCESAc6E6ZlPGnP6eaT9eKMFuBtj+vVFV+ZnhrOz0tejlITyLWrBWtlalDOZOsygmKLq/4WaJdZDh4ul/uLI5k2Bj/FvJRb1lerUx7cAn0n24Wp0R7oYe0J9LFwnW+H24MQ== 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: List-Subscribe: List-Unsubscribe: On Thu, Jan 25, 2024 at 03:17:17PM -0800, Suren Baghdasaryan wrote: > On Thu, Jan 25, 2024 at 2:44 PM Paul E. McKenney wrote: > > > > On Thu, Jan 25, 2024 at 01:27:34PM -0800, Suren Baghdasaryan wrote: > > > On Thu, Jan 25, 2024 at 2:23 AM kernel test robot wrote: > > > > > > > > tree: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master > > > > head: 01af33cc9894b4489fb68fa35c40e9fe85df63dc > > > > commit: 0c30c4cd953025979b7689e49844837f762303ec [1589/1892] mm/maps: read proc/pid/maps under RCU > > > > config: x86_64-randconfig-121-20240125 (https://download.01.org/0day-ci/archive/20240125/202401251829.0m6Eo4LI-lkp@intel.com/config) > > > > compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18) > > > > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240125/202401251829.0m6Eo4LI-lkp@intel.com/reproduce) > > > > > > > > If you fix the issue in a separate patch/commit (i.e. not just a new version of > > > > the same patch/commit), kindly add following tags > > > > | Reported-by: kernel test robot > > > > | Closes: https://lore.kernel.org/oe-kbuild-all/202401251829.0m6Eo4LI-lkp@intel.com/ > > > > > > > > sparse warnings: (new ones prefixed by >>) > > > > >> fs/proc/task_mmu.c:143:45: sparse: sparse: incorrect type in argument 1 (different address spaces) @@ expected struct file [noderef] __rcu **f @@ got struct file ** @@ > > > > > > Uh, this is a problem. > > > I missed that get_file_rcu() is used only with mm->exe_file and > > > vma->vm_file is not really RCU-safe. It's freed via a call to fput() > > > which schedules its freeing using schedule_delayed_work(..., 1) but I > > > don't think that constitutes RCU grace period. Paul, Matthew, could > > > you please confirm? In the meantime I'm going to ask Andrew to remove > > > my patchset from mm-unstable to be safe. > > > > Sadly, no, schedule_delayed_work() does not imply an RCU grace period. > > > > There is a queue_rcu_work() that schedules work after a grace period, > > which could be combined with a timer to get the delay. > > > > Another approach would be to use get_state_synchronize_rcu() before > > the schedule_delayed_work() in fput(), then do cond_synchronize_rcu() > > in delayed_fput(). This would require adding an unsigned long to > > struct file to keep track of which grace period a given struct file > > needed to wait for. > > > > Perhaps something like this: > > > > ------------------------------------------------------------------------ > > > > void fput(struct file *file) > > { > > if (atomic_long_dec_and_test(&file->f_count)) { > > struct task_struct *task = current; > > > > if (likely(!in_interrupt() && !(task->flags & PF_KTHREAD))) { > > init_task_work(&file->f_rcuhead, ____fput); > > if (!task_work_add(task, &file->f_rcuhead, TWA_RESUME)) > > return; > > /* > > * After this task has run exit_task_work(), > > * task_work_add() will fail. Fall through to delayed > > * fput to avoid leaking *file. > > */ > > } > > > > file->f_rcu_seq = get_state_synchronize_rcu(); > > if (llist_add(&file->f_llist, &delayed_fput_list)) > > schedule_delayed_work(&delayed_fput_work, 1); > > } > > } > > > > ------------------------------------------------------------------------ > > > > And this: > > > > ------------------------------------------------------------------------ > > > > static void delayed_fput(struct work_struct *unused) > > { > > struct llist_node *node = llist_del_all(&delayed_fput_list); > > struct file *f, *t; > > > > llist_for_each_entry_safe(f, t, node, f_llist) { > > cond_synchronize_rcu(f->f_rcu_seq); > > __fput(f); > > } > > } > > > > ------------------------------------------------------------------------ > > > > Note that if you called fput() on a long sequence of struct file > > structures, the cond_synchronize_rcu() would be a near-noop almost all the > > time, actually blocking at most about every once per every few jiffies. > > After all, once a grace period has been waited for, it covers all of > > the struct file structures that were passed to fput() during a given > > RCU grace period. > > > > Still, it would add the occasional delay. And it would increase the > > size of struct file, though there are workarounds for that, if size > > is an issue. > > > > Thoughts? > > Thanks for the suggestion, Paul. I'm worried about this occasional > delay but otherwise this seems like a nice and simple approach. One potential saving grace is that the more heavily loaded the mechanism, the smaller a fraction of the cond_synchronize_rcu() calls will do a delay. > Do you > guys think that making *all* files RCU-safe with this approach is > warranted? For my particular case I need only vma->vm_file to be > RCU-safe but maybe there are other cases which would benefit from > this? To this, I can only give an unqualified "I don't know". :-( But if there is some condition that can be sampled on a per-file-structure basis, you could use that to invoke cond_synchronize_rcu() only when needed. Or send only those file structures that need the extra delay through queue_rcu_work(), perhaps by accumulating a list of them. Thanx, Paul > > > > fs/proc/task_mmu.c:143:45: sparse: expected struct file [noderef] __rcu **f > > > > fs/proc/task_mmu.c:143:45: sparse: got struct file ** > > > > fs/proc/task_mmu.c: note: in included file (through include/linux/rbtree.h, include/linux/mm_types.h, include/linux/mmzone.h, ...): > > > > include/linux/rcupdate.h:781:9: sparse: sparse: context imbalance in 'get_vma_snapshot' - unexpected unlock > > > > fs/proc/task_mmu.c:264:22: sparse: sparse: context imbalance in 'm_start' - different lock contexts for basic block > > > > include/linux/rcupdate.h:781:9: sparse: sparse: context imbalance in 'm_stop' - unexpected unlock > > > > include/linux/rcupdate.h:781:9: sparse: sparse: context imbalance in 'smaps_pte_range' - unexpected unlock > > > > include/linux/rcupdate.h:781:9: sparse: sparse: context imbalance in 'clear_refs_pte_range' - unexpected unlock > > > > include/linux/rcupdate.h:781:9: sparse: sparse: context imbalance in 'pagemap_pmd_range' - unexpected unlock > > > > include/linux/rcupdate.h:781:9: sparse: sparse: context imbalance in 'pagemap_scan_pmd_entry' - unexpected unlock > > > > fs/proc/task_mmu.c: note: in included file (through arch/x86/include/asm/uaccess.h, include/linux/uaccess.h, include/linux/sched/task.h, ...): > > > > arch/x86/include/asm/uaccess_64.h:88:24: sparse: sparse: cast removes address space '__user' of expression > > > > arch/x86/include/asm/uaccess_64.h:88:24: sparse: sparse: cast removes address space '__user' of expression > > > > > > > > vim +143 fs/proc/task_mmu.c > > > > > > > > 132 > > > > 133 /* > > > > 134 * Take VMA snapshot and pin vm_file and anon_name as they are used by > > > > 135 * show_map_vma. > > > > 136 */ > > > > 137 static int get_vma_snapshot(struct proc_maps_private *priv, struct vm_area_struct *vma) > > > > 138 { > > > > 139 struct vm_area_struct *copy = &priv->vma_copy; > > > > 140 int ret = -EAGAIN; > > > > 141 > > > > 142 memcpy(copy, vma, sizeof(*vma)); > > > > > 143 if (copy->vm_file && !get_file_rcu(©->vm_file)) > > > > 144 goto out; > > > > 145 > > > > 146 if (!anon_vma_name_get_if_valid(copy)) > > > > 147 goto put_file; > > > > 148 > > > > 149 if (priv->mm_wr_seq == mmap_write_seq_read(priv->mm)) > > > > 150 return 0; > > > > 151 > > > > 152 /* Address space got modified, vma might be stale. Wait and retry. */ > > > > 153 rcu_read_unlock(); > > > > 154 ret = mmap_read_lock_killable(priv->mm); > > > > 155 mmap_write_seq_record(priv->mm, &priv->mm_wr_seq); > > > > 156 mmap_read_unlock(priv->mm); > > > > 157 rcu_read_lock(); > > > > 158 > > > > 159 if (!ret) > > > > 160 ret = -EAGAIN; /* no other errors, ok to retry */ > > > > 161 > > > > 162 anon_vma_name_put_if_valid(copy); > > > > 163 put_file: > > > > 164 if (copy->vm_file) > > > > 165 fput(copy->vm_file); > > > > 166 out: > > > > 167 return ret; > > > > 168 } > > > > 169 > > > > > > > > -- > > > > 0-DAY CI Kernel Test Service > > > > https://github.com/intel/lkp-tests/wiki