Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Thu, 8 Apr 2021 07:15:00 +1000
From:   Dave Chinner <david@fromorbit.com>
To:     "Darrick J. Wong" <djwong@kernel.org>
Cc:     Leah Rumancik <leah.rumancik@gmail.com>, linux-ext4@vger.kernel.org
Subject: Re: [PATCH v2 2/2] ext4: add ioctl FS_IOC_CHKPT_JRNL
Message-ID: <20210407211500.GG1990290@dread.disaster.area>
References: <20210407154202.1527941-1-leah.rumancik@gmail.com>
 <20210407154202.1527941-3-leah.rumancik@gmail.com>
 <20210407183547.GG22091@magnolia>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210407183547.GG22091@magnolia>
Precedence: bulk

On Wed, Apr 07, 2021 at 11:35:47AM -0700, Darrick J. Wong wrote:
> On Wed, Apr 07, 2021 at 03:42:02PM +0000, Leah Rumancik wrote:
> > ioctl FS_IOC_CHKPT_JRNL checkpoints and flushes the journal. With the
> > CHKPT_JRNL_DISCARD flag set, the journal blocks are also discarded.
> > With the filename wipeout patch, Ext4 guarantees that all data will be
> > discarded on deletion. This ioctl allows for periodically discarding
> > journal contents too.
> 
> This needs a documentation update to cover what this new userspace ABI
> does, and probably linux-api and linux-fsdevel should be cc'd.

You need to describe the semantics that you are exporting to
userspace. Exactly what does a "journal checkpoint" mean from the
point of view of user visible metadata and data integrity? How is it
different to running fsync() on a directory, syncfs() or freeze on a
filesystem, or any of the other methods we already have for
guaranteeing completed changes are committed to stable storage? All
of these methods imply a journal checkpoint of some kind is done in
ext4, so why do we need a specific ioctl to do this?

But, really, if this is for secure deletion, then why isn't
"fsync(dirfd)" sufficient to force the unlinks into the journal and
onto stable storage? Why does this functionality need some special
new CAP_SYS_ADMIN only ioctl to checkpoint the journal when, by
definition, fsync() should already be doing that?  Indeed, users can't
actually use this new ioctl for secure deletion because it is root
only, so how do users and their applications actually ensure that
secure deletion of their files has actually occurred?

I'm also curious as to what "journal checkpoint" means for
filesystems that don't have journals? Or that have multiple and/or
partial state journals (e.g. iper-inode journals in NOVA, fsync
journals in brtfs, etc)? What does it mean for filesystems that use
COW instead of journals?

> > Also, add journal discard (if discard supported) during journal load
> > after recovery. This provides a potential solution to
> > https://lore.kernel.org/linux-ext4/YDZoaacIYStFQT8g@mit.edu/ for
> > disks that support discard. After a successful journal recovery, e2fsck
> > can call this ioctl to discard the journal as well.

If you need discard after recovery for secure remove, then you also
really need discard on every extent being freed, too.  Which then
implies that the -o discard mount option needs to be used in
conjunction with this functionality. Perhaps, then, journal discard
at mount should be tied in to the -o discard mount option, and then
the need for an ioctl to trigger this goes away completely.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com