Hi Pawel,

Unfortunately your fix will not work correctly, because the DMU code
expects all of the dnode_t fields to be initialized to 0 after
allocation, which will not happen if you eliminate that call.

In fact we had the same "fix" in the Lustre tree for a while, but it led
to assertion failures during testing due to that problem.

I am attaching a patch that contains a more thorough fix, however please
note that this hasn't gone through a complete code review, not from the
Lustre team nor from the ZFS team. It seems to be working reasonably for
us in our very limited testing, but use at your own risk :-)

I am planning to do a code review of this patch and integrate it into
Nevada some time in the future, along with some other minor bug fixes.

Best regards,
Ricardo

On S?b, 2009-09-05 at 14:16 +0200, Pawel Jakub Dawidek wrote:
> Hi.
> 
> I just noticed that we have a bug fix in FreeBSD that I never reported
> back. The thing is that ZFS creates dnode cache zone:
> 
>       dnode_cache = kmem_cache_create("dnode_t",
>           sizeof (dnode_t),
>           0, dnode_cons, dnode_dest, NULL, NULL, NULL, 0);
> 
> And declares dnode_cons() as constructor callback. But in the code we
> can find that dnode_cons() is called directly for the second time after
> allocation:
> 
>       dnode_t *dn = kmem_cache_alloc(dnode_cache, KM_SLEEP);
>       (void) dnode_cons(dn, NULL, 0); /* XXX */
> 
> This way we initialize everything in dnode_cons() twice, which might not
> be critial on OpenSolaris, but it is critical on FreeBSD. And it is a
> waste of time for both systems.
> 
> This simple patch works on FreeBSD:
> 
> --- dnode.c.orig      2009-09-05 14:01:09.971645686 +0200
> +++ dnode.c   2009-09-05 14:01:28.208648713 +0200
> @@ -276,7 +276,6 @@
>      uint64_t object)
>  {
>       dnode_t *dn = kmem_cache_alloc(dnode_cache, KM_SLEEP);
> -     (void) dnode_cons(dn, NULL, 0); /* XXX */
>  
>       dn->dn_objset = os;
>       dn->dn_object = object;
> 
> _______________________________________________
> zfs-code mailing list
> zfs-code at opensolaris.org
> http://mail.opensolaris.org/mailman/listinfo/zfs-code
-------------- next part --------------
An embedded message was scrubbed...
From: Brian Behlendorf <behlendo...@llnl.gov>
Subject: [PATCH] fix dnode constructor
Date: Sat, 05 Sep 2009 14:47:10 +0200
Size: 8335
URL: 
<http://mail.opensolaris.org/pipermail/zfs-code/attachments/20090905/b896f4cd/attachment.nws>
-------------- next part --------------
An embedded message was scrubbed...
From: Brian Behlendorf <behlendo...@llnl.gov>
Subject: [PATCH] fix dnode constructor
Date: Sat, 05 Sep 2009 14:55:10 +0200
Size: 8336
URL: 
<http://mail.opensolaris.org/pipermail/zfs-code/attachments/20090905/b896f4cd/attachment-0001.nws>

Reply via email to