Project

General

Profile

Bug #1356

zfs dataset prefetch code not working

Added by Martin Matuška about 8 years ago. Updated almost 8 years ago.

Status:
Resolved
Priority:
Normal
Category:
zfs - Zettabyte File System
Start date:
2011-08-10
Due date:
% Done:

0%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage

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

zfs_prefetch.patch (1.45 KB) zfs_prefetch.patch Martin Matuška, 2011-08-10 06:31 PM
zfs_prefetch.patch (2.15 KB) zfs_prefetch.patch Martin Matuška, 2011-08-11 12:17 PM
zfs_ioctl.c.patch (619 Bytes) zfs_ioctl.c.patch Martin Matuška, 2011-08-18 01:04 PM

History

#1

Updated by Martin Matuška about 8 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.

#2

Updated by Martin Matuška about 8 years ago

I am attaching a complete version of the patch.

#3

Updated by Martin Matuška about 8 years ago

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 {
#4

Updated by Martin Matuška almost 8 years ago

To explain the link with this issue and issue #1346:
After you have fixed the prefetch code, the bug #1346 becomes triggerable.
The prefetch code alone is fixed just by changing:

(void) dmu_objset_prefetch(p, NULL);
to
(void) dmu_objset_prefetch(zc->zc_name, NULL);

#5

Updated by Gordon Ross almost 8 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>

Also available in: Atom PDF