Re: svn commit: r300881 - in head/sys: cddl/contrib/opensolaris/uts/common/fs/zfs geom

2016-06-20 Thread Kristof Provost


On 20 Jun 2016, at 17:34, Allan Jude wrote:
> Looking at the backtrace, do you have one or more ZVOLs?
>
No, there are no zvols:

% zfs list -t volume
no datasets available

Regards,
Kristof
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"


Re: svn commit: r300881 - in head/sys: cddl/contrib/opensolaris/uts/common/fs/zfs geom

2016-06-20 Thread Allan Jude
On 2016-06-20 11:31, Kristof Provost wrote:
> No, it’s an HP Microserver. 4 data disks and that’s it.
> 
> Bug: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=210409
> 
> Regards,
> Kristof
> 
> On 20 Jun 2016, at 17:27, Alan Somers wrote:
> 
> You say it's a 4-disk RAIDZ1. Anything topologically weird, like a
> log, cache or spare device? SAS or SATA? Any SAS expanders? Please
> open a bug for this and assign to me so we can be sure to get this
> fixed in time for 11.0.
> 
> -Alan
> 
> On Mon, Jun 20, 2016 at 8:59 AM, Kristof Provost k...@freebsd.org
>  wrote:
> 
> Hi,
> 
> It looks like this change breaks boot on my machine.
> I’m running a root-on-ZFS system and reliably see this panic
> during boot.
> It’s a 4 disk raidz-1.
> 
> It’s now running r302028 with r300881 backed out, and booting fine.
> 
> The panic:
> panic: solaris assert: refcount(count(&spa->spa_refcount) >=
> spa->spa_minref
> ||
> MUTEX_HELD(&spa_namespace_lock), file:
> /usr/src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa_misc.c,
> line:
> 863
> 
> Unfortunately I can’t get a dump, but here’s a picture of the
> backtrace:
> https://people.freebsd.org/~kp/zfs_panic.jpg
> 
> 
> Regards,
> Kristof
> 
> 

Looking at the backtrace, do you have one or more ZVOLs?

-- 
Allan Jude
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r300881 - in head/sys: cddl/contrib/opensolaris/uts/common/fs/zfs geom

2016-06-20 Thread Kristof Provost

No, it’s an HP Microserver. 4 data disks and that’s it.

Bug: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=210409

Regards,
Kristof

On 20 Jun 2016, at 17:27, Alan Somers wrote:


You say it's a 4-disk RAIDZ1.  Anything topologically weird, like a
log, cache or spare device?  SAS or SATA?  Any SAS expanders?  Please
open a bug for this and assign to me so we can be sure to get this
fixed in time for 11.0.

-Alan

On Mon, Jun 20, 2016 at 8:59 AM, Kristof Provost  
wrote:

Hi,

It looks like this change breaks boot on my machine.
I’m running a root-on-ZFS system and reliably see this panic during 
boot.

It’s a 4 disk raidz-1.

It’s now running r302028 with r300881 backed out, and booting fine.

The panic:
panic: solaris assert: refcount(count(&spa->spa_refcount) >= 
spa->spa_minref

||
MUTEX_HELD(&spa_namespace_lock), file:
/usr/src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa_misc.c, 
line:

863

Unfortunately I can’t get a dump, but here’s a picture of the 
backtrace:

https://people.freebsd.org/~kp/zfs_panic.jpg

Regards,
Kristof

On 28 May 2016, at 0:32, Alan Somers wrote:

Author: asomers
Date: Fri May 27 22:32:44 2016
New Revision: 300881
URL: https://svnweb.freebsd.org/changeset/base/300881

Log:
Avoid issuing spa config updates for physical path when not necessary

ZFS's configuration needs to be updated whenever the physical path 
for a
device changes, but not when a new device is introduced. This is 
because new
devices necessarily cause config updates, but only if they are 
actually

accepted into the pool.

sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
Split vdev_geom_set_physpath out of vdev_geom_attrchanged. When
setting the vdev's physical path, only request a config update if
the physical path has changed. Don't request it when opening a
device for the first time, because the config sync will happen
anyway upstack.

sys/geom/geom_dev.c
Split g_dev_set_physpath and g_dev_set_media out of
g_dev_attrchanged

Submitted by: will, asomers
MFC after: 4 weeks
Sponsored by: Spectra Logic Corp
Differential Revision: https://reviews.freebsd.org/D6428

Modified:
head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
head/sys/geom/geom_dev.c

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r300881 - in head/sys: cddl/contrib/opensolaris/uts/common/fs/zfs geom

2016-06-20 Thread Alan Somers
You say it's a 4-disk RAIDZ1.  Anything topologically weird, like a
log, cache or spare device?  SAS or SATA?  Any SAS expanders?  Please
open a bug for this and assign to me so we can be sure to get this
fixed in time for 11.0.

-Alan

On Mon, Jun 20, 2016 at 8:59 AM, Kristof Provost  wrote:
> Hi,
>
> It looks like this change breaks boot on my machine.
> I’m running a root-on-ZFS system and reliably see this panic during boot.
> It’s a 4 disk raidz-1.
>
> It’s now running r302028 with r300881 backed out, and booting fine.
>
> The panic:
> panic: solaris assert: refcount(count(&spa->spa_refcount) >= spa->spa_minref
> ||
> MUTEX_HELD(&spa_namespace_lock), file:
> /usr/src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa_misc.c, line:
> 863
>
> Unfortunately I can’t get a dump, but here’s a picture of the backtrace:
> https://people.freebsd.org/~kp/zfs_panic.jpg
>
> Regards,
> Kristof
>
> On 28 May 2016, at 0:32, Alan Somers wrote:
>
> Author: asomers
> Date: Fri May 27 22:32:44 2016
> New Revision: 300881
> URL: https://svnweb.freebsd.org/changeset/base/300881
>
> Log:
> Avoid issuing spa config updates for physical path when not necessary
>
> ZFS's configuration needs to be updated whenever the physical path for a
> device changes, but not when a new device is introduced. This is because new
> devices necessarily cause config updates, but only if they are actually
> accepted into the pool.
>
> sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
> Split vdev_geom_set_physpath out of vdev_geom_attrchanged. When
> setting the vdev's physical path, only request a config update if
> the physical path has changed. Don't request it when opening a
> device for the first time, because the config sync will happen
> anyway upstack.
>
> sys/geom/geom_dev.c
> Split g_dev_set_physpath and g_dev_set_media out of
> g_dev_attrchanged
>
> Submitted by: will, asomers
> MFC after: 4 weeks
> Sponsored by: Spectra Logic Corp
> Differential Revision: https://reviews.freebsd.org/D6428
>
> Modified:
> head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
> head/sys/geom/geom_dev.c
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r300881 - in head/sys: cddl/contrib/opensolaris/uts/common/fs/zfs geom

2016-06-20 Thread Kristof Provost

Hi,

It looks like this change breaks boot on my machine.
I’m running a root-on-ZFS system and reliably see this panic during 
boot. It’s a 4 disk raidz-1.


It’s now running r302028 with r300881 backed out, and booting fine.

The panic:
panic: solaris assert: refcount(count(&spa->spa_refcount) >= 
spa->spa_minref ||
MUTEX_HELD(&spa_namespace_lock), file: 
/usr/src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/spa_misc.c, 
line: 863


Unfortunately I can’t get a dump, but here’s a picture of the 
backtrace:

https://people.freebsd.org/~kp/zfs_panic.jpg


Regards,
Kristof

On 28 May 2016, at 0:32, Alan Somers wrote:


Author: asomers
Date: Fri May 27 22:32:44 2016
New Revision: 300881
URL: https://svnweb.freebsd.org/changeset/base/300881

Log:
  Avoid issuing spa config updates for physical path when not 
necessary


  ZFS's configuration needs to be updated whenever the physical path 
for a
  device changes, but not when a new device is introduced. This is 
because new
  devices necessarily cause config updates, but only if they are 
actually

  accepted into the pool.

  sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
Split vdev_geom_set_physpath out of vdev_geom_attrchanged.  When
setting the vdev's physical path, only request a config update if
the physical path has changed.  Don't request it when opening a
device for the first time, because the config sync will happen
anyway upstack.

  sys/geom/geom_dev.c
Split g_dev_set_physpath and g_dev_set_media out of
g_dev_attrchanged

  Submitted by: will, asomers
  MFC after:4 weeks
  Sponsored by: Spectra Logic Corp
  Differential Revision:https://reviews.freebsd.org/D6428

Modified:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
  head/sys/geom/geom_dev.c

___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r300881 - in head/sys: cddl/contrib/opensolaris/uts/common/fs/zfs geom

2016-05-27 Thread Alan Somers
Author: asomers
Date: Fri May 27 22:32:44 2016
New Revision: 300881
URL: https://svnweb.freebsd.org/changeset/base/300881

Log:
  Avoid issuing spa config updates for physical path when not necessary
  
  ZFS's configuration needs to be updated whenever the physical path for a
  device changes, but not when a new device is introduced. This is because new
  devices necessarily cause config updates, but only if they are actually
  accepted into the pool.
  
  sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
Split vdev_geom_set_physpath out of vdev_geom_attrchanged.  When
setting the vdev's physical path, only request a config update if
the physical path has changed.  Don't request it when opening a
device for the first time, because the config sync will happen
anyway upstack.
  
  sys/geom/geom_dev.c
Split g_dev_set_physpath and g_dev_set_media out of
g_dev_attrchanged
  
  Submitted by: will, asomers
  MFC after:4 weeks
  Sponsored by: Spectra Logic Corp
  Differential Revision:https://reviews.freebsd.org/D6428

Modified:
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
  head/sys/geom/geom_dev.c

Modified: head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c
==
--- head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c Fri May 
27 22:26:43 2016(r300880)
+++ head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/vdev_geom.c Fri May 
27 22:32:44 2016(r300881)
@@ -85,32 +85,17 @@ vdev_geom_set_rotation_rate(vdev_t *vd, 
 }
 
 static void
-vdev_geom_attrchanged(struct g_consumer *cp, const char *attr)
+vdev_geom_set_physpath(struct g_consumer *cp, boolean_t do_null_update)
 {
+   boolean_t needs_update;
vdev_t *vd;
-   spa_t *spa;
char *physpath;
int error, physpath_len;
 
-   vd = cp->private;
-   if (vd == NULL)
-   return;
-
-   if (strcmp(attr, "GEOM::rotation_rate") == 0) {
-   vdev_geom_set_rotation_rate(vd, cp);
-   return;
-   }
-
-   if (strcmp(attr, "GEOM::physpath") != 0)
-   return;
-
if (g_access(cp, 1, 0, 0) != 0)
return;
 
-   /*
-* Record/Update physical path information for this device.
-*/
-   spa = vd->vdev_spa;
+   vd = cp->private;
physpath_len = MAXPATHLEN;
physpath = g_malloc(physpath_len, M_WAITOK|M_ZERO);
error = g_io_getattr("GEOM::physpath", cp, &physpath_len, physpath);
@@ -122,12 +107,46 @@ vdev_geom_attrchanged(struct g_consumer 
g_topology_assert();
old_physpath = vd->vdev_physpath;
vd->vdev_physpath = spa_strdup(physpath);
-   spa_async_request(spa, SPA_ASYNC_CONFIG_UPDATE);
 
-   if (old_physpath != NULL)
+   if (old_physpath != NULL) {
+   needs_update = (strcmp(old_physpath,
+   vd->vdev_physpath) != 0);
spa_strfree(old_physpath);
+   } else
+   needs_update = do_null_update;
}
g_free(physpath);
+
+   /*
+* If the physical path changed, update the config.
+* Only request an update for previously unset physpaths if
+* requested by the caller.
+*/
+   if (needs_update)
+   spa_async_request(vd->vdev_spa, SPA_ASYNC_CONFIG_UPDATE);
+
+}
+
+static void
+vdev_geom_attrchanged(struct g_consumer *cp, const char *attr)
+{
+   vdev_t *vd;
+   char *old_physpath;
+   int error;
+
+   vd = cp->private;
+   if (vd == NULL)
+   return;
+
+   if (strcmp(attr, "GEOM::rotation_rate") == 0) {
+   vdev_geom_set_rotation_rate(vd, cp);
+   return;
+   }
+
+   if (strcmp(attr, "GEOM::physpath") == 0) {
+   vdev_geom_set_physpath(cp, /*do_null_update*/B_TRUE);
+   return;
+   }
 }
 
 static void
@@ -257,8 +276,10 @@ vdev_geom_attach(struct g_provider *pp, 
 * 2) Set it to a linked list of vdevs, not just a single vdev
 */
cp->private = vd;
-   if (vd != NULL)
+   if (vd != NULL) {
vd->vdev_tsd = cp;
+   vdev_geom_set_physpath(cp, /*do_null_update*/B_FALSE);
+   }
 
cp->flags |= G_CF_DIRECT_SEND | G_CF_DIRECT_RECEIVE;
return (cp);

Modified: head/sys/geom/geom_dev.c
==
--- head/sys/geom/geom_dev.cFri May 27 22:26:43 2016(r300880)
+++ head/sys/geom/geom_dev.cFri May 27 22:32:44 2016(r300881)
@@ -222,55 +222,68 @@ g_dev_print(void)
 }
 
 static void
-g_dev_attrchanged(struct g_consumer *cp, const char *attr)
+g_dev_set_physpath(struct g_consumer *cp)
+{
+   struct g_dev_softc *sc