Skip to content

Commit

Permalink
Skip dnode handles use when not needed
Browse files Browse the repository at this point in the history
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.

Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
  • Loading branch information
amotin committed Jul 22, 2024
1 parent d33e2ca commit 64341ea
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 1 deletion.
16 changes: 15 additions & 1 deletion include/sys/dbuf.h
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 8 additions & 0 deletions module/zfs/dbuf.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
13 changes: 13 additions & 0 deletions module/zfs/dnode.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 64341ea

Please sign in to comment.