Some fixes for ZFS on 7-stable (more testers wanted)

Alexander Leidinger Alexander at Leidinger.net
Wed Dec 30 14:59:26 UTC 2009


Hi,

I backported some changes from 8-stable to 7-stable, I have this  
running on one 7-stable machine. I would like to get some more  
feedback for it (even an "it works for me" would be great). The main  
part of this change is that the FreeBSD taskqueue is used now instead  
of the opensolaris one (a more detailed list is below).

It would also be nice if someone could have a look at the  
FIRST_THREAD_IN_PROC part. Can there be more than one thread at this  
place (I do not think so) and I should use  
FOREACH_THREAD_IN_PROC_instead?

How to apply:
  - cd /usr/src/
  - fetch http://www.Leidinger.net/FreeBSD/test/releng7_zfs_merge3.diff
  - fetch http://www.Leidinger.net/FreeBSD/test/opensolaris_taskq.c
  - fetch http://www.Leidinger.net/FreeBSD/test/taskq.h
  - mv taskq.h sys/cddl/contrib/opensolaris/uts/common/sys/taskq.h
  - mv opensolaris_taskq.c sys/cddl/compat/opensolaris/kern/opensolaris_taskq.c
  - patch -p0 --quiet <releng7_zfs_merge3.diff
  - ignore the 2 .rej files
  - rm -f sys/cddl/compat/opensolaris/sys/taskq_impl.h*
  - rm -f sys/cddl/compat/opensolaris/sys/taskq.h*
  - rm -f sys/cddl/contrib/opensolaris/uts/common/os/taskq.c*
  - rebuild kernel

Here is the list of those 16 of 58 outstanding patches which are  
included in this patch:

MERGE
http://svn.freebsd.org/viewvc/base?view=revision&revision=185310
ganbold - very easy merge
---snip---
Remove unused variable.

Found with:     Coverity Prevent(tm)
CID: 3669,3671
---snip---

MERGE
http://svn.freebsd.org/viewvc/base?view=revision&revision=185319
pjd - applicable to RELENG_7?
---snip---
Fix locking (file descriptor table and Giant around VFS).
---snip---

MERGE
http://svn.freebsd.org/viewvc/base?view=revision&revision=192689
trasz - very easy merge
---snip---
Fix comment.
---snip---

MERGE
http://svn.freebsd.org/viewvc/base?view=revision&revision=193110
kmacy - easy merge
---snip---
work around snapshot shutdown race reported by Henri Hennebert
---snip---

NOT APPLICABLE to ZFS in 7-stable
http://svn.freebsd.org/viewvc/base?view=revision&revision=193128
kmacy - first maybe, second to check, 3rd to check
---snip---
fix xdrmem_control to be safe in an if statement
fix zfs to depend on krpc
remove xdr from zfs makefile
---snip---

MERGE+CHANGES (no shared locks in 7-stable, this is only diff reduction)
http://svn.freebsd.org/viewvc/base?view=revision&revision=193440
ps - shared vnode locks available in RELENG_7, vn_lock same syntax?
---snip---
Support shared vnode locks for write operations when the offset is
provided on filesystems that support it.  This really improves mysql
+ innodb performance on ZFS.
---snip---

MERGE
http://svn.freebsd.org/viewvc/base?view=revision&revision=195627
marcel - easy merge
---snip---
In nvpair_native_embedded_array(), meaningless pointers are zeroed.
The programmer was aware that alignment was not guaranteed in the
packed structure and used bzero() to NULL out the pointers.
However, on ia64, the compiler is quite agressive in finding ILP
and calls to bzero() are often replaced by simple assignments (i.e.
stores). Especially when the width or size in question corresponds
with a store instruction (i.e. st1, st2, st4 or st8).

The problem here is not a compiler bug. The address of the memory
to zero-out was given by '&packed->nvl_priv' and given the type of
the 'packed' pointer the compiler could assume proper alignment for
the replacement of bzero() with an 8-byte wide store to be valid.
The problem is with the programmer. The programmer knew that the
address did not have the alignment guarantees needed for a regular
assignment, but failed to inform the compiler of that fact. In
fact, the programmer told the compiler the opposite: alignment is
guaranteed.

The fix is to avoid using a pointer of type "nvlist_t *" and
instead use a "char *" pointer as the basis for calculating the
address. This tells the compiler that only 1-byte alignment can
be assumed and the compiler will either keep the bzero() call
or instead replace it with a sequence of byte-wise stores. Both
are valid.
---snip---

MERGE
http://svn.freebsd.org/viewvc/base?view=revision&revision=195822
trasz - easy merge
---snip---
Fix extattr_list_file(2) on ZFS in case the attribute directory
doesn't exist and user doesn't have write access to the file.
Without this fix, it returns bogus value instead of 0.  For some
reason this didn't manifest on my kernel compiled with -O0.
---snip---

MERGE
http://svn.freebsd.org/viewvc/base?view=revision&revision=195909
pjd - very easy merge
---snip---
We don't support ephemeral IDs in FreeBSD and without this fix ZFS can
panic when in zfs_fuid_create_cred() when userid is negative. It is
converted to unsigned value which makes IS_EPHEMERAL() macro to
incorrectly report that this is ephemeral ID. The most reasonable
solution for now is to always report that the given ID is not ephemeral.
---snip---

MERGE
http://svn.freebsd.org/viewvc/base?view=revision&revision=196291
pjd - probably easy merge
---snip---
- Fix a race where /dev/zfs control device is created before ZFS is fully
   initialized. Also destroy /dev/zfs before doing other deinitializations.
- Initialization through taskq is no longer needed and there is a race
   where one of the zpool/zfs command loads zfs.ko and tries to do some work
   immediately, but /dev/zfs is not there yet.
---snip---

MERGE
http://svn.freebsd.org/viewvc/base?view=revision&revision=196269
marcel - easy merge
---snip---
Fix misalignment in nvpair_native_embedded() caused by the compiler
replacing the bzero(). See also revision 195627, which fixed the
misalignment in nvpair_native_embedded_array().
---snip---

MERGE (this also affects dtrace)
http://svn.freebsd.org/viewvc/base?view=revision&revision=196295
pjd - added stuff to be reviewed, taskqueue available & same syntax?
---snip---
Remove OpenSolaris taskq port (it performs very poorly in our kernel) and
replace it with wrappers around our taskqueue(9).
To make it possible implement taskqueue_member() function which returns 1
if the given thread was created by the given taskqueue.
---snip---

MERGE
http://svn.freebsd.org/viewvc/base?view=revision&revision=196297
pjd - easy merge
---snip---
Fix panic in zfs recv code. The last vnode (mountpoint's vnode) can have
0 usecount.
---snip---

MERGE
http://svn.freebsd.org/viewvc/base?view=revision&revision=196299
pjd - VI_UNLOCK same syntax in RELENG_7?
---snip---
- We need to recycle vnode instead of freeing znode.

Submitted by:        avg

- Add missing vnode interlock unlock.
- Remove redundant znode locking.
---snip---

MERGE
http://svn.freebsd.org/viewvc/base?view=revision&revision=196301
pjd - probably easy merge
---snip---
If z_buf is NULL, we should free znode immediately.
---snip---

MERGE
http://svn.freebsd.org/viewvc/base?view=revision&revision=196307
pjd - to be reviewed
---snip---
Manage asynchronous vnode release just like Solaris.
---snip---

Bye,
Alexander.

-- 
Zeus gave Leda the bird.

http://www.Leidinger.net    Alexander @ Leidinger.net: PGP ID = B0063FE7
http://www.FreeBSD.org       netchild @ FreeBSD.org  : PGP ID = 72077137


More information about the freebsd-stable mailing list