Linux filesystem development
 help / color / mirror / Atom feed
From: David Laight <david.laight.linux@gmail.com>
To: Jori Koolstra <jkoolstra@xs4all.nl>
Cc: Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
	Al Viro <viro@zeniv.linux.org.uk>, NeilBrown <neil@brown.name>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 02/12] fs/namei.c: move create error && negative dentry case in lookup_open up
Date: Mon, 15 Jun 2026 10:37:07 +0100	[thread overview]
Message-ID: <20260615103707.1a55bab6@pumpkin> (raw)
In-Reply-To: <20260614164438.2980769-3-jkoolstra@xs4all.nl>

On Sun, 14 Jun 2026 18:44:28 +0200
Jori Koolstra <jkoolstra@xs4all.nl> wrote:

> O_CREAT is stripped when create_error is set in lookup_open(), so when
> lookup does not return an inode, the case
> 
> 	if (!dentry->d_inode && (open_flag & O_CREAT))
> 
> is always skipped. We can get rid of this cognitive step by handling the
> error case first.

You've just added an extra test into a normal/fast path.
OTOH the test isn't ideal the compiler needs to reverse the order of
the arguments to the && in spite of the unlikely().

Perhaps:
	if (dentry->d_inode)
		return dentry;

	if (open_flag & O_CREAT) {
		...
	} else {
		if (unlikely(create_error)) {
			error = create_error();
			goto out_dput;
		}
	}
	return dentry;

-- David

> 
> Signed-off-by: Jori Koolstra <jkoolstra@xs4all.nl>
> ---
>  fs/namei.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 64b91ed9efb7..f169d1123b17 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -4499,6 +4499,11 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  		}
>  	}
>  
> +	if (unlikely(create_error) && !dentry->d_inode) {
> +		error = create_error;
> +		goto out_dput;
> +	}
> +
>  	/* Negative dentry, just create the file */
>  	if (!dentry->d_inode && (open_flag & O_CREAT)) {
>  		/* but break the directory lease first! */
> @@ -4518,10 +4523,7 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
>  		if (error)
>  			goto out_dput;
>  	}
> -	if (unlikely(create_error) && !dentry->d_inode) {
> -		error = create_error;
> -		goto out_dput;
> -	}
> +
>  	return dentry;
>  
>  out_dput:


  reply	other threads:[~2026-06-15  9:37 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-14 16:44 [PATCH 00/12] vfs: add O_CREAT|O_DIRECTORY to open*(2) Jori Koolstra
2026-06-14 16:44 ` [PATCH 01/12] fs/namei.c: use trailing_slashes() Jori Koolstra
2026-06-15  9:26   ` David Laight
2026-06-14 16:44 ` [PATCH 02/12] fs/namei.c: move create error && negative dentry case in lookup_open up Jori Koolstra
2026-06-15  9:37   ` David Laight [this message]
2026-06-14 16:44 ` [PATCH 03/12] vfs: prepare vfs_creat|mkdir_no_perm for reuse in lookup_open() Jori Koolstra
2026-06-14 16:44 ` [PATCH 04/12] fs/namei.c: lookup_open(): move audit_inode_child() up Jori Koolstra
2026-06-15 21:43   ` Jori Koolstra
2026-06-14 16:44 ` [PATCH 05/12] vfs: lookup_open(): move setting FMODE_CREATED up when calling create() Jori Koolstra
2026-06-14 16:44 ` [PATCH 06/12] vfs: lookup_open(): move i_op->create check to before try_break_deleg() Jori Koolstra
2026-06-14 16:44 ` [PATCH 07/12] vfs: lookup_open(): use vfs_create_no_perm() Jori Koolstra
2026-06-14 16:44 ` [PATCH 08/12] vfs: add O_CREAT|O_DIRECTORY to open*(2) Jori Koolstra
2026-06-14 16:44 ` [PATCH 09/12] vfs: move O_IS_MKDIR check out atomic_open() to individual filesystems Jori Koolstra
2026-06-14 16:44 ` [PATCH 10/12] vfs: refuse O_CREAT for directories through a dangling symlink Jori Koolstra
2026-06-14 16:44 ` [PATCH 11/12] vfs: short-circuit MAY_WRITE access for O_DIRECTORY opens Jori Koolstra
2026-06-14 17:01   ` Jori Koolstra
2026-06-15 12:56   ` Jori Koolstra
2026-06-14 16:44 ` [PATCH 12/12] selftest: add tests for open*(O_CREAT|O_DIRECTORY) Jori Koolstra

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=20260615103707.1a55bab6@pumpkin \
    --to=david.laight.linux@gmail.com \
    --cc=brauner@kernel.org \
    --cc=jack@suse.cz \
    --cc=jkoolstra@xs4all.nl \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=neil@brown.name \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox