Bug #1356
closedzfs dataset prefetch code not working
0%
Description
The prefetch code in zfs_ioctl.c is not working correctly.
dmu_objset_prefetch() takes absolute and not relative names.
--- a/usr/src/uts/common/fs/zfs/zfs_ioctl.c Fri Jul 22 09:27:57 2011 -0700 +++ b/usr/src/uts/common/fs/zfs/zfs_ioctl.c Wed Aug 10 20:22:50 2011 +0200 @@ -1944,7 +1944,7 @@ zfs_ioc_dataset_list_next() int len = sizeof (zc->zc_name) - (p - zc->zc_name); while (dmu_dir_list_next(os, len, p, NULL, &cookie) == 0) - (void) dmu_objset_prefetch(p, NULL); + (void) dmu_objset_prefetch(zc->zc_name, NULL); } do {
When the prefetch is fixed, it can race with dsl_dir_destroy_check() (Issue #1346). This demonstrates easily upon receiving incremental snapshots using temporary clones (zfs send -i/-I + zfs receive -F). A fix similiar to dmu_objset_snapshot_one() is required:
--- a/usr/src/uts/common/fs/zfs/dmu_objset.c Fri Jul 22 09:27:57 2011 -0700 +++ b/usr/src/uts/common/fs/zfs/dmu_objset.c Wed Aug 10 20:22:50 2011 +0200 @@ -1748,10 +1748,29 @@ int dmu_objset_prefetch(const char *name, void *arg) { dsl_dataset_t *ds; + char *cp; + + /* + * If the objset starts with a '%', then ignore it. + * These hidden datasets are always inconsistent and by not opening + * them here, we can avoid a race with dsl_dir_destroy_check(). + */ + cp = strrchr(name, '/'); + if (cp && cp[1] == '%') + return (0); if (dsl_dataset_hold(name, FTAG, &ds)) return (0); + /* + * If the objset is in an inconsistent state (eg, in the process + * of being destroyed), don't prefetch it. + */ + if (ds->ds_phys->ds_flags & DS_FLAG_INCONSISTENT) { + dsl_dataset_rele(ds, FTAG); + return (0); + } + if (!BP_IS_HOLE(&ds->ds_phys->ds_bp)) { mutex_enter(&ds->ds_opening_lock); if (ds->ds_objset == NULL) {
Files
Updated by Martin Matuška about 11 years ago
The question also raises if it makes sense to prefetch internal datasets.
The only internal dataset that prefetches successfully is $ORIGIN, the other ones - $MOS and $FREE fail on dmu_objset_hold() and are not prefetched. The function dmu_objset_find() that is the second function used with dmu_objset_prefetch() and it intentionally skips these datasets so they are not prefetched.
And in general, in zfs_ioc_dataset_list_next() we should prefetch only what we are going to process later.
Updated by Martin Matuška about 11 years ago
- File zfs_prefetch.patch zfs_prefetch.patch added
I am attaching a complete version of the patch.
Updated by Martin Matuška almost 11 years ago
- File zfs_ioctl.c.patch zfs_ioctl.c.patch added
We have finally used just this patch in FreeBSD to fix this whole issue:
diff -r a4e1558c0599 usr/src/uts/common/fs/zfs/zfs_ioctl.c --- a/usr/src/uts/common/fs/zfs/zfs_ioctl.c Fri Jul 22 09:27:57 2011 -0700 +++ b/usr/src/uts/common/fs/zfs/zfs_ioctl.c Thu Aug 18 15:01:46 2011 +0200 @@ -1943,8 +1943,10 @@ zfs_ioc_dataset_list_next() uint64_t cookie = 0; int len = sizeof (zc->zc_name) - (p - zc->zc_name); - while (dmu_dir_list_next(os, len, p, NULL, &cookie) == 0) - (void) dmu_objset_prefetch(p, NULL); + while (dmu_dir_list_next(os, len, p, NULL, &cookie) == 0) { + if (!dataset_name_hidden(zc->zc_name)) + (void) dmu_objset_prefetch(zc->zc_name, NULL); + } } do {
Updated by Martin Matuška almost 11 years ago
Updated by Gordon Ross almost 11 years ago
- Status changed from New to Resolved
- Assignee set to Martin Matuška
changeset: 13492:83d135508f56 user: Martin Matuska <mm@FreeBSD.org> date: Fri Oct 21 11:49:36 2011 -0400 description: 1346 zfs incremental receive may leave behind temporary clones 1356 zfs dataset prefetch code not working Reviewed by: Matthew Ahrens <matt@delphix.com> Reviewed by: Dan McDonald <danmcd@nexenta.com> Approved by: Gordon Ross <gwr@nexenta.com>