| Message ID | 20190530125756.10281-1-colin.king@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [SRU,BIONIC] UBUNTU: SAUCE: (noup) Update zfs to 0.7.5-1ubuntu16.6 | expand |
Fix is small, is upstream, has been on cosmic and disco, and test has shown
good results.
Acked-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
On 5/30/19 5:57 AM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > https://bugs.launchpad.net/bugs/1772412 > > Sync from zfs 0.7.5-1ubuntu16.6 that contains a backport of upstream zfs > commit 77d8a0f1a4d0b2f59cee63088f7987cb38e66538 ("Fix hung z_zvol tasks > during 'zfs receive'"). Should this still be prefixed with "UBUNTU: SAUCE: (noup)" if this fix is in upstream ZFS? I'm not sure if this is a side effect of how this package is synced, but I think we should also carry provenance from here: https://github.com/zfsonlinux/zfs/commit/77d8a0f1a4d0b2f59cee63088f7987cb38e66538 > > Adds a dedicated, per-pool, prefetch taskq to prevent the traverse > code from monopolizing the global (and limited) system_taskq by > inappropriately scheduling long running tasks on it. This fixes > z_zvol hung tasks when performing large I/O operations, for example > when performing huge ZFS send/receives (on slow media). > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > zfs/META | 2 +- > zfs/include/sys/spa_impl.h | 1 + > zfs/module/zfs/dmu_traverse.c | 3 ++- > zfs/module/zfs/spa.c | 13 +++++++++++++ > 4 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/zfs/META b/zfs/META > index 9864f8f..0823859 100644 > --- a/zfs/META > +++ b/zfs/META > @@ -2,7 +2,7 @@ Meta: 1 > Name: zfs > Branch: 1.0 > Version: 0.7.5 > -Release: 1ubuntu16.5 > +Release: 1ubuntu16.6 > Release-Tags: relext > License: CDDL > Author: OpenZFS on Linux > diff --git a/zfs/include/sys/spa_impl.h b/zfs/include/sys/spa_impl.h > index 06de244..34b1199 100644 > --- a/zfs/include/sys/spa_impl.h > +++ b/zfs/include/sys/spa_impl.h > @@ -275,6 +275,7 @@ struct spa { > spa_stats_t spa_stats; /* assorted spa statistics */ > hrtime_t spa_ccw_fail_time; /* Conf cache write fail time */ > taskq_t *spa_zvol_taskq; /* Taskq for minor management */ > + taskq_t *spa_prefetch_taskq; /* Taskq for prefetch threads */ > uint64_t spa_multihost; /* multihost aware (mmp) */ > mmp_thread_t spa_mmp; /* multihost mmp thread */ > > diff --git a/zfs/module/zfs/dmu_traverse.c b/zfs/module/zfs/dmu_traverse.c > index 62f770e..c9171ee 100644 > --- a/zfs/module/zfs/dmu_traverse.c > +++ b/zfs/module/zfs/dmu_traverse.c > @@ -31,6 +31,7 @@ > #include <sys/dsl_pool.h> > #include <sys/dnode.h> > #include <sys/spa.h> > +#include <sys/spa_impl.h> > #include <sys/zio.h> > #include <sys/dmu_impl.h> > #include <sys/sa.h> > @@ -626,7 +627,7 @@ traverse_impl(spa_t *spa, dsl_dataset_t *ds, uint64_t objset, blkptr_t *rootbp, > } > > if (!(flags & TRAVERSE_PREFETCH_DATA) || > - taskq_dispatch(system_taskq, traverse_prefetch_thread, > + taskq_dispatch(spa->spa_prefetch_taskq, traverse_prefetch_thread, > td, TQ_NOQUEUE) == TASKQID_INVALID) > pd->pd_exited = B_TRUE; > > diff --git a/zfs/module/zfs/spa.c b/zfs/module/zfs/spa.c > index 00587d8..b777afc 100644 > --- a/zfs/module/zfs/spa.c > +++ b/zfs/module/zfs/spa.c > @@ -1183,6 +1183,14 @@ spa_activate(spa_t *spa, int mode) > 1, INT_MAX, 0); > > /* > + * Taskq dedicated to prefetcher threads: this is used to prevent the > + * pool traverse code from monopolizing the global (and limited) > + * system_taskq by inappropriately scheduling long running tasks on it. > + */ > + spa->spa_prefetch_taskq = taskq_create("z_prefetch", boot_ncpus, > + defclsyspri, 1, INT_MAX, TASKQ_DYNAMIC); > + > + /* > * The taskq to upgrade datasets in this pool. Currently used by > * feature SPA_FEATURE_USEROBJ_ACCOUNTING. > */ > @@ -1211,6 +1219,11 @@ spa_deactivate(spa_t *spa) > spa->spa_zvol_taskq = NULL; > } > > + if (spa->spa_prefetch_taskq) { > + taskq_destroy(spa->spa_prefetch_taskq); > + spa->spa_prefetch_taskq = NULL; > + } > + > if (spa->spa_upgrade_taskq) { > taskq_destroy(spa->spa_upgrade_taskq); > spa->spa_upgrade_taskq = NULL; >
On 04/06/2019 16:45, Connor Kuehl wrote: > On 5/30/19 5:57 AM, Colin King wrote: >> From: Colin Ian King <colin.king@canonical.com> >> >> https://bugs.launchpad.net/bugs/1772412 >> >> Sync from zfs 0.7.5-1ubuntu16.6 that contains a backport of upstream zfs >> commit 77d8a0f1a4d0b2f59cee63088f7987cb38e66538 ("Fix hung z_zvol tasks >> during 'zfs receive'"). > > Should this still be prefixed with "UBUNTU: SAUCE: (noup)" if this fix > is in upstream ZFS? These special ZFS/SPL patches are auto-generated from a script that grabs the dkms source package and generates a delta from the current code and the new updated dkms source. We've always accepted these patches in this format in the past. > > I'm not sure if this is a side effect of how this package is synced, but > I think we should also carry provenance from here: > https://github.com/zfsonlinux/zfs/commit/77d8a0f1a4d0b2f59cee63088f7987cb38e66538 > >> >> Adds a dedicated, per-pool, prefetch taskq to prevent the traverse >> code from monopolizing the global (and limited) system_taskq by >> inappropriately scheduling long running tasks on it. This fixes >> z_zvol hung tasks when performing large I/O operations, for example >> when performing huge ZFS send/receives (on slow media). >> >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> zfs/META | 2 +- >> zfs/include/sys/spa_impl.h | 1 + >> zfs/module/zfs/dmu_traverse.c | 3 ++- >> zfs/module/zfs/spa.c | 13 +++++++++++++ >> 4 files changed, 17 insertions(+), 2 deletions(-) >> >> diff --git a/zfs/META b/zfs/META >> index 9864f8f..0823859 100644 >> --- a/zfs/META >> +++ b/zfs/META >> @@ -2,7 +2,7 @@ Meta: 1 >> Name: zfs >> Branch: 1.0 >> Version: 0.7.5 >> -Release: 1ubuntu16.5 >> +Release: 1ubuntu16.6 >> Release-Tags: relext >> License: CDDL >> Author: OpenZFS on Linux >> diff --git a/zfs/include/sys/spa_impl.h b/zfs/include/sys/spa_impl.h >> index 06de244..34b1199 100644 >> --- a/zfs/include/sys/spa_impl.h >> +++ b/zfs/include/sys/spa_impl.h >> @@ -275,6 +275,7 @@ struct spa { >> spa_stats_t spa_stats; /* assorted spa statistics */ >> hrtime_t spa_ccw_fail_time; /* Conf cache write fail time */ >> taskq_t *spa_zvol_taskq; /* Taskq for minor management */ >> + taskq_t *spa_prefetch_taskq; /* Taskq for prefetch threads */ >> uint64_t spa_multihost; /* multihost aware (mmp) */ >> mmp_thread_t spa_mmp; /* multihost mmp thread */ >> >> diff --git a/zfs/module/zfs/dmu_traverse.c b/zfs/module/zfs/dmu_traverse.c >> index 62f770e..c9171ee 100644 >> --- a/zfs/module/zfs/dmu_traverse.c >> +++ b/zfs/module/zfs/dmu_traverse.c >> @@ -31,6 +31,7 @@ >> #include <sys/dsl_pool.h> >> #include <sys/dnode.h> >> #include <sys/spa.h> >> +#include <sys/spa_impl.h> >> #include <sys/zio.h> >> #include <sys/dmu_impl.h> >> #include <sys/sa.h> >> @@ -626,7 +627,7 @@ traverse_impl(spa_t *spa, dsl_dataset_t *ds, uint64_t objset, blkptr_t *rootbp, >> } >> >> if (!(flags & TRAVERSE_PREFETCH_DATA) || >> - taskq_dispatch(system_taskq, traverse_prefetch_thread, >> + taskq_dispatch(spa->spa_prefetch_taskq, traverse_prefetch_thread, >> td, TQ_NOQUEUE) == TASKQID_INVALID) >> pd->pd_exited = B_TRUE; >> >> diff --git a/zfs/module/zfs/spa.c b/zfs/module/zfs/spa.c >> index 00587d8..b777afc 100644 >> --- a/zfs/module/zfs/spa.c >> +++ b/zfs/module/zfs/spa.c >> @@ -1183,6 +1183,14 @@ spa_activate(spa_t *spa, int mode) >> 1, INT_MAX, 0); >> >> /* >> + * Taskq dedicated to prefetcher threads: this is used to prevent the >> + * pool traverse code from monopolizing the global (and limited) >> + * system_taskq by inappropriately scheduling long running tasks on it. >> + */ >> + spa->spa_prefetch_taskq = taskq_create("z_prefetch", boot_ncpus, >> + defclsyspri, 1, INT_MAX, TASKQ_DYNAMIC); >> + >> + /* >> * The taskq to upgrade datasets in this pool. Currently used by >> * feature SPA_FEATURE_USEROBJ_ACCOUNTING. >> */ >> @@ -1211,6 +1219,11 @@ spa_deactivate(spa_t *spa) >> spa->spa_zvol_taskq = NULL; >> } >> >> + if (spa->spa_prefetch_taskq) { >> + taskq_destroy(spa->spa_prefetch_taskq); >> + spa->spa_prefetch_taskq = NULL; >> + } >> + >> if (spa->spa_upgrade_taskq) { >> taskq_destroy(spa->spa_upgrade_taskq); >> spa->spa_upgrade_taskq = NULL; >> > >
On Thu, May 30, 2019 at 01:57:56PM +0100, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > https://bugs.launchpad.net/bugs/1772412 > > Sync from zfs 0.7.5-1ubuntu16.6 that contains a backport of upstream zfs > commit 77d8a0f1a4d0b2f59cee63088f7987cb38e66538 ("Fix hung z_zvol tasks > during 'zfs receive'"). > > Adds a dedicated, per-pool, prefetch taskq to prevent the traverse > code from monopolizing the global (and limited) system_taskq by > inappropriately scheduling long running tasks on it. This fixes > z_zvol hung tasks when performing large I/O operations, for example > when performing huge ZFS send/receives (on slow media). > > Signed-off-by: Colin Ian King <colin.king@canonical.com> It's a small fix, clean upstream cherry-pick (minor offset changes), all good to me. Acked-by: Andrea Righi <andrea.righi@canonical.com>
On 5/30/19 5:57 AM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > https://bugs.launchpad.net/bugs/1772412 > > Sync from zfs 0.7.5-1ubuntu16.6 that contains a backport of upstream zfs > commit 77d8a0f1a4d0b2f59cee63088f7987cb38e66538 ("Fix hung z_zvol tasks > during 'zfs receive'"). > > Adds a dedicated, per-pool, prefetch taskq to prevent the traverse > code from monopolizing the global (and limited) system_taskq by > inappropriately scheduling long running tasks on it. This fixes > z_zvol hung tasks when performing large I/O operations, for example > when performing huge ZFS send/receives (on slow media). > > Signed-off-by: Colin Ian King <colin.king@canonical.com> Acked-by: Connor Kuehl <connor.kuehl@canonical.com> > --- > zfs/META | 2 +- > zfs/include/sys/spa_impl.h | 1 + > zfs/module/zfs/dmu_traverse.c | 3 ++- > zfs/module/zfs/spa.c | 13 +++++++++++++ > 4 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/zfs/META b/zfs/META > index 9864f8f..0823859 100644 > --- a/zfs/META > +++ b/zfs/META > @@ -2,7 +2,7 @@ Meta: 1 > Name: zfs > Branch: 1.0 > Version: 0.7.5 > -Release: 1ubuntu16.5 > +Release: 1ubuntu16.6 > Release-Tags: relext > License: CDDL > Author: OpenZFS on Linux > diff --git a/zfs/include/sys/spa_impl.h b/zfs/include/sys/spa_impl.h > index 06de244..34b1199 100644 > --- a/zfs/include/sys/spa_impl.h > +++ b/zfs/include/sys/spa_impl.h > @@ -275,6 +275,7 @@ struct spa { > spa_stats_t spa_stats; /* assorted spa statistics */ > hrtime_t spa_ccw_fail_time; /* Conf cache write fail time */ > taskq_t *spa_zvol_taskq; /* Taskq for minor management */ > + taskq_t *spa_prefetch_taskq; /* Taskq for prefetch threads */ > uint64_t spa_multihost; /* multihost aware (mmp) */ > mmp_thread_t spa_mmp; /* multihost mmp thread */ > > diff --git a/zfs/module/zfs/dmu_traverse.c b/zfs/module/zfs/dmu_traverse.c > index 62f770e..c9171ee 100644 > --- a/zfs/module/zfs/dmu_traverse.c > +++ b/zfs/module/zfs/dmu_traverse.c > @@ -31,6 +31,7 @@ > #include <sys/dsl_pool.h> > #include <sys/dnode.h> > #include <sys/spa.h> > +#include <sys/spa_impl.h> > #include <sys/zio.h> > #include <sys/dmu_impl.h> > #include <sys/sa.h> > @@ -626,7 +627,7 @@ traverse_impl(spa_t *spa, dsl_dataset_t *ds, uint64_t objset, blkptr_t *rootbp, > } > > if (!(flags & TRAVERSE_PREFETCH_DATA) || > - taskq_dispatch(system_taskq, traverse_prefetch_thread, > + taskq_dispatch(spa->spa_prefetch_taskq, traverse_prefetch_thread, > td, TQ_NOQUEUE) == TASKQID_INVALID) > pd->pd_exited = B_TRUE; > > diff --git a/zfs/module/zfs/spa.c b/zfs/module/zfs/spa.c > index 00587d8..b777afc 100644 > --- a/zfs/module/zfs/spa.c > +++ b/zfs/module/zfs/spa.c > @@ -1183,6 +1183,14 @@ spa_activate(spa_t *spa, int mode) > 1, INT_MAX, 0); > > /* > + * Taskq dedicated to prefetcher threads: this is used to prevent the > + * pool traverse code from monopolizing the global (and limited) > + * system_taskq by inappropriately scheduling long running tasks on it. > + */ > + spa->spa_prefetch_taskq = taskq_create("z_prefetch", boot_ncpus, > + defclsyspri, 1, INT_MAX, TASKQ_DYNAMIC); > + > + /* > * The taskq to upgrade datasets in this pool. Currently used by > * feature SPA_FEATURE_USEROBJ_ACCOUNTING. > */ > @@ -1211,6 +1219,11 @@ spa_deactivate(spa_t *spa) > spa->spa_zvol_taskq = NULL; > } > > + if (spa->spa_prefetch_taskq) { > + taskq_destroy(spa->spa_prefetch_taskq); > + spa->spa_prefetch_taskq = NULL; > + } > + > if (spa->spa_upgrade_taskq) { > taskq_destroy(spa->spa_upgrade_taskq); > spa->spa_upgrade_taskq = NULL; >
On 5/30/19 2:57 PM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > https://bugs.launchpad.net/bugs/1772412 > > Sync from zfs 0.7.5-1ubuntu16.6 that contains a backport of upstream zfs > commit 77d8a0f1a4d0b2f59cee63088f7987cb38e66538 ("Fix hung z_zvol tasks > during 'zfs receive'"). > > Adds a dedicated, per-pool, prefetch taskq to prevent the traverse > code from monopolizing the global (and limited) system_taskq by > inappropriately scheduling long running tasks on it. This fixes > z_zvol hung tasks when performing large I/O operations, for example > when performing huge ZFS send/receives (on slow media). > > Signed-off-by: Colin Ian King <colin.king@canonical.com> Applied to bionic/master-next branch. Thanks, Kleber > --- > zfs/META | 2 +- > zfs/include/sys/spa_impl.h | 1 + > zfs/module/zfs/dmu_traverse.c | 3 ++- > zfs/module/zfs/spa.c | 13 +++++++++++++ > 4 files changed, 17 insertions(+), 2 deletions(-) > > diff --git a/zfs/META b/zfs/META > index 9864f8f..0823859 100644 > --- a/zfs/META > +++ b/zfs/META > @@ -2,7 +2,7 @@ Meta: 1 > Name: zfs > Branch: 1.0 > Version: 0.7.5 > -Release: 1ubuntu16.5 > +Release: 1ubuntu16.6 > Release-Tags: relext > License: CDDL > Author: OpenZFS on Linux > diff --git a/zfs/include/sys/spa_impl.h b/zfs/include/sys/spa_impl.h > index 06de244..34b1199 100644 > --- a/zfs/include/sys/spa_impl.h > +++ b/zfs/include/sys/spa_impl.h > @@ -275,6 +275,7 @@ struct spa { > spa_stats_t spa_stats; /* assorted spa statistics */ > hrtime_t spa_ccw_fail_time; /* Conf cache write fail time */ > taskq_t *spa_zvol_taskq; /* Taskq for minor management */ > + taskq_t *spa_prefetch_taskq; /* Taskq for prefetch threads */ > uint64_t spa_multihost; /* multihost aware (mmp) */ > mmp_thread_t spa_mmp; /* multihost mmp thread */ > > diff --git a/zfs/module/zfs/dmu_traverse.c b/zfs/module/zfs/dmu_traverse.c > index 62f770e..c9171ee 100644 > --- a/zfs/module/zfs/dmu_traverse.c > +++ b/zfs/module/zfs/dmu_traverse.c > @@ -31,6 +31,7 @@ > #include <sys/dsl_pool.h> > #include <sys/dnode.h> > #include <sys/spa.h> > +#include <sys/spa_impl.h> > #include <sys/zio.h> > #include <sys/dmu_impl.h> > #include <sys/sa.h> > @@ -626,7 +627,7 @@ traverse_impl(spa_t *spa, dsl_dataset_t *ds, uint64_t objset, blkptr_t *rootbp, > } > > if (!(flags & TRAVERSE_PREFETCH_DATA) || > - taskq_dispatch(system_taskq, traverse_prefetch_thread, > + taskq_dispatch(spa->spa_prefetch_taskq, traverse_prefetch_thread, > td, TQ_NOQUEUE) == TASKQID_INVALID) > pd->pd_exited = B_TRUE; > > diff --git a/zfs/module/zfs/spa.c b/zfs/module/zfs/spa.c > index 00587d8..b777afc 100644 > --- a/zfs/module/zfs/spa.c > +++ b/zfs/module/zfs/spa.c > @@ -1183,6 +1183,14 @@ spa_activate(spa_t *spa, int mode) > 1, INT_MAX, 0); > > /* > + * Taskq dedicated to prefetcher threads: this is used to prevent the > + * pool traverse code from monopolizing the global (and limited) > + * system_taskq by inappropriately scheduling long running tasks on it. > + */ > + spa->spa_prefetch_taskq = taskq_create("z_prefetch", boot_ncpus, > + defclsyspri, 1, INT_MAX, TASKQ_DYNAMIC); > + > + /* > * The taskq to upgrade datasets in this pool. Currently used by > * feature SPA_FEATURE_USEROBJ_ACCOUNTING. > */ > @@ -1211,6 +1219,11 @@ spa_deactivate(spa_t *spa) > spa->spa_zvol_taskq = NULL; > } > > + if (spa->spa_prefetch_taskq) { > + taskq_destroy(spa->spa_prefetch_taskq); > + spa->spa_prefetch_taskq = NULL; > + } > + > if (spa->spa_upgrade_taskq) { > taskq_destroy(spa->spa_upgrade_taskq); > spa->spa_upgrade_taskq = NULL; >
diff --git a/zfs/META b/zfs/META index 9864f8f..0823859 100644 --- a/zfs/META +++ b/zfs/META @@ -2,7 +2,7 @@ Meta: 1 Name: zfs Branch: 1.0 Version: 0.7.5 -Release: 1ubuntu16.5 +Release: 1ubuntu16.6 Release-Tags: relext License: CDDL Author: OpenZFS on Linux diff --git a/zfs/include/sys/spa_impl.h b/zfs/include/sys/spa_impl.h index 06de244..34b1199 100644 --- a/zfs/include/sys/spa_impl.h +++ b/zfs/include/sys/spa_impl.h @@ -275,6 +275,7 @@ struct spa { spa_stats_t spa_stats; /* assorted spa statistics */ hrtime_t spa_ccw_fail_time; /* Conf cache write fail time */ taskq_t *spa_zvol_taskq; /* Taskq for minor management */ + taskq_t *spa_prefetch_taskq; /* Taskq for prefetch threads */ uint64_t spa_multihost; /* multihost aware (mmp) */ mmp_thread_t spa_mmp; /* multihost mmp thread */ diff --git a/zfs/module/zfs/dmu_traverse.c b/zfs/module/zfs/dmu_traverse.c index 62f770e..c9171ee 100644 --- a/zfs/module/zfs/dmu_traverse.c +++ b/zfs/module/zfs/dmu_traverse.c @@ -31,6 +31,7 @@ #include <sys/dsl_pool.h> #include <sys/dnode.h> #include <sys/spa.h> +#include <sys/spa_impl.h> #include <sys/zio.h> #include <sys/dmu_impl.h> #include <sys/sa.h> @@ -626,7 +627,7 @@ traverse_impl(spa_t *spa, dsl_dataset_t *ds, uint64_t objset, blkptr_t *rootbp, } if (!(flags & TRAVERSE_PREFETCH_DATA) || - taskq_dispatch(system_taskq, traverse_prefetch_thread, + taskq_dispatch(spa->spa_prefetch_taskq, traverse_prefetch_thread, td, TQ_NOQUEUE) == TASKQID_INVALID) pd->pd_exited = B_TRUE; diff --git a/zfs/module/zfs/spa.c b/zfs/module/zfs/spa.c index 00587d8..b777afc 100644 --- a/zfs/module/zfs/spa.c +++ b/zfs/module/zfs/spa.c @@ -1183,6 +1183,14 @@ spa_activate(spa_t *spa, int mode) 1, INT_MAX, 0); /* + * Taskq dedicated to prefetcher threads: this is used to prevent the + * pool traverse code from monopolizing the global (and limited) + * system_taskq by inappropriately scheduling long running tasks on it. + */ + spa->spa_prefetch_taskq = taskq_create("z_prefetch", boot_ncpus, + defclsyspri, 1, INT_MAX, TASKQ_DYNAMIC); + + /* * The taskq to upgrade datasets in this pool. Currently used by * feature SPA_FEATURE_USEROBJ_ACCOUNTING. */ @@ -1211,6 +1219,11 @@ spa_deactivate(spa_t *spa) spa->spa_zvol_taskq = NULL; } + if (spa->spa_prefetch_taskq) { + taskq_destroy(spa->spa_prefetch_taskq); + spa->spa_prefetch_taskq = NULL; + } + if (spa->spa_upgrade_taskq) { taskq_destroy(spa->spa_upgrade_taskq); spa->spa_upgrade_taskq = NULL;