All of lore.kernel.org
 help / color / mirror / Atom feed
From: Gabriel Krisman Bertazi <krisman@collabora.com>
To: Gao Xiang <gaoxiang25@huawei.com>
Cc: Daniel Rosenberg <drosen@google.com>,
	Theodore Ts'o <tytso@mit.edu>, <linux-ext4@vger.kernel.org>,
	Jaegeuk Kim <jaegeuk@kernel.org>, Chao Yu <chao@kernel.org>,
	<linux-f2fs-devel@lists.sourceforge.net>,
	Eric Biggers <ebiggers@kernel.org>,
	<linux-fscrypt@vger.kernel.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Andreas Dilger <adilger.kernel@dilger.ca>,
	Jonathan Corbet <corbet@lwn.net>, <linux-doc@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-fsdevel@vger.kernel.org>,
	<kernel-team@android.com>
Subject: Re: [PATCH 4/8] vfs: Fold casefolding into vfs
Date: Tue, 03 Dec 2019 14:42:10 -0500	[thread overview]
Message-ID: <85wobdb3hp.fsf@collabora.com> (raw)
In-Reply-To: <20191203074154.GA216261@architecture4> (Gao Xiang's message of "Tue, 3 Dec 2019 15:41:54 +0800")

Gao Xiang <gaoxiang25@huawei.com> writes:

> On Mon, Dec 02, 2019 at 09:10:45PM -0800, Daniel Rosenberg wrote:
>> Ext4 and F2fs are both using casefolding, and they, along with any other
>> filesystem that adds the feature, will be using identical dentry_ops.
>> Additionally, those dentry ops interfere with the dentry_ops required
>> for fscrypt once we add support for casefolding and encryption.
>> Moving this into the vfs removes code duplication as well as the
>> complication with encryption.
>> 
>> Currently this is pretty close to just moving the existing f2fs/ext4
>> code up a level into the vfs, although there is a lot of room for
>> improvement now.
>> 
>> Signed-off-by: Daniel Rosenberg <drosen@google.com>
>
> I'm afraid that such vfs modification is unneeded.
>
> Just a quick glance it seems just can be replaced by introducing some
> .d_cmp, .d_hash helpers (or with little modification) and most non-Android
> emulated storage files are not casefolded (even in Android).
>
> "those dentry ops interfere with the dentry_ops required for fscrypt",
> I don't think it's a real diffculty and it could be done with some
> better approach instead.

It would be good to avoid dentry_ops in general for these cases.  It
doesn't just interfere with fscrypt, but also overlayfs and others.

The difficulty is that it is not trivial to change dentry_ops after
dentries are already installed in the dcache.  Which means that it is
hard to use different dentry_ops for different parts of the filesystem,
for instance when converting a directory to case-insensitive or back
to case-sensitive.

In fact, currently and for case-insensitive at least, we install generic
hooks for the entire case-insensitive filesystem and use it even for
!IS_CASEFOLDED() directories. This breaks overlayfs even if we don't
have a single IS_CASEFOLDED() directory at all, just by having the
superblock flag, we *must* set the dentry_ops, which already breaks
overlayfs.

I think Daniel's approach of moving this into VFS is the simplest way to
actually solve the issue, instead of extending and duplicating a lot of
functionality into filesystem hooks to support the possible mixes of
case-insensitive, overlayfs and fscrypt.

-- 
Gabriel Krisman Bertazi

WARNING: multiple messages have this Message-ID (diff)
From: Gabriel Krisman Bertazi <krisman@collabora.com>
To: Gao Xiang <gaoxiang25@huawei.com>
Cc: Theodore Ts'o <tytso@mit.edu>,
	Daniel Rosenberg <drosen@google.com>,
	Jonathan Corbet <corbet@lwn.net>,
	linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-f2fs-devel@lists.sourceforge.net,
	Eric Biggers <ebiggers@kernel.org>,
	linux-fscrypt@vger.kernel.org,
	Andreas Dilger <adilger.kernel@dilger.ca>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	linux-fsdevel@vger.kernel.org, Jaegeuk Kim <jaegeuk@kernel.org>,
	linux-ext4@vger.kernel.org, kernel-team@android.com
Subject: Re: [f2fs-dev] [PATCH 4/8] vfs: Fold casefolding into vfs
Date: Tue, 03 Dec 2019 14:42:10 -0500	[thread overview]
Message-ID: <85wobdb3hp.fsf@collabora.com> (raw)
In-Reply-To: <20191203074154.GA216261@architecture4> (Gao Xiang's message of "Tue, 3 Dec 2019 15:41:54 +0800")

Gao Xiang <gaoxiang25@huawei.com> writes:

> On Mon, Dec 02, 2019 at 09:10:45PM -0800, Daniel Rosenberg wrote:
>> Ext4 and F2fs are both using casefolding, and they, along with any other
>> filesystem that adds the feature, will be using identical dentry_ops.
>> Additionally, those dentry ops interfere with the dentry_ops required
>> for fscrypt once we add support for casefolding and encryption.
>> Moving this into the vfs removes code duplication as well as the
>> complication with encryption.
>> 
>> Currently this is pretty close to just moving the existing f2fs/ext4
>> code up a level into the vfs, although there is a lot of room for
>> improvement now.
>> 
>> Signed-off-by: Daniel Rosenberg <drosen@google.com>
>
> I'm afraid that such vfs modification is unneeded.
>
> Just a quick glance it seems just can be replaced by introducing some
> .d_cmp, .d_hash helpers (or with little modification) and most non-Android
> emulated storage files are not casefolded (even in Android).
>
> "those dentry ops interfere with the dentry_ops required for fscrypt",
> I don't think it's a real diffculty and it could be done with some
> better approach instead.

It would be good to avoid dentry_ops in general for these cases.  It
doesn't just interfere with fscrypt, but also overlayfs and others.

The difficulty is that it is not trivial to change dentry_ops after
dentries are already installed in the dcache.  Which means that it is
hard to use different dentry_ops for different parts of the filesystem,
for instance when converting a directory to case-insensitive or back
to case-sensitive.

In fact, currently and for case-insensitive at least, we install generic
hooks for the entire case-insensitive filesystem and use it even for
!IS_CASEFOLDED() directories. This breaks overlayfs even if we don't
have a single IS_CASEFOLDED() directory at all, just by having the
superblock flag, we *must* set the dentry_ops, which already breaks
overlayfs.

I think Daniel's approach of moving this into VFS is the simplest way to
actually solve the issue, instead of extending and duplicating a lot of
functionality into filesystem hooks to support the possible mixes of
case-insensitive, overlayfs and fscrypt.

-- 
Gabriel Krisman Bertazi


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

  reply	other threads:[~2019-12-03 19:42 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-03  5:10 [PATCH 0/8] Support for Casefolding and Encryption Daniel Rosenberg
2019-12-03  5:10 ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel
2019-12-03  5:10 ` [PATCH 1/8] fscrypt: Add siphash and hash key for policy v2 Daniel Rosenberg
2019-12-03  5:10   ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel
2019-12-03 23:25   ` Eric Biggers
2019-12-03 23:25     ` [f2fs-dev] " Eric Biggers
2019-12-03  5:10 ` [PATCH 2/8] fscrypt: Don't allow v1 policies with casefolding Daniel Rosenberg
2019-12-03  5:10   ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel
2019-12-03 23:37   ` Eric Biggers
2019-12-03 23:37     ` [f2fs-dev] " Eric Biggers
2019-12-03  5:10 ` [PATCH 3/8] fscrypt: Change format of no-key token Daniel Rosenberg
2019-12-03  5:10   ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel
2019-12-04  0:09   ` Eric Biggers
2019-12-04  0:09     ` [f2fs-dev] " Eric Biggers
2019-12-03  5:10 ` [PATCH 4/8] vfs: Fold casefolding into vfs Daniel Rosenberg
2019-12-03  5:10   ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel
2019-12-03  7:41   ` Gao Xiang
2019-12-03  7:41     ` [f2fs-dev] " Gao Xiang
2019-12-03 19:42     ` Gabriel Krisman Bertazi [this message]
2019-12-03 19:42       ` Gabriel Krisman Bertazi
2019-12-03 20:34       ` Eric Biggers
2019-12-03 20:34         ` [f2fs-dev] " Eric Biggers
2019-12-03 21:21         ` Gabriel Krisman Bertazi
2019-12-03 21:21           ` [f2fs-dev] " Gabriel Krisman Bertazi
2019-12-04  0:32           ` Eric Biggers
2019-12-04  0:32             ` [f2fs-dev] " Eric Biggers
2019-12-03 19:31   ` Gabriel Krisman Bertazi
2019-12-03 19:31     ` [f2fs-dev] " Gabriel Krisman Bertazi
2020-01-03 20:26   ` Theodore Y. Ts'o
2020-01-03 20:26     ` [f2fs-dev] " Theodore Y. Ts'o
2019-12-03  5:10 ` [PATCH 5/8] f2fs: Handle casefolding with Encryption Daniel Rosenberg
2019-12-03  5:10   ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel
2019-12-05  1:17   ` kbuild test robot
2019-12-05  1:17     ` kbuild test robot
2019-12-05  1:17     ` [f2fs-dev] " kbuild test robot
2019-12-03  5:10 ` [PATCH 6/8] ext4: Use struct super_block's casefold data Daniel Rosenberg
2019-12-03  5:10   ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel
2019-12-03 19:44   ` Gabriel Krisman Bertazi
2019-12-03 19:44     ` [f2fs-dev] " Gabriel Krisman Bertazi
2019-12-03  5:10 ` [PATCH 7/8] ext4: Hande casefolding with encryption Daniel Rosenberg
2019-12-03  5:10   ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel
2019-12-03  5:10 ` [PATCH 8/8] ext4: Optimize match for casefolded encrypted dirs Daniel Rosenberg
2019-12-03  5:10   ` [f2fs-dev] " Daniel Rosenberg via Linux-f2fs-devel

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=85wobdb3hp.fsf@collabora.com \
    --to=krisman@collabora.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=chao@kernel.org \
    --cc=corbet@lwn.net \
    --cc=drosen@google.com \
    --cc=ebiggers@kernel.org \
    --cc=gaoxiang25@huawei.com \
    --cc=jaegeuk@kernel.org \
    --cc=kernel-team@android.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-f2fs-devel@lists.sourceforge.net \
    --cc=linux-fscrypt@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tytso@mit.edu \
    --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.