On Wed, Sep 19, 2018 at 07:44:05AM -0700, Darrick J. Wong wrote:
> On Wed, Sep 19, 2018 at 02:56:16PM +0200, Jan Tulak wrote:
> > Discard should happen only when everything has been validated, just
> > before we start writing to the device. If it happens earlier, it is
> > possible that mkfs will abort, but managed to already wipe data. This
> > patch moves the discard to the latest possible moment.
> >
> > Signed-off-by: Jan Tulak <jtulak@xxxxxxxxxx>
> > ---
> > mkfs/xfs_mkfs.c | 20 ++++++++++++++++----
> > 1 file changed, 16 insertions(+), 4 deletions(-)
> >
> > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> > index 2e53c1e8..81d9859a 100644
> > --- a/mkfs/xfs_mkfs.c
> > +++ b/mkfs/xfs_mkfs.c
> > @@ -2389,8 +2389,7 @@ _("log stripe unit (%d bytes) is too large (maximum is 256KiB)\n"
> > static void
> > open_devices(
> > struct mkfs_params *cfg,
> > - struct libxfs_xinit *xi,
> > - bool discard)
> > + struct libxfs_xinit *xi)
> > {
> > uint64_t sector_mask;
> >
> > @@ -2419,8 +2418,16 @@ open_devices(
> > xi->dsize &= sector_mask;
> > xi->rtsize &= sector_mask;
> > xi->logBBsize &= (uint64_t)-1 << (max(cfg->lsectorlog, 10) - BBSHIFT);
> > +}
> >
> > -
> > +static void
> > +discard_data(
>
> Perhaps discard_devices(), since this function can DISCARD more than
> just the data device.
>
> > + struct libxfs_xinit *xi,
> > + bool discard)
> > +{
> > + /*
> > + * This function has to be called after libxfs has been initialized.
> > + */
> > if (!discard)
> > return;
> >
>
> While we're on the topic, I notice that we skip discard for any device
> that's actually a file. Seeing as fallocate(PUNCH_HOLE) works on files
> (and block devices), is there a reason why we avoid punching out fs
> image files?
Yeah - preallocated image files shouldn't be punched, because it
defeats the whole purpose of setting up preallocated image files.
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
