Alexander Leidinger
2009-Dec-30 14:59 UTC
Some fixes for ZFS on 7-stable (more testers wanted)
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
Alexander Leidinger
2010-Jan-06 08:23 UTC
Some fixes for ZFS on 7-stable (more testers wanted)
On Wed, 30 Dec 2009 15:59:10 +0100 Alexander Leidinger <Alexander@Leidinger.net> wrote:> 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).I committed this. On my system I didn't see more unstable behavior with this patch, as I was seeing without this patch. I will continue with merging more ZFS fixes to 7-stable. Bye, Alexander.