Hi. I''m currently implementing something around ZFS dedup and I run across the following panic: ASSERT(dde->dde_loaded); in ddt_sync_entry():996. This is because I make heavy use of ddt_lookup() function, which marks DDT entry as loading and drops ddt_lock by calling ddt_exit(). I see nothing which would stop ddt_sync() -> ddt_sync_table() -> ddt_sync_entry() from operating on DDT entry which is being loaded by ddt_lookup(). It is not even protected by the ddt_lock, so I think it is also possible that DDT entry can be removed and freed from under us. The following patch fixes the problem(s) for me. Note that I don''t use avl_destroy_nodes() anymore, so we pay the cost of rebalancing the tree during avl_remove(), not sure what I can do about that. The patch adds ddt_lock protection around ddt_sync_table(), checks for entries that are being loaded, wait for them and continues the process. http://people.freebsd.org/~pjd/patches/ddt.c.patch Am I missing something in my analysis? -- Pawel Jakub Dawidek http://www.wheelsystems.com pjd at FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 196 bytes Desc: not available URL: <http://mail.opensolaris.org/pipermail/zfs-code/attachments/20100625/cb7ecfec/attachment.bin>
Pawel, We only call ddt_lookup() from syncing context and by the time we''re in ddt_sync_table() there aren''t any other threads which can manipulate the entries we''re about to sync. It might be helpful to better understand how you''re trying to use this interface. Thanks, George Pawel Jakub Dawidek wrote:> Hi. > > I''m currently implementing something around ZFS dedup and I run across > the following panic: > > ASSERT(dde->dde_loaded); > > in ddt_sync_entry():996. > > This is because I make heavy use of ddt_lookup() function, which marks > DDT entry as loading and drops ddt_lock by calling ddt_exit(). > I see nothing which would stop ddt_sync() -> ddt_sync_table() -> > ddt_sync_entry() from operating on DDT entry which is being loaded by > ddt_lookup(). It is not even protected by the ddt_lock, so I think it is > also possible that DDT entry can be removed and freed from under us. > > The following patch fixes the problem(s) for me. Note that I don''t use > avl_destroy_nodes() anymore, so we pay the cost of rebalancing the tree > during avl_remove(), not sure what I can do about that. > > The patch adds ddt_lock protection around ddt_sync_table(), checks for > entries that are being loaded, wait for them and continues the process. > > http://people.freebsd.org/~pjd/patches/ddt.c.patch > > Am I missing something in my analysis? > > > > ------------------------------------------------------------------------ > > _______________________________________________ > zfs-code mailing list > zfs-code at opensolaris.org > http://mail.opensolaris.org/mailman/listinfo/zfs-code
On Fri, Jun 25, 2010 at 10:59:48PM -0700, George Wilson wrote:> Pawel, > > We only call ddt_lookup() from syncing context and by the time we''re in > ddt_sync_table() there aren''t any other threads which can manipulate the > entries we''re about to sync.I can see ddt_lookup() being called from zio_write_issue thread (taskq) where ddt_sync_table() is called from txg_thread. So bascially during the spa_sync() all I/O threads are not running? -- Pawel Jakub Dawidek http://www.wheelsystems.com pjd at FreeBSD.org http://www.FreeBSD.org FreeBSD committer Am I Evil? Yes, I Am! -------------- next part -------------- A non-text attachment was scrubbed... Name: not available Type: application/pgp-signature Size: 196 bytes Desc: not available URL: <http://mail.opensolaris.org/pipermail/zfs-code/attachments/20100628/3a9fd7de/attachment.bin>
Pawel Jakub Dawidek wrote:> On Fri, Jun 25, 2010 at 10:59:48PM -0700, George Wilson wrote: >> Pawel, >> >> We only call ddt_lookup() from syncing context and by the time we''re in >> ddt_sync_table() there aren''t any other threads which can manipulate the >> entries we''re about to sync. > > I can see ddt_lookup() being called from zio_write_issue thread > (taskq) where ddt_sync_table() is called from txg_thread. > So bascially during the spa_sync() all I/O threads are not running? >I don''t think you meant spa_sync() but rather ddt_sync(). Once you are in ddt_sync() all dedup related I/O has completed. Thanks, George