| Message ID | 156693231821.718166.10779376892986285406.stgit@dwillia2-desk3.amr.corp.intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | b522dd55c0b329b08cd019f332b6c11bd1c1ac39 |
| Headers | show |
| Series | [ndctl] ndctl/namespace: Fix 'clear-error -s' excessive scrubbing | expand |
On Tue, 2019-08-27 at 11:58 -0700, Dan Williams wrote: > Erwin reports: > The current implementation of ndctl clear-errors takes a very long time, > because a full scrub happens for every namespace. > > Doing a full system ARS scrub is obviously not necessary, it just needs > to happen once. > > Indeed, it only needs to happen once per bus. Clear the 'scrub' option > after one namespace_clear_bb() invocation, and reset it when looping to > the next bus. > > Link: https://github.com/pmem/ndctl/issues/104 > Reported-by: Erwin Tsaur <erwin.tsaur@oracle.com> > Fixes: 2ba7b8277075 ("ndctl: add a 'clear-errors' command") > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > ndctl/namespace.c | 18 +++++++++++++----- > 1 file changed, 13 insertions(+), 5 deletions(-) Looks good, applied. > > diff --git a/ndctl/namespace.c b/ndctl/namespace.c > index bbc9107c6baa..c1af7d0db153 100644 > --- a/ndctl/namespace.c > +++ b/ndctl/namespace.c > @@ -1295,7 +1295,7 @@ static int raw_clear_badblocks(struct ndctl_namespace *ndns) > return nstype_clear_badblocks(ndns, devname, begin, size); > } > > -static int namespace_wait_scrub(struct ndctl_namespace *ndns) > +static int namespace_wait_scrub(struct ndctl_namespace *ndns, bool do_scrub) > { > const char *devname = ndctl_namespace_get_devname(ndns); > struct ndctl_bus *bus = ndctl_namespace_get_bus(ndns); > @@ -1309,7 +1309,7 @@ static int namespace_wait_scrub(struct ndctl_namespace *ndns) > } > > /* start a scrub if asked and if one isn't in progress */ > - if (scrub && (!in_progress)) { > + if (do_scrub && (!in_progress)) { > rc = ndctl_bus_start_scrub(bus); > if (rc) { > error("%s: Unable to start scrub: %s\n", devname, > @@ -1332,7 +1332,7 @@ static int namespace_wait_scrub(struct ndctl_namespace *ndns) > return 0; > } > > -static int namespace_clear_bb(struct ndctl_namespace *ndns) > +static int namespace_clear_bb(struct ndctl_namespace *ndns, bool do_scrub) > { > struct ndctl_pfn *pfn = ndctl_namespace_get_pfn(ndns); > struct ndctl_dax *dax = ndctl_namespace_get_dax(ndns); > @@ -1347,7 +1347,7 @@ static int namespace_clear_bb(struct ndctl_namespace *ndns) > return 1; > } > > - rc = namespace_wait_scrub(ndns); > + rc = namespace_wait_scrub(ndns, do_scrub); > if (rc) > return rc; > > @@ -1716,9 +1716,14 @@ static int do_xaction_namespace(const char *namespace, > ndctl_set_log_priority(ctx, LOG_DEBUG); > > ndctl_bus_foreach(ctx, bus) { > + bool do_scrub; > + > if (!util_bus_filter(bus, param.bus)) > continue; > > + /* only request scrubbing once per bus */ > + do_scrub = scrub; > + > ndctl_region_foreach(bus, region) { > if (!util_region_filter(region, param.region)) > continue; > @@ -1778,7 +1783,10 @@ static int do_xaction_namespace(const char *namespace, > (*processed)++; > break; > case ACTION_CLEAR: > - rc = namespace_clear_bb(ndns); > + rc = namespace_clear_bb(ndns, do_scrub); > + > + /* one scrub per bus is sufficient */ > + do_scrub = false; > if (rc == 0) > (*processed)++; > break; >
diff --git a/ndctl/namespace.c b/ndctl/namespace.c index bbc9107c6baa..c1af7d0db153 100644 --- a/ndctl/namespace.c +++ b/ndctl/namespace.c @@ -1295,7 +1295,7 @@ static int raw_clear_badblocks(struct ndctl_namespace *ndns) return nstype_clear_badblocks(ndns, devname, begin, size); } -static int namespace_wait_scrub(struct ndctl_namespace *ndns) +static int namespace_wait_scrub(struct ndctl_namespace *ndns, bool do_scrub) { const char *devname = ndctl_namespace_get_devname(ndns); struct ndctl_bus *bus = ndctl_namespace_get_bus(ndns); @@ -1309,7 +1309,7 @@ static int namespace_wait_scrub(struct ndctl_namespace *ndns) } /* start a scrub if asked and if one isn't in progress */ - if (scrub && (!in_progress)) { + if (do_scrub && (!in_progress)) { rc = ndctl_bus_start_scrub(bus); if (rc) { error("%s: Unable to start scrub: %s\n", devname, @@ -1332,7 +1332,7 @@ static int namespace_wait_scrub(struct ndctl_namespace *ndns) return 0; } -static int namespace_clear_bb(struct ndctl_namespace *ndns) +static int namespace_clear_bb(struct ndctl_namespace *ndns, bool do_scrub) { struct ndctl_pfn *pfn = ndctl_namespace_get_pfn(ndns); struct ndctl_dax *dax = ndctl_namespace_get_dax(ndns); @@ -1347,7 +1347,7 @@ static int namespace_clear_bb(struct ndctl_namespace *ndns) return 1; } - rc = namespace_wait_scrub(ndns); + rc = namespace_wait_scrub(ndns, do_scrub); if (rc) return rc; @@ -1716,9 +1716,14 @@ static int do_xaction_namespace(const char *namespace, ndctl_set_log_priority(ctx, LOG_DEBUG); ndctl_bus_foreach(ctx, bus) { + bool do_scrub; + if (!util_bus_filter(bus, param.bus)) continue; + /* only request scrubbing once per bus */ + do_scrub = scrub; + ndctl_region_foreach(bus, region) { if (!util_region_filter(region, param.region)) continue; @@ -1778,7 +1783,10 @@ static int do_xaction_namespace(const char *namespace, (*processed)++; break; case ACTION_CLEAR: - rc = namespace_clear_bb(ndns); + rc = namespace_clear_bb(ndns, do_scrub); + + /* one scrub per bus is sufficient */ + do_scrub = false; if (rc == 0) (*processed)++; break;
Erwin reports: The current implementation of ndctl clear-errors takes a very long time, because a full scrub happens for every namespace. Doing a full system ARS scrub is obviously not necessary, it just needs to happen once. Indeed, it only needs to happen once per bus. Clear the 'scrub' option after one namespace_clear_bb() invocation, and reset it when looping to the next bus. Link: https://github.com/pmem/ndctl/issues/104 Reported-by: Erwin Tsaur <erwin.tsaur@oracle.com> Fixes: 2ba7b8277075 ("ndctl: add a 'clear-errors' command") Signed-off-by: Dan Williams <dan.j.williams@intel.com> --- ndctl/namespace.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-)