From ed87d456e4ebcd031d43c29b93d0a664581a51f7 Mon Sep 17 00:00:00 2001 From: Alexander Motin Date: Sun, 21 Jul 2024 21:13:42 -0400 Subject: [PATCH] Skip dnode handles use when not needed Neither FreeBSD nor Linux currently implement kmem_cache_set_move(), which means dnode_move() is never called. In such situation use of dnode handles with respective locking to access dnode from dbuf is a waste of time for no benefit. This patch implements optional simplified code for such platforms, saving at least 3 dnode lock/dereference/unlock per dbuf life cycle. Originally I hoped to drop the handles completely to save memory, but they are still used in dnodes allocation code, so left for now. Before this change in CPU profiles of some workloads I saw 4-20% of CPU time spent in zrl_add_impl()/zrl_remove(), which are gone now. Reviewed-by: Rob Wing Reviewed-by: Allan Jude Reviewed-by: Brian Behlendorf Signed-off-by: Alexander Motin Sponsored by: iXsystems, Inc. Closes #16374 --- include/sys/dbuf.h | 16 +++++++++++++++- module/zfs/dbuf.c | 8 ++++++++ module/zfs/dnode.c | 13 +++++++++++++ 3 files changed, 36 insertions(+), 1 deletion(-) diff --git a/include/sys/dbuf.h b/include/sys/dbuf.h index 3808a04cba..8b03b1f895 100644 --- a/include/sys/dbuf.h +++ b/include/sys/dbuf.h @@ -214,9 +214,15 @@ typedef struct dmu_buf_impl { struct objset *db_objset; /* - * handle to safely access the dnode we belong to (NULL when evicted) + * Handle to safely access the dnode we belong to (NULL when evicted) + * if dnode_move() is used on the platform, or just dnode otherwise. */ +#if !defined(__linux__) && !defined(__FreeBSD__) +#define USE_DNODE_HANDLE 1 struct dnode_handle *db_dnode_handle; +#else + struct dnode *db_dnode; +#endif /* * our parent buffer; if the dnode points to us directly, @@ -417,11 +423,19 @@ void dbuf_stats_destroy(void); int dbuf_dnode_findbp(dnode_t *dn, uint64_t level, uint64_t blkid, blkptr_t *bp, uint16_t *datablkszsec, uint8_t *indblkshift); +#ifdef USE_DNODE_HANDLE #define DB_DNODE(_db) ((_db)->db_dnode_handle->dnh_dnode) #define DB_DNODE_LOCK(_db) ((_db)->db_dnode_handle->dnh_zrlock) #define DB_DNODE_ENTER(_db) (zrl_add(&DB_DNODE_LOCK(_db))) #define DB_DNODE_EXIT(_db) (zrl_remove(&DB_DNODE_LOCK(_db))) #define DB_DNODE_HELD(_db) (!zrl_is_zero(&DB_DNODE_LOCK(_db))) +#else +#define DB_DNODE(_db) ((_db)->db_dnode) +#define DB_DNODE_LOCK(_db) +#define DB_DNODE_ENTER(_db) +#define DB_DNODE_EXIT(_db) +#define DB_DNODE_HELD(_db) (B_TRUE) +#endif void dbuf_init(void); void dbuf_fini(void); diff --git a/module/zfs/dbuf.c b/module/zfs/dbuf.c index 7cffaebf34..3173e06974 100644 --- a/module/zfs/dbuf.c +++ b/module/zfs/dbuf.c @@ -3103,7 +3103,11 @@ dbuf_destroy(dmu_buf_impl_t *db) */ mutex_enter(&dn->dn_mtx); dnode_rele_and_unlock(dn, db, B_TRUE); +#ifdef USE_DNODE_HANDLE db->db_dnode_handle = NULL; +#else + db->db_dnode = NULL; +#endif dbuf_hash_remove(db); } else { @@ -3252,7 +3256,11 @@ dbuf_create(dnode_t *dn, uint8_t level, uint64_t blkid, db->db_level = level; db->db_blkid = blkid; db->db_dirtycnt = 0; +#ifdef USE_DNODE_HANDLE db->db_dnode_handle = dn->dn_handle; +#else + db->db_dnode = dn; +#endif db->db_parent = parent; db->db_blkptr = blkptr; db->db_hash = hash; diff --git a/module/zfs/dnode.c b/module/zfs/dnode.c index 2904c7bfe1..5058ca374a 100644 --- a/module/zfs/dnode.c +++ b/module/zfs/dnode.c @@ -1020,6 +1020,19 @@ dnode_move(void *buf, void *newbuf, size_t size, void *arg) int64_t refcount; uint32_t dbufs; +#ifndef USE_DNODE_HANDLE + /* + * We can't move dnodes if dbufs reference them directly without + * using handles and respecitve locking. Unless USE_DNODE_HANDLE + * is defined the code below is only to make sure it still builds, + * but it should never be used, since it is unsafe. + */ +#ifdef ZFS_DEBUG + PANIC("dnode_move() called without USE_DNODE_HANDLE"); +#endif + return (KMEM_CBRC_NO); +#endif + /* * The dnode is on the objset's list of known dnodes if the objset * pointer is valid. We set the low bit of the objset pointer when