----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.csiden.org/r/245/#review805 -----------------------------------------------------------
Ship it! Ship It! - Xin LI On Oct. 6, 2015, 11:39 a.m., Justin Gibbs wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.csiden.org/r/245/ > ----------------------------------------------------------- > > (Updated Oct. 6, 2015, 11:39 a.m.) > > > Review request for OpenZFS Developer Mailing List and Christopher Siden. > > > Bugs: 6267 > https://www.illumos.org/issues/6267 > > > Repository: illumos-gate > > > Description > ------- > > ``` > Summary > ======= > o Fix bonus buffer eviction regression introduced by the fixes for > illumos issue 5056. > > o Remove assert that can trigger due to a long standing race between > draining references on dbufs and processing of freed dnodes in the > syncer. References are guaranteed to drain, and the introduction > of db_pending_evict ensures proper dbuf eviction when references > finally do drain. > > Details > ======= > The bonus buffer associated with a dnode is expected to remain resident > until: the dnode is evicted via dnode_buf_pageout(), the dnode is > freed in dnode_sync_free(), or the objset containing the dnode is > evicted via dmu_objset_evict(). However, since bonus buffers (and DMU > buffers in general) can have draining references when these events occur, > dbuf_rele_and_unlock() has logic to ensure that once these late references > are released, affected buffers will be evicted. > > dbuf_rele_and_unlock() currently checks for a dbuf for an evicting > objset via the os->os_evicting flag, and detects buffers for a freed > dnode by testing dn->dn_type and dn->dn_free_txg fields. Unfortunately, > the free'd dnode test can fire prematurely - anytime after the dnode is > scheduled to be freed via dnode_free() until the free is committed in > dnode_sync_free(). If all references to the bonus buffer are dropped > within this window, the bonus buffer will be evicted and code in > dnode_sync() that relies on the bonus buffer will fail. > > Additionally, the "free'd dnode test" isn't applied to normal buffers > (buffers that are not the bonus buffer) and there is no mechanism to > guarantee eviction in the dnode_buf_pageout() case (the dnode is not > being freed nor is the objset being evicted). > > Replace the two existing deferred eviction mechanisms with a per-dbuf > flag, db_pending_evict. This is set when explicit eviction is requested > via either dnode_evict_dbufs() or dnode_evict_bonus(). These actions > only occur after it is safe for dnode buffers to be evicted (e.g. the > bonus buffer will not be referenced again). > > uts/common/fs/zfs/sys/dbuf.h: > Add comments for boolean fields in dmu_buf_impl_t. > > Add the db_pending_evict field. > > uts/common/fs/zfs/sys/dbuf.h: > uts/common/fs/zfs/dbuf.c: > Rename db_immediate_evict to db_user_immediate_evict to avoid > confusion between dbuf user state eviction and deferred eviction > of a dbuf. > > uts/common/fs/zfs/dbuf.c: > Consistently use TRUE/FALSE for boolean fields in > dmu_buf_impl_t. > > Simplify pending eviction logic to use the new db_pending_evict > flag in all cases. > > uts/common/fs/zfs/dmu_objset.c: > uts/common/fs/zfs/sys/dmu_objset.h: > Remove objset_t's os_evicting field. This same functionality > is now provided by db_pending_evict. > > uts/common/fs/zfs/dnode_sync.c: > In dnode_evict_dbufs() and dnode_evict_bonus(), mark dbufs > with draining references (dbufs that can't be evicted inline) > as pending_evict. > > In dnode_sync_free(), remove ASSERT() that a dnode being free'd > has no active dbufs. This is usually the case, but is not > guaranteed due to draining references. (e.g. The deadlist for a > deleted dataset may still be open if another thread referenced > the dataset just before it was freed and the dsl_dataset_t hasn't > been released or is still being evicted). > ``` > > > Diffs > ----- > > usr/src/uts/common/fs/zfs/sys/dmu_objset.h > 9e98350f001f81a5399b8d8034d9f502d8c771ad > usr/src/uts/common/fs/zfs/dnode_sync.c > 0787885229d43e78fdbfd21bd12e75b2e67e9951 > usr/src/uts/common/fs/zfs/sys/dbuf.h > 482ccb01dacbdfa5e4ee890d076fb9e74dc30614 > usr/src/uts/common/fs/zfs/dmu_objset.c > f84ff378c947b913a671dd1790466f940513666c > usr/src/uts/common/fs/zfs/dbuf.c e75bac094ecba982dc4bbcb8a5d2faad2a5c0a9f > > Diff: https://reviews.csiden.org/r/245/diff/ > > > Testing > ------- > > ztest on illumos. Runtime testing on ZOL. > > > Thanks, > > Justin Gibbs > >
_______________________________________________ developer mailing list [email protected] http://lists.open-zfs.org/mailman/listinfo/developer
