From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4D7773D3319; Fri, 20 Mar 2026 18:57:40 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774033060; cv=none; b=nS16dggtj8mNCctU3Xj9FqIPy45t97Xa3P8NC6/n2M/RvhjsppqF8wkjhG8YFZ/dTz3eUr+7XNDFvOBto0XrjxPBegcs/ueJEdYRn8XNWp04PoKxCWuF6BkAp+Hv4AMl60fm/+A6eYGfG2fVyyNY8qcrsiYcc42p05qNjsp3sl8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774033060; c=relaxed/simple; bh=a8ys166E/M77dCSrucnKwE5heGp6aoVZbHs9H3pgrKI=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=WrrQ1lesZE8m4/kji9IOPsuGzZTIInsW014Ix3vci3fWAQvRNe9jT3/2qlzm11FWmRk8FK9oYLlmQ1k/BESHJ2+Vfzh7cdTod9yyb0el2XhbFFT9IMs0ZxtdezL7FaqajHZgKrPwENW+ulEyKDV+yPTq6g3JoqZwM5+39Boztak= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=d1UmBx/K; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="d1UmBx/K" Received: by smtp.kernel.org (Postfix) with ESMTPSA id B1C94C4CEF7; Fri, 20 Mar 2026 18:57:31 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1774033060; bh=a8ys166E/M77dCSrucnKwE5heGp6aoVZbHs9H3pgrKI=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=d1UmBx/KAU+ZajRc0Utu3iGMoA6qXkfpo2A3GmQu8VB00T2ciUsv0bUxBPTN6cpTm m3xxfTtoFLkqoxFN2UkxrqdyEtIRum9BSaNPu6j+994kGaQQ67cuDJi2oDwvWgDmKh 6PWVHSUzCXkNdeUl6lKfFgj/jcJTsJl9L7GIzzLjsbklBRc7rOGYW4Fn923e39Uvvr ttgPeY3wAum2TPoqAT48arege0caAn/E8yJkUN7Up2QA4ZleyQSJbDwORXj7psDga2 /GC2DRXLDgEDvmGPPLRO944+KOeHvwAeZxrk9FTglWfQhRpZGclw4Gn1hTFIShfbwA AZE7RKjyLC7dQ== Message-ID: <608ba54c-f19e-4e27-8142-0870f91d6514@kernel.org> Date: Fri, 20 Mar 2026 19:57:29 +0100 Precedence: bulk X-Mailing-List: linux-doc@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v3 05/16] fs: afs: correctly drop reference count on mapping failure Content-Language: en-US To: "Lorenzo Stoakes (Oracle)" , Andrew Morton Cc: Jonathan Corbet , Clemens Ladisch , Arnd Bergmann , Greg Kroah-Hartman , "K . Y . Srinivasan" , Haiyang Zhang , Wei Liu , Dexuan Cui , Long Li , Alexander Shishkin , Maxime Coquelin , Alexandre Torgue , Miquel Raynal , Richard Weinberger , Vignesh Raghavendra , Bodo Stroesser , "Martin K . Petersen" , David Howells , Marc Dionne , Alexander Viro , Christian Brauner , Jan Kara , David Hildenbrand , "Liam R . Howlett" , Mike Rapoport , Suren Baghdasaryan , Michal Hocko , Jann Horn , Pedro Falcato , linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-hyperv@vger.kernel.org, linux-mtd@lists.infradead.org, linux-staging@lists.linux.dev, linux-scsi@vger.kernel.org, target-devel@vger.kernel.org, linux-afs@lists.infradead.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, Ryan Roberts References: <018cd0d8b2dae44de6d3952527e754e52ef02da8.1773944114.git.ljs@kernel.org> From: "Vlastimil Babka (SUSE)" In-Reply-To: <018cd0d8b2dae44de6d3952527e754e52ef02da8.1773944114.git.ljs@kernel.org> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 3/19/26 19:23, Lorenzo Stoakes (Oracle) wrote: > Commit 9d5403b1036c ("fs: convert most other generic_file_*mmap() users to > .mmap_prepare()") updated AFS to use the mmap_prepare callback in favour > of the deprecated mmap callback. > > However, it did not account for the fact that mmap_prepare is called > pre-merge, and may then be merged, nor that mmap_prepare can fail to map > due to an out of memory error. So that means a file can become pinned forever? OOM is probably only a problem with fault injection in practice, but the merge case can happen. And 9d5403b1036c is pre-6.18 LTS. Are we going to need Fixes: and Cc: stable then? > Both of those are cases in which we should not be incrementing a reference > count. > > With the newly added vm_ops->mapped callback available, we can simply > defer this operation to that callback which is only invoked once the > mapping is successfully in place (but not yet visible to userspace as the > mmap and VMA write locks are held). > > Therefore add afs_mapped() to implement this callback for AFS, and remove > the code doing so in afs_mmap_prepare(). > > Also update afs_vm_open(), afs_vm_close() and afs_vm_map_pages() to be > consistent in how the vnode is accessed. > > Signed-off-by: Lorenzo Stoakes (Oracle) > --- > fs/afs/file.c | 36 ++++++++++++++++++++++++++---------- > 1 file changed, 26 insertions(+), 10 deletions(-) > > diff --git a/fs/afs/file.c b/fs/afs/file.c > index f609366fd2ac..85696ac984cc 100644 > --- a/fs/afs/file.c > +++ b/fs/afs/file.c > @@ -28,6 +28,8 @@ static ssize_t afs_file_splice_read(struct file *in, loff_t *ppos, > static void afs_vm_open(struct vm_area_struct *area); > static void afs_vm_close(struct vm_area_struct *area); > static vm_fault_t afs_vm_map_pages(struct vm_fault *vmf, pgoff_t start_pgoff, pgoff_t end_pgoff); > +static int afs_mapped(unsigned long start, unsigned long end, pgoff_t pgoff, > + const struct file *file, void **vm_private_data); > > const struct file_operations afs_file_operations = { > .open = afs_open, > @@ -61,6 +63,7 @@ const struct address_space_operations afs_file_aops = { > }; > > static const struct vm_operations_struct afs_vm_ops = { > + .mapped = afs_mapped, > .open = afs_vm_open, > .close = afs_vm_close, > .fault = filemap_fault, > @@ -494,32 +497,45 @@ static void afs_drop_open_mmap(struct afs_vnode *vnode) > */ > static int afs_file_mmap_prepare(struct vm_area_desc *desc) > { > - struct afs_vnode *vnode = AFS_FS_I(file_inode(desc->file)); > int ret; > > - afs_add_open_mmap(vnode); > - > ret = generic_file_mmap_prepare(desc); > - if (ret == 0) > - desc->vm_ops = &afs_vm_ops; > - else > - afs_drop_open_mmap(vnode); > + if (ret) > + return ret; > + > + desc->vm_ops = &afs_vm_ops; > return ret; > } > > +static int afs_mapped(unsigned long start, unsigned long end, pgoff_t pgoff, > + const struct file *file, void **vm_private_data) > +{ > + struct afs_vnode *vnode = AFS_FS_I(file_inode(file)); > + > + afs_add_open_mmap(vnode); > + return 0; > +} > + > static void afs_vm_open(struct vm_area_struct *vma) > { > - afs_add_open_mmap(AFS_FS_I(file_inode(vma->vm_file))); > + struct file *file = vma->vm_file; > + struct afs_vnode *vnode = AFS_FS_I(file_inode(file)); > + > + afs_add_open_mmap(vnode); > } > > static void afs_vm_close(struct vm_area_struct *vma) > { > - afs_drop_open_mmap(AFS_FS_I(file_inode(vma->vm_file))); > + struct file *file = vma->vm_file; > + struct afs_vnode *vnode = AFS_FS_I(file_inode(file)); > + > + afs_drop_open_mmap(vnode); > } > > static vm_fault_t afs_vm_map_pages(struct vm_fault *vmf, pgoff_t start_pgoff, pgoff_t end_pgoff) > { > - struct afs_vnode *vnode = AFS_FS_I(file_inode(vmf->vma->vm_file)); > + struct file *file = vmf->vma->vm_file; > + struct afs_vnode *vnode = AFS_FS_I(file_inode(file)); > > if (afs_check_validity(vnode)) > return filemap_map_pages(vmf, start_pgoff, end_pgoff);