| Message ID | 20210809164410.27750-1-jlayton@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | ceph: enable async dirops by default | expand |
On Mon, 2021-08-09 at 12:44 -0400, Jeff Layton wrote: > Async dirops have been supported in mainline kernels for quite some time > now, and we've recently (as of June) started doing regular testing in > teuthology with '-o nowsync'. So far, that hasn't uncovered any issues, > so I think the time is right to flip the default for this option. > > Enable async dirops by default, and change /proc/mounts to show "wsync" > when they are disabled rather than "nowsync" when they are enabled. > > Cc: Patrick Donnelly <pdonnell@redhat.com> > Signed-off-by: Jeff Layton <jlayton@kernel.org> > --- > fs/ceph/super.c | 4 ++-- > fs/ceph/super.h | 3 ++- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/fs/ceph/super.c b/fs/ceph/super.c > index 609ffc8c2d78..f517ad9eeb26 100644 > --- a/fs/ceph/super.c > +++ b/fs/ceph/super.c > @@ -698,8 +698,8 @@ static int ceph_show_options(struct seq_file *m, struct dentry *root) > if (fsopt->flags & CEPH_MOUNT_OPT_CLEANRECOVER) > seq_show_option(m, "recover_session", "clean"); > > - if (fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS) > - seq_puts(m, ",nowsync"); > + if (!(fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS)) > + seq_puts(m, ",wsync"); > > if (fsopt->wsize != CEPH_MAX_WRITE_SIZE) > seq_printf(m, ",wsize=%u", fsopt->wsize); > diff --git a/fs/ceph/super.h b/fs/ceph/super.h > index 389b45ac291b..0bc36cf4c683 100644 > --- a/fs/ceph/super.h > +++ b/fs/ceph/super.h > @@ -48,7 +48,8 @@ > > #define CEPH_MOUNT_OPT_DEFAULT \ > (CEPH_MOUNT_OPT_DCACHE | \ > - CEPH_MOUNT_OPT_NOCOPYFROM) > + CEPH_MOUNT_OPT_NOCOPYFROM | \ > + CEPH_MOUNT_OPT_ASYNC_DIROPS) > > #define ceph_set_mount_opt(fsc, opt) \ > (fsc)->mount_options->flags |= CEPH_MOUNT_OPT_##opt I think we ought to wait to merge this into mainline just yet, but I'd like to leave it in the "testing" branch for now. I've been working this bug with Patrick for the last month or two: https://tracker.ceph.com/issues/51279 In at least one case, the problem seems to be that the MDS failed an async create with an ENOSPC error. The kclient's error handling around this is pretty non-existent right now, so it caused an unmount to hang. It's a pity we still don't have revoke()... I've started working on a series to clean this up, but in the meantime I think we ought to wait until that's in place before we make nowsync the default. Sound ok? -- Jeff Layton <jlayton@kernel.org>
On Wed, Sep 1, 2021 at 6:54 PM Jeff Layton <jlayton@kernel.org> wrote: > > On Mon, 2021-08-09 at 12:44 -0400, Jeff Layton wrote: > > Async dirops have been supported in mainline kernels for quite some time > > now, and we've recently (as of June) started doing regular testing in > > teuthology with '-o nowsync'. So far, that hasn't uncovered any issues, > > so I think the time is right to flip the default for this option. > > > > Enable async dirops by default, and change /proc/mounts to show "wsync" > > when they are disabled rather than "nowsync" when they are enabled. > > > > Cc: Patrick Donnelly <pdonnell@redhat.com> > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > --- > > fs/ceph/super.c | 4 ++-- > > fs/ceph/super.h | 3 ++- > > 2 files changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/fs/ceph/super.c b/fs/ceph/super.c > > index 609ffc8c2d78..f517ad9eeb26 100644 > > --- a/fs/ceph/super.c > > +++ b/fs/ceph/super.c > > @@ -698,8 +698,8 @@ static int ceph_show_options(struct seq_file *m, struct dentry *root) > > if (fsopt->flags & CEPH_MOUNT_OPT_CLEANRECOVER) > > seq_show_option(m, "recover_session", "clean"); > > > > - if (fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS) > > - seq_puts(m, ",nowsync"); > > + if (!(fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS)) > > + seq_puts(m, ",wsync"); > > > > if (fsopt->wsize != CEPH_MAX_WRITE_SIZE) > > seq_printf(m, ",wsize=%u", fsopt->wsize); > > diff --git a/fs/ceph/super.h b/fs/ceph/super.h > > index 389b45ac291b..0bc36cf4c683 100644 > > --- a/fs/ceph/super.h > > +++ b/fs/ceph/super.h > > @@ -48,7 +48,8 @@ > > > > #define CEPH_MOUNT_OPT_DEFAULT \ > > (CEPH_MOUNT_OPT_DCACHE | \ > > - CEPH_MOUNT_OPT_NOCOPYFROM) > > + CEPH_MOUNT_OPT_NOCOPYFROM | \ > > + CEPH_MOUNT_OPT_ASYNC_DIROPS) > > > > #define ceph_set_mount_opt(fsc, opt) \ > > (fsc)->mount_options->flags |= CEPH_MOUNT_OPT_##opt > > I think we ought to wait to merge this into mainline just yet, but I'd > like to leave it in the "testing" branch for now. > > I've been working this bug with Patrick for the last month or two: > > https://tracker.ceph.com/issues/51279 > > In at least one case, the problem seems to be that the MDS failed an > async create with an ENOSPC error. The kclient's error handling around > this is pretty non-existent right now, so it caused an unmount to hang. > > It's a pity we still don't have revoke()... > > I've started working on a series to clean this up, but in the meantime I > think we ought to wait until that's in place before we make nowsync the > default. > > Sound ok? Yeah, I don't see any rush to make it the default especially given that it hasn't been on in testing that long. Thanks, Ilya
On Wed, Sep 1, 2021 at 12:54 PM Jeff Layton <jlayton@kernel.org> wrote: > > On Mon, 2021-08-09 at 12:44 -0400, Jeff Layton wrote: > > Async dirops have been supported in mainline kernels for quite some time > > now, and we've recently (as of June) started doing regular testing in > > teuthology with '-o nowsync'. So far, that hasn't uncovered any issues, > > so I think the time is right to flip the default for this option. > > > > Enable async dirops by default, and change /proc/mounts to show "wsync" > > when they are disabled rather than "nowsync" when they are enabled. > > > > Cc: Patrick Donnelly <pdonnell@redhat.com> > > Signed-off-by: Jeff Layton <jlayton@kernel.org> > > --- > > fs/ceph/super.c | 4 ++-- > > fs/ceph/super.h | 3 ++- > > 2 files changed, 4 insertions(+), 3 deletions(-) > > > > diff --git a/fs/ceph/super.c b/fs/ceph/super.c > > index 609ffc8c2d78..f517ad9eeb26 100644 > > --- a/fs/ceph/super.c > > +++ b/fs/ceph/super.c > > @@ -698,8 +698,8 @@ static int ceph_show_options(struct seq_file *m, struct dentry *root) > > if (fsopt->flags & CEPH_MOUNT_OPT_CLEANRECOVER) > > seq_show_option(m, "recover_session", "clean"); > > > > - if (fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS) > > - seq_puts(m, ",nowsync"); > > + if (!(fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS)) > > + seq_puts(m, ",wsync"); > > > > if (fsopt->wsize != CEPH_MAX_WRITE_SIZE) > > seq_printf(m, ",wsize=%u", fsopt->wsize); > > diff --git a/fs/ceph/super.h b/fs/ceph/super.h > > index 389b45ac291b..0bc36cf4c683 100644 > > --- a/fs/ceph/super.h > > +++ b/fs/ceph/super.h > > @@ -48,7 +48,8 @@ > > > > #define CEPH_MOUNT_OPT_DEFAULT \ > > (CEPH_MOUNT_OPT_DCACHE | \ > > - CEPH_MOUNT_OPT_NOCOPYFROM) > > + CEPH_MOUNT_OPT_NOCOPYFROM | \ > > + CEPH_MOUNT_OPT_ASYNC_DIROPS) > > > > #define ceph_set_mount_opt(fsc, opt) \ > > (fsc)->mount_options->flags |= CEPH_MOUNT_OPT_##opt > > I think we ought to wait to merge this into mainline just yet, but I'd > like to leave it in the "testing" branch for now. > > I've been working this bug with Patrick for the last month or two: > > https://tracker.ceph.com/issues/51279 > > In at least one case, the problem seems to be that the MDS failed an > async create with an ENOSPC error. The kclient's error handling around > this is pretty non-existent right now, so it caused an unmount to hang. > > It's a pity we still don't have revoke()... > > I've started working on a series to clean this up, but in the meantime I > think we ought to wait until that's in place before we make nowsync the > default. > > Sound ok? Sounds good to me. -- Patrick Donnelly, Ph.D. He / Him / His Principal Software Engineer Red Hat Sunnyvale, CA GPG: 19F28A586F808C2402351B93C3301A3E258DD79D
diff --git a/fs/ceph/super.c b/fs/ceph/super.c index 609ffc8c2d78..f517ad9eeb26 100644 --- a/fs/ceph/super.c +++ b/fs/ceph/super.c @@ -698,8 +698,8 @@ static int ceph_show_options(struct seq_file *m, struct dentry *root) if (fsopt->flags & CEPH_MOUNT_OPT_CLEANRECOVER) seq_show_option(m, "recover_session", "clean"); - if (fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS) - seq_puts(m, ",nowsync"); + if (!(fsopt->flags & CEPH_MOUNT_OPT_ASYNC_DIROPS)) + seq_puts(m, ",wsync"); if (fsopt->wsize != CEPH_MAX_WRITE_SIZE) seq_printf(m, ",wsize=%u", fsopt->wsize); diff --git a/fs/ceph/super.h b/fs/ceph/super.h index 389b45ac291b..0bc36cf4c683 100644 --- a/fs/ceph/super.h +++ b/fs/ceph/super.h @@ -48,7 +48,8 @@ #define CEPH_MOUNT_OPT_DEFAULT \ (CEPH_MOUNT_OPT_DCACHE | \ - CEPH_MOUNT_OPT_NOCOPYFROM) + CEPH_MOUNT_OPT_NOCOPYFROM | \ + CEPH_MOUNT_OPT_ASYNC_DIROPS) #define ceph_set_mount_opt(fsc, opt) \ (fsc)->mount_options->flags |= CEPH_MOUNT_OPT_##opt
Async dirops have been supported in mainline kernels for quite some time now, and we've recently (as of June) started doing regular testing in teuthology with '-o nowsync'. So far, that hasn't uncovered any issues, so I think the time is right to flip the default for this option. Enable async dirops by default, and change /proc/mounts to show "wsync" when they are disabled rather than "nowsync" when they are enabled. Cc: Patrick Donnelly <pdonnell@redhat.com> Signed-off-by: Jeff Layton <jlayton@kernel.org> --- fs/ceph/super.c | 4 ++-- fs/ceph/super.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-)