linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Pavel Reichl <preichl@redhat.com>
To: "Darrick J. Wong" <darrick.wong@oracle.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 1/2] mkfs: Break block discard into chunks of 2 GB
Date: Fri, 22 Nov 2019 15:46:39 +0100	[thread overview]
Message-ID: <CAJc7PzW+ibAZ_tPJ_oRPAdfhbop59z53C2pLxmENzWDCu+Gcrg@mail.gmail.com> (raw)
In-Reply-To: <20191121215501.GZ6219@magnolia>

Thanks Darrick for the comments. It makes sense to me, the next
iteration of the patchset will address that.

On Thu, Nov 21, 2019 at 10:57 PM Darrick J. Wong
<darrick.wong@oracle.com> wrote:
>
> On Thu, Nov 21, 2019 at 10:44:44PM +0100, Pavel Reichl wrote:
> > Signed-off-by: Pavel Reichl <preichl@redhat.com>
> > ---
> >  mkfs/xfs_mkfs.c | 32 +++++++++++++++++++++++++-------
> >  1 file changed, 25 insertions(+), 7 deletions(-)
> >
> > diff --git a/mkfs/xfs_mkfs.c b/mkfs/xfs_mkfs.c
> > index 18338a61..a02d6f66 100644
> > --- a/mkfs/xfs_mkfs.c
> > +++ b/mkfs/xfs_mkfs.c
> > @@ -1242,15 +1242,33 @@ done:
> >  static void
> >  discard_blocks(dev_t dev, uint64_t nsectors)
> >  {
> > -     int fd;
> > +     int             fd;
> > +     uint64_t        offset          = 0;
> > +     /* Maximal chunk of bytes to discard is 2GB */
> > +     const uint64_t  step            = (uint64_t)2<<30;
>
> You don't need the tabs after the variable name, e.g.
>
>         /* Maximal chunk of bytes to discard is 2GB */
>         const uint64_t  step = 2ULL << 30;
>
> > +     /* Sector size is 512 bytes */
> > +     const uint64_t  count           = nsectors << 9;
>
> count = BBTOB(nsectors)?
>
> >
> > -     /*
> > -      * We intentionally ignore errors from the discard ioctl.  It is
> > -      * not necessary for the mkfs functionality but just an optimization.
> > -      */
> >       fd = libxfs_device_to_fd(dev);
> > -     if (fd > 0)
> > -             platform_discard_blocks(fd, 0, nsectors << 9);
> > +     if (fd <= 0)
> > +             return;
> > +
> > +     while (offset < count) {
> > +             uint64_t        tmp_step = step;
>
> tmp_step = min(step, count - offset); ?
>
> Otherwise seems reasonable to me, if nothing else to avoid the problem
> where you ask mkfs to discard and can't cancel it....
>
> --D
>
> > +
> > +             if ((offset + step) > count)
> > +                     tmp_step = count - offset;
> > +
> > +             /*
> > +              * We intentionally ignore errors from the discard ioctl. It is
> > +              * not necessary for the mkfs functionality but just an
> > +              * optimization. However we should stop on error.
> > +              */
> > +             if (platform_discard_blocks(fd, offset, tmp_step))
> > +                     return;
> > +
> > +             offset += tmp_step;
> > +     }
> >  }
> >
> >  static __attribute__((noreturn)) void
> > --
> > 2.23.0
> >
>


  reply	other threads:[~2019-11-22 14:46 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-21 21:44 [PATCH 0/2] mkfs: inform during block discarding Pavel Reichl
2019-11-21 21:44 ` [PATCH 1/2] mkfs: Break block discard into chunks of 2 GB Pavel Reichl
2019-11-21 21:55   ` Darrick J. Wong
2019-11-22 14:46     ` Pavel Reichl [this message]
2019-11-22 21:07     ` Eric Sandeen
2019-11-21 23:18   ` Dave Chinner
2019-11-22 15:38     ` Darrick J. Wong
2019-11-22 15:59       ` Pavel Reichl
2019-11-22 21:00         ` Dave Chinner
2019-11-22 16:09     ` Pavel Reichl
2019-11-22 21:10     ` Eric Sandeen
2019-11-22 21:30       ` Eric Sandeen
2019-11-26 19:40         ` Eric Sandeen
2019-11-26 20:53   ` Eric Sandeen
2019-11-21 21:44 ` [PATCH 2/2] mkfs: Show progress during block discard Pavel Reichl
2019-11-21 21:59   ` Darrick J. Wong
2019-11-22 16:27     ` Pavel Reichl
2019-11-22 16:31       ` Darrick J. Wong
2019-11-21 23:41   ` Dave Chinner
2019-11-22 16:43     ` Pavel Reichl
2019-11-22 21:11       ` Dave Chinner
2019-11-22 21:19       ` Eric Sandeen

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=CAJc7PzW+ibAZ_tPJ_oRPAdfhbop59z53C2pLxmENzWDCu+Gcrg@mail.gmail.com \
    --to=preichl@redhat.com \
    --cc=darrick.wong@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    /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 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).