All of lore.kernel.org
 help / color / mirror / Atom feed
From: Al Viro <viro@zeniv.linux.org.uk>
To: Gabriel Krisman Bertazi <gabriel@krisman.be>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
	Christian Brauner <brauner@kernel.org>,
	tytso@mit.edu, linux-f2fs-devel@lists.sourceforge.net,
	ebiggers@kernel.org, linux-fsdevel@vger.kernel.org,
	jaegeuk@kernel.org, linux-ext4@vger.kernel.org
Subject: Re: [f2fs-dev] [PATCH v6 0/9] Support negative dentries on case-insensitive ext4 and f2fs
Date: Thu, 23 Nov 2023 19:53:27 +0000	[thread overview]
Message-ID: <20231123195327.GP38156@ZenIV> (raw)
In-Reply-To: <87bkbki91c.fsf@>

On Thu, Nov 23, 2023 at 02:06:39PM -0500, Gabriel Krisman Bertazi wrote:

> > A paragraph above you've said that it's not constant over the entire
> > filesystem.
> 
> The same ->d_op is used by every dentry in the filesystem if the superblock
> has the casefold bit enabled, regardless of whether a specific inode is
> casefolded or not. See generic_set_encrypted_ci_d_ops in my tree. It is
> called unconditionally by ext4_lookup and only checks the superblock:
> 
> void generic_set_encrypted_ci_d_ops(struct dentry *dentry)
> {
>         if (dentry->d_sb->s_encoding) {
> 		d_set_d_op(dentry, &generic_encrypted_ci_dentry_ops);
> 		return;
> 	}
>         ...
> 
> What I meant was that this used to be set once at sb->s_d_op, and
> propagated during dentry allocation.  Therefore, the propagation to the
> alias would happen inside __d_alloc.  Once we enabled fscrypt and
> casefold to work together, sb->s_d_op is NULL

Why?  That's what I don't understand - if you really want it for
all dentries on that filesystem, that's what ->s_d_op is for.
If it is not, you have that problem, no matter which way you flip ->d_op
value.

> and we always set the same
> handler for every dentry during lookup.

Not every dentry goes through lookup - see upthread for details.

> > Look, it's really simple - any setup work of that sort done in ->lookup()
> > is either misplaced, or should be somehow transferred over to the alias
> > if one gets picked.
> >
> > As for d_obtain_alias()... AFAICS, it's far more limited in what information
> > it could access.  It knows the inode, but it has no idea about the parent
> > to be.
> 
> Since it has the inode, d_obtain_alias has the superblock.  I think that's all
> we need for generic_set_encrypted_ci_d_ops.

Huh?  If it really depends only upon the superblock, just set it in ->s_d_op
when you set the superblock up.

Again, whatever setup you do for dentry in ->lookup(), you either
	* have a filesystem that never picks an existing directory alias
(e.g. doesn't allow open-by-fhandle or has a very unusual implementation
of related methods, like e.g. shmem), or
	* have that setup misplaced, in part that applies to all dentries out
there (->s_d_op for universal ->d_op value, ->d_init() for uniform allocation
of objects hanging from ->d_fsdata and other things like that), or
	* need to figure out how to transfer the result to alias (manually
after d_splice_alias(), if races do not matter or using a new method explicitly
for that), or
	* lose that state for aliases.

WARNING: multiple messages have this Message-ID (diff)
From: Al Viro <viro@zeniv.linux.org.uk>
To: Gabriel Krisman Bertazi <gabriel@krisman.be>
Cc: Christian Brauner <brauner@kernel.org>,
	tytso@mit.edu, linux-f2fs-devel@lists.sourceforge.net,
	ebiggers@kernel.org, linux-fsdevel@vger.kernel.org,
	jaegeuk@kernel.org, linux-ext4@vger.kernel.org,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: Re: [f2fs-dev] [PATCH v6 0/9] Support negative dentries on case-insensitive ext4 and f2fs
Date: Thu, 23 Nov 2023 19:53:27 +0000	[thread overview]
Message-ID: <20231123195327.GP38156@ZenIV> (raw)
In-Reply-To: <87bkbki91c.fsf@>

On Thu, Nov 23, 2023 at 02:06:39PM -0500, Gabriel Krisman Bertazi wrote:

> > A paragraph above you've said that it's not constant over the entire
> > filesystem.
> 
> The same ->d_op is used by every dentry in the filesystem if the superblock
> has the casefold bit enabled, regardless of whether a specific inode is
> casefolded or not. See generic_set_encrypted_ci_d_ops in my tree. It is
> called unconditionally by ext4_lookup and only checks the superblock:
> 
> void generic_set_encrypted_ci_d_ops(struct dentry *dentry)
> {
>         if (dentry->d_sb->s_encoding) {
> 		d_set_d_op(dentry, &generic_encrypted_ci_dentry_ops);
> 		return;
> 	}
>         ...
> 
> What I meant was that this used to be set once at sb->s_d_op, and
> propagated during dentry allocation.  Therefore, the propagation to the
> alias would happen inside __d_alloc.  Once we enabled fscrypt and
> casefold to work together, sb->s_d_op is NULL

Why?  That's what I don't understand - if you really want it for
all dentries on that filesystem, that's what ->s_d_op is for.
If it is not, you have that problem, no matter which way you flip ->d_op
value.

> and we always set the same
> handler for every dentry during lookup.

Not every dentry goes through lookup - see upthread for details.

> > Look, it's really simple - any setup work of that sort done in ->lookup()
> > is either misplaced, or should be somehow transferred over to the alias
> > if one gets picked.
> >
> > As for d_obtain_alias()... AFAICS, it's far more limited in what information
> > it could access.  It knows the inode, but it has no idea about the parent
> > to be.
> 
> Since it has the inode, d_obtain_alias has the superblock.  I think that's all
> we need for generic_set_encrypted_ci_d_ops.

Huh?  If it really depends only upon the superblock, just set it in ->s_d_op
when you set the superblock up.

Again, whatever setup you do for dentry in ->lookup(), you either
	* have a filesystem that never picks an existing directory alias
(e.g. doesn't allow open-by-fhandle or has a very unusual implementation
of related methods, like e.g. shmem), or
	* have that setup misplaced, in part that applies to all dentries out
there (->s_d_op for universal ->d_op value, ->d_init() for uniform allocation
of objects hanging from ->d_fsdata and other things like that), or
	* need to figure out how to transfer the result to alias (manually
after d_splice_alias(), if races do not matter or using a new method explicitly
for that), or
	* lose that state for aliases.


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

  reply	other threads:[~2023-11-23 19:53 UTC|newest]

Thread overview: 130+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-16  5:07 [PATCH v6 0/9] Support negative dentries on case-insensitive ext4 and f2fs Gabriel Krisman Bertazi
2023-08-16  5:07 ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-08-16  5:07 ` [PATCH v6 1/9] ecryptfs: Reject casefold directory inodes Gabriel Krisman Bertazi
2023-08-16  5:07   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-08-16  5:07 ` [PATCH v6 2/9] 9p: Split ->weak_revalidate from ->revalidate Gabriel Krisman Bertazi
2023-08-16  5:07   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-08-16  5:07 ` [PATCH v6 3/9] fs: Expose name under lookup to d_revalidate hooks Gabriel Krisman Bertazi
2023-08-16  5:07   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-11-22 20:59   ` Al Viro
2023-11-22 20:59     ` [f2fs-dev] " Al Viro
2023-08-16  5:07 ` [PATCH v6 4/9] fs: Add DCACHE_CASEFOLDED_NAME flag Gabriel Krisman Bertazi
2023-08-16  5:07   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-11-22 20:32   ` Al Viro
2023-11-22 20:32     ` [f2fs-dev] " Al Viro
2023-08-16  5:07 ` [PATCH v6 5/9] libfs: Validate negative dentries in case-insensitive directories Gabriel Krisman Bertazi
2023-08-16  5:07   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-11-22 20:20   ` Al Viro
2023-11-22 20:20     ` [f2fs-dev] " Al Viro
2023-08-16  5:08 ` [PATCH v6 6/9] libfs: Chain encryption checks after case-insensitive revalidation Gabriel Krisman Bertazi
2023-08-16  5:08   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-08-16  5:08 ` [PATCH v6 7/9] libfs: Merge encrypted_ci_dentry_ops and ci_dentry_ops Gabriel Krisman Bertazi
2023-08-16  5:08   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-08-16  5:08 ` [PATCH v6 8/9] ext4: Enable negative dentries on case-insensitive lookup Gabriel Krisman Bertazi
2023-08-16  5:08   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-08-16  5:08 ` [PATCH v6 9/9] f2fs: " Gabriel Krisman Bertazi
2023-08-16  5:08   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-08-17 17:06 ` [PATCH v6 0/9] Support negative dentries on case-insensitive ext4 and f2fs Eric Biggers
2023-08-17 17:06   ` [f2fs-dev] " Eric Biggers
2023-08-21 15:52   ` Christian Brauner
2023-08-21 15:52     ` [f2fs-dev] " Christian Brauner
2023-08-21 18:53     ` Gabriel Krisman Bertazi
2023-08-21 18:53       ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-08-22  9:03       ` Christian Brauner
2023-08-22  9:03         ` [f2fs-dev] " Christian Brauner
2023-10-24 22:20         ` Gabriel Krisman Bertazi
2023-10-24 22:20           ` Gabriel Krisman Bertazi
2023-10-25 13:32 ` Christian Brauner
2023-10-25 13:32   ` [f2fs-dev] " Christian Brauner
2023-10-25 15:19   ` Gabriel Krisman Bertazi
2023-10-25 15:19     ` Gabriel Krisman Bertazi
2023-11-19 23:11   ` [f2fs-dev] " Gabriel Krisman Bertazi
2023-11-19 23:11     ` Gabriel Krisman Bertazi
     [not found]   ` <655a9634.630a0220.d50d7.5063SMTPIN_ADDED_BROKEN@mx.google.com>
2023-11-20 15:06     ` Christian Brauner
2023-11-20 15:06       ` Christian Brauner
2023-11-20 16:59       ` Gabriel Krisman Bertazi
2023-11-20 16:59         ` Gabriel Krisman Bertazi
2023-11-20 18:07       ` Linus Torvalds
2023-11-20 18:07         ` Linus Torvalds
2023-11-21  2:02         ` Theodore Ts'o
2023-11-21  2:02           ` Theodore Ts'o
2023-11-21  2:29           ` Linus Torvalds
2023-11-21  2:29             ` Linus Torvalds
2023-11-21  3:03             ` Linus Torvalds
2023-11-21  3:03               ` Linus Torvalds
2023-11-21  5:12               ` Theodore Ts'o
2023-11-21  5:12                 ` Theodore Ts'o
2023-11-22 21:04                 ` Al Viro
2023-11-22 21:04                   ` Al Viro
2023-11-21  2:27         ` Al Viro
2023-11-21  2:27           ` Al Viro
2023-11-22 21:19           ` Al Viro
2023-11-22 21:19             ` Al Viro
2023-11-23  0:18             ` Linus Torvalds
2023-11-23  0:18               ` Linus Torvalds
2023-11-23  5:09               ` Al Viro
2023-11-23  5:09                 ` Al Viro
2023-11-23 15:57               ` Gabriel Krisman Bertazi
2023-11-23 15:57                 ` Gabriel Krisman Bertazi
2023-11-23 17:12                 ` Al Viro
2023-11-23 17:12                   ` Al Viro
2023-11-23 17:37                   ` Gabriel Krisman Bertazi
2023-11-23 17:37                     ` Gabriel Krisman Bertazi
2023-11-23 18:24                     ` Al Viro
2023-11-23 18:24                       ` Al Viro
2023-11-23 19:06                       ` Gabriel Krisman Bertazi
2023-11-23 19:06                         ` Gabriel Krisman Bertazi
2023-11-23 19:53                         ` Al Viro [this message]
2023-11-23 19:53                           ` Al Viro
2023-11-23 20:15                           ` Al Viro
2023-11-23 20:15                             ` Al Viro
2023-11-24 15:20                           ` Gabriel Krisman Bertazi
2023-11-24 15:20                             ` Gabriel Krisman Bertazi
2023-11-28  0:02                             ` Al Viro
2023-11-28  0:02                               ` Al Viro
2023-11-23 21:52                         ` Al Viro
2023-11-23 21:52                           ` Al Viro
2023-11-24 15:22                           ` Gabriel Krisman Bertazi
2023-11-24 15:22                             ` Gabriel Krisman Bertazi
2023-11-25 22:01                             ` Al Viro
2023-11-25 22:01                               ` Al Viro
2023-11-26  4:52                               ` Al Viro
2023-11-26  4:52                                 ` Al Viro
2023-11-26 18:41                                 ` fun with d_invalidate() vs. d_splice_alias() was " Al Viro
2023-11-26 18:41                                   ` [f2fs-dev] fun with d_invalidate() vs. d_splice_alias() was " Al Viro
2023-11-27  6:38                                   ` fun with d_invalidate() vs. d_splice_alias() was Re: [f2fs-dev] " Al Viro
2023-11-27  6:38                                     ` [f2fs-dev] fun with d_invalidate() vs. d_splice_alias() was " Al Viro
2023-11-27 15:47                                     ` fun with d_invalidate() vs. d_splice_alias() was Re: [f2fs-dev] " Eric W. Biederman
2023-11-27 15:47                                       ` [f2fs-dev] fun with d_invalidate() vs. d_splice_alias() was " Eric W. Biederman
2023-11-27 16:01                                       ` fun with d_invalidate() vs. d_splice_alias() was Re: [f2fs-dev] " Eric W. Biederman
2023-11-27 16:01                                         ` [f2fs-dev] fun with d_invalidate() vs. d_splice_alias() was " Eric W. Biederman
2023-11-27 17:25                                         ` fun with d_invalidate() vs. d_splice_alias() was Re: [f2fs-dev] " Al Viro
2023-11-27 17:25                                           ` [f2fs-dev] fun with d_invalidate() vs. d_splice_alias() was " Al Viro
2023-11-27 18:26                                           ` fun with d_invalidate() vs. d_splice_alias() was Re: [f2fs-dev] " Al Viro
2023-11-27 18:26                                             ` [f2fs-dev] fun with d_invalidate() vs. d_splice_alias() was " Al Viro
2023-11-27 16:03                                       ` fun with d_invalidate() vs. d_splice_alias() was Re: [f2fs-dev] " Al Viro
2023-11-27 16:03                                         ` [f2fs-dev] fun with d_invalidate() vs. d_splice_alias() was " Al Viro
2023-11-27 16:14                                         ` fun with d_invalidate() vs. d_splice_alias() was Re: [f2fs-dev] " Al Viro
2023-11-27 16:14                                           ` [f2fs-dev] fun with d_invalidate() vs. d_splice_alias() was " Al Viro
2023-11-27 18:19                                           ` fun with d_invalidate() vs. d_splice_alias() was Re: [f2fs-dev] " Eric W. Biederman
2023-11-27 18:19                                             ` [f2fs-dev] fun with d_invalidate() vs. d_splice_alias() was " Eric W. Biederman
2023-11-27 18:43                                             ` fun with d_invalidate() vs. d_splice_alias() was Re: [f2fs-dev] " Al Viro
2023-11-27 18:43                                               ` [f2fs-dev] fun with d_invalidate() vs. d_splice_alias() was " Al Viro
2023-11-27 16:33                                     ` fun with d_invalidate() vs. d_splice_alias() was Re: [f2fs-dev] " Christian Brauner
2023-11-27 16:33                                       ` [f2fs-dev] fun with d_invalidate() vs. d_splice_alias() was " Christian Brauner
2023-11-29  4:53                                     ` fun with d_invalidate() vs. d_splice_alias() was Re: [f2fs-dev] " Al Viro
2023-11-29  4:53                                       ` [f2fs-dev] fun with d_invalidate() vs. d_splice_alias() was " Al Viro
2023-11-29 10:21                                       ` fun with d_invalidate() vs. d_splice_alias() was Re: [f2fs-dev] " Christian Brauner
2023-11-29 10:21                                         ` [f2fs-dev] fun with d_invalidate() vs. d_splice_alias() was " Christian Brauner
2023-11-29 15:19                                       ` fun with d_invalidate() vs. d_splice_alias() was Re: [f2fs-dev] " Eric W. Biederman
2023-11-29 15:19                                         ` [f2fs-dev] fun with d_invalidate() vs. d_splice_alias() was " Eric W. Biederman
     [not found]               ` <655f7665.df0a0220.58a21.e84fSMTPIN_ADDED_BROKEN@mx.google.com>
2023-11-23 16:41                 ` [f2fs-dev] " Linus Torvalds
2023-11-23 16:41                   ` Linus Torvalds
2023-11-23  1:12             ` Al Viro
2023-11-23  1:12               ` Al Viro
2023-11-23  1:22               ` Al Viro
2023-11-23  1:22                 ` Al Viro
2023-11-22  3:30         ` Gabriel Krisman Bertazi
2023-11-22  3:30           ` Gabriel Krisman Bertazi
2024-01-16 19:02 ` patchwork-bot+f2fs
2024-01-16 19:02   ` patchwork-bot+f2fs

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=20231123195327.GP38156@ZenIV \
    --to=viro@zeniv.linux.org.uk \
    --cc=brauner@kernel.org \
    --cc=ebiggers@kernel.org \
    --cc=gabriel@krisman.be \
    --cc=jaegeuk@kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=tytso@mit.edu \
    /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.