Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Andreas Dilger <adilger@dilger.ca>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: Ira Weiny <ira.weiny@intel.com>,
	Ext4 Developers List <linux-ext4@vger.kernel.org>,
	"Theodore Y. Ts'o" <tytso@mit.edu>, Jan Kara <jack@suse.cz>,
	Eric Biggers <ebiggers@kernel.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Dan Williams <dan.j.williams@intel.com>,
	Dave Chinner <david@fromorbit.com>,
	Christoph Hellwig <hch@lst.de>, Jeff Moyer <jmoyer@redhat.com>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Li Xi <lixi@ddn.com>
Subject: Re: [PATCH V3 7/8] fs/ext4: Introduce DAX inode flag
Date: Wed, 20 May 2020 18:57:49 -0600	[thread overview]
Message-ID: <5D85188E-34E0-49F0-8A77-0AF4CD0EC3E1@dilger.ca> (raw)
In-Reply-To: <20200520205509.GA17615@magnolia>

[-- Attachment #1: Type: text/plain, Size: 3361 bytes --]

On May 20, 2020, at 2:55 PM, Darrick J. Wong <darrick.wong@oracle.com> wrote:
> On Wed, May 20, 2020 at 01:02:42PM -0700, Ira Weiny wrote:
>> On Wed, May 20, 2020 at 01:26:44PM -0600, Andreas Dilger wrote:
>>> On May 19, 2020, at 11:57 PM, ira.weiny@intel.com wrote:
>>>> 
>>>> From: Ira Weiny <ira.weiny@intel.com>
>>>> 
>>>> Add a flag to preserve FS_XFLAG_DAX in the ext4 inode.
>>>> 
>>>> Set the flag to be user visible and changeable.  Set the flag to be
>>>> inherited.  Allow applications to change the flag at any time with the
>>>> exception of if VERITY or ENCRYPT is set.
>>>> 
>>>> Disallow setting VERITY or ENCRYPT if DAX is set.
>>>> 
>>>> Finally, on regular files, flag the inode to not be cached to facilitate
>>>> changing S_DAX on the next creation of the inode.
>>>> 
>>>> Signed-off-by: Ira Weiny <ira.weiny@intel.com>
>>>> 
>>>> ---
>>>> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
>>>> index 6235440e4c39..467c30a789b6 100644
>>>> --- a/fs/ext4/ext4.h
>>>> +++ b/fs/ext4/ext4.h
>>>> @@ -415,13 +415,16 @@ struct flex_groups {
>>>> #define EXT4_VERITY_FL			0x00100000 /* Verity protected inode */
>>>> #define EXT4_EA_INODE_FL	        0x00200000 /* Inode used for large EA */
>>>> /* 0x00400000 was formerly EXT4_EOFBLOCKS_FL */
>>>> +
>>>> +#define EXT4_DAX_FL			0x01000000 /* Inode is DAX */
>>>> +
>>>> #define EXT4_INLINE_DATA_FL		0x10000000 /* Inode has inline data. */
>>>> #define EXT4_PROJINHERIT_FL		0x20000000 /* Create with parents projid */
>>>> #define EXT4_CASEFOLD_FL		0x40000000 /* Casefolded file */
>>>> #define EXT4_RESERVED_FL		0x80000000 /* reserved for ext4 lib */
>>> 
>>> Hi Ira,
>>> This flag value conflicts with the reserved flag in e2fsprogs for snapshots:
>>> 
>>> #define EXT4_SNAPFILE_FL                0x01000000  /* Inode is a snapshot */
>> 
>> Sure NP but is that new?  I'm building off of 5.7-rc4.
>> 
>> Just curious if I completely missed something.
> 
> Yeah, you missed that ... for some reason the kernel ext4 driver is
> missing flags that are in e2fsprogs.  (huh??)

It's no different than ext2 not having the full set of bits defined or
in use.

> I would say you could probably just take over the flag because the 2010s
> called and they don't want next3 back.  I guess that leaves 0x02000000
> as the sole unclaimed bit, but this seriously needs some cleaning.

Darrick,
we are in the process of updating the snapshot code for ext4, so need to
keep the 0x01000000 bit for snapshots.  Since 0x02000000 has never been
used for anything, there is no reason not to use it instead.

If we need to reclaim flags, it would be better to look at "COMPR" flags:

/* Reserved for compression usage... */
#define FS_COMPR_FL           0x00000004 /* Compress file */
#define FS_DIRTY_FL           0x00000100
#define FS_COMPRBLK_FL        0x00000200 /* One or more compressed clusters */
#define FS_NOCOMP_FL          0x00000400 /* Don't compress */

since I don't think they have ever been used.  I don't think we need 4x
on-disk state flags for that, especially not as part of the API.  It is
enough to have FS_COMPR_FL for the API, and then handle internal state
separately (e.g. compress into a separate on-disk extent and then swap
extents atomically instead of storing transient state on disk).

Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

  reply	other threads:[~2020-05-21  0:58 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-20  5:57 [PATCH V3 0/8] Enable ext4 support for per-file/directory DAX operations ira.weiny
2020-05-20  5:57 ` [PATCH V3 1/8] fs/ext4: Narrow scope of DAX check in setflags ira.weiny
2020-05-20  5:57 ` [PATCH V3 2/8] fs/ext4: Disallow verity if inode is DAX ira.weiny
2020-05-20  5:57 ` [PATCH V3 3/8] fs/ext4: Change EXT4_MOUNT_DAX to EXT4_MOUNT_DAX_ALWAYS ira.weiny
2020-05-20  5:57 ` [PATCH V3 4/8] fs/ext4: Update ext4_should_use_dax() ira.weiny
2020-05-20 13:37   ` Jan Kara
2020-05-20 19:40     ` Ira Weiny
2020-05-21 10:24       ` Jan Kara
2020-05-20  5:57 ` [PATCH V3 5/8] fs/ext4: Only change S_DAX on inode load ira.weiny
2020-05-20  5:57 ` [PATCH V3 6/8] fs/ext4: Make DAX mount option a tri-state ira.weiny
2020-05-20  5:57 ` [PATCH V3 7/8] fs/ext4: Introduce DAX inode flag ira.weiny
2020-05-20 14:11   ` Jan Kara
2020-05-20 18:34     ` Ira Weiny
2020-05-20 19:26   ` Andreas Dilger
2020-05-20 20:02     ` Ira Weiny
2020-05-20 20:55       ` Darrick J. Wong
2020-05-21  0:57         ` Andreas Dilger [this message]
2020-05-20  5:57 ` [PATCH V3 8/8] Documentation/dax: Update DAX enablement for ext4 ira.weiny

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=5D85188E-34E0-49F0-8A77-0AF4CD0EC3E1@dilger.ca \
    --to=adilger@dilger.ca \
    --cc=dan.j.williams@intel.com \
    --cc=darrick.wong@oracle.com \
    --cc=david@fromorbit.com \
    --cc=ebiggers@kernel.org \
    --cc=hch@lst.de \
    --cc=ira.weiny@intel.com \
    --cc=jack@suse.cz \
    --cc=jmoyer@redhat.com \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lixi@ddn.com \
    --cc=tytso@mit.edu \
    --cc=viro@zeniv.linux.org.uk \
    --subject='Re: [PATCH V3 7/8] fs/ext4: Introduce DAX inode flag' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).