All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Brauner <brauner@kernel.org>
To: Mateusz Guzik <mjguzik@gmail.com>
Cc: viro@zeniv.linux.org.uk, jack@suse.cz,
	linux-kernel@vger.kernel.org,  linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH v3 5/7] fs: handle hypothetical filesystems which use I_DONTCACHE and drop the lock in ->drop_inode
Date: Tue, 31 Mar 2026 13:23:35 +0200	[thread overview]
Message-ID: <20260331-ellbogen-wegrand-e98370e446e3@brauner> (raw)
In-Reply-To: <20260329172002.3557801-6-mjguzik@gmail.com>

On Sun, Mar 29, 2026 at 07:20:00PM +0200, Mateusz Guzik wrote:
> f2fs and ntfs play games where they transitiong the refcount 0->1 and release
> the inode spinlock, allowing other threads to grab a ref of their own.
> They also return 0 in that case, making this problem harmless.
> 
> Should they start using the I_DONTCACHE machinery down the road while
> retaining the above, iput_final() will get a race where it can proceed
> to teardown an inode with references.
> 
> Future-proof it.
> 
> Developing better ->drop_inode and sanitizing all users is left as en
> exercise for the reader.
> 
> Signed-off-by: Mateusz Guzik <mjguzik@gmail.com>
> ---
>  fs/inode.c | 27 ++++++++++++++++++---------
>  1 file changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/fs/inode.c b/fs/inode.c
> index 0050eca6d83f..a417abc64822 100644
> --- a/fs/inode.c
> +++ b/fs/inode.c
> @@ -1935,20 +1935,29 @@ static void iput_final(struct inode *inode)
>  	else
>  		drop = inode_generic_drop(inode);
>  
> -	if (!drop &&
> -	    !(inode_state_read(inode) & I_DONTCACHE) &&
> -	    (sb->s_flags & SB_ACTIVE)) {
> +	/*
> +	 * XXXCRAP: there are ->drop_inode hooks playing nasty games releasing the

Let's make that a "Note:" and remove that "XXXCRAP", please. The "nasty
games" is plenty meta commentary.

> +	 * spinlock and temporarily grabbing refs. This opens a possibility someone
> +	 * else will sneak in and grab a ref while it happens.
> +	 *
> +	 * If such a hook returns 0 (== don't drop) this happens to be harmless as long
> +	 * as the inode is not marked with I_DONTCACHE. Otherwise we are proceeding with
> +	 * teardown despite references being present.
> +	 *
> +	 * Damage-control the problem by including the count in the decision. However,
> +	 * assert no refs showed up if the hook decided to drop the inode.
> +	 */
> +	if (drop)
> +		VFS_BUG_ON_INODE(icount_read(inode) != 0, inode);
> +
> +	if (icount_read(inode) > 0 ||
> +	    (!drop && !(inode_state_read(inode) & I_DONTCACHE) &&
> +	    (sb->s_flags & SB_ACTIVE))) {
>  		__inode_lru_list_add(inode, true);
>  		spin_unlock(&inode->i_lock);
>  		return;
>  	}
>  
> -	/*
> -	 * Re-check ->i_count in case the ->drop_inode() hooks played games.
> -	 * Note we only execute this if the verdict was to drop the inode.
> -	 */
> -	VFS_BUG_ON_INODE(icount_read(inode) != 0, inode);
> -
>  	if (drop) {
>  		inode_state_set(inode, I_FREEING);
>  	} else {
> -- 
> 2.48.1
> 

  reply	other threads:[~2026-03-31 11:23 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-29 17:19 [PATCH v3 0/7] assorted ->i_count changes + extension of lockless handling Mateusz Guzik
2026-03-29 17:19 ` [PATCH v3 1/7] fs: add icount_read_once() Mateusz Guzik
2026-03-29 17:19 ` [PATCH v3 2/7] Use icount_read() and icount_read_once() as appropriate Mateusz Guzik
2026-03-29 17:19 ` [PATCH v3 3/7] fs: enforce locking in icount_read(), add some commentary Mateusz Guzik
2026-03-29 17:19 ` [PATCH v3 4/7] fs: relocate and tidy up ihold() Mateusz Guzik
2026-03-29 17:20 ` [PATCH v3 5/7] fs: handle hypothetical filesystems which use I_DONTCACHE and drop the lock in ->drop_inode Mateusz Guzik
2026-03-31 11:23   ` Christian Brauner [this message]
2026-03-31 11:37     ` Mateusz Guzik
2026-03-29 17:20 ` [PATCH v3 6/7] fs: locklessly bump refs in igrab as long as it does not transition 0->1 Mateusz Guzik
2026-03-29 17:20 ` [PATCH v3 7/7] fs: locklessly bump refs in the inode hash when possible Mateusz Guzik
2026-03-30  7:38 ` [syzbot ci] Re: assorted ->i_count changes + extension of lockless handling syzbot ci
2026-03-31 11:31 ` [PATCH v3 0/7] " Christian Brauner
2026-03-31 11:36   ` Mateusz Guzik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260331-ellbogen-wegrand-e98370e446e3@brauner \
    --to=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjguzik@gmail.com \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.