OpenZFS 6950 - ARC should cache compressed data

Authored by: George Wilson <george.wilson@delphix.com>
Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Reviewed by: Dan Kimmel <dan.kimmel@delphix.com>
Reviewed by: Matt Ahrens <mahrens@delphix.com>
Reviewed by: Paul Dagnelie <pcd@delphix.com>
Reviewed by: Tom Caputi <tcaputi@datto.com>
Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov>
Ported by: David Quigley <david.quigley@intel.com>

This review covers the reading and writing of compressed arc headers, sharing
data between the arc_hdr_t and the arc_buf_t, and the implementation of a new
dbuf cache to keep frequently access data uncompressed.

I've added a new member to l1 arc hdr called b_pdata. The b_pdata always hangs
off the arc_buf_hdr_t (if an L1 hdr is in use) and points to the physical block
for that DVA. The physical block may or may not be compressed. If compressed
arc is enabled and the block on-disk is compressed, then the b_pdata will match
the block on-disk and remain compressed in memory. If the block on disk is not
compressed, then neither will the b_pdata. Lastly, if compressed arc is
disabled, then b_pdata will always be an uncompressed version of the on-disk
block.

Typically the arc will cache only the arc_buf_hdr_t and will aggressively evict
any arc_buf_t's that are no longer referenced. This means that the arc will
primarily have compressed blocks as the arc_buf_t's are considered overhead and
are always uncompressed. When a consumer reads a block we first look to see if
the arc_buf_hdr_t is cached. If the hdr is cached then we allocate a new
arc_buf_t and decompress the b_pdata contents into the arc_buf_t's b_data. If
the hdr already has a arc_buf_t, then we will allocate an additional arc_buf_t
and bcopy the uncompressed contents from the first arc_buf_t to the new one.

Writing to the compressed arc requires that we first discard the b_pdata since
the physical block is about to be rewritten. The new data contents will be
passed in via an arc_buf_t (uncompressed) and during the I/O pipeline stages we
will copy the physical block contents to a newly allocated b_pdata.

When an l2arc is inuse it will also take advantage of the b_pdata. Now the
l2arc will always write the contents of b_pdata to the l2arc. This means that
when compressed arc is enabled that the l2arc blocks are identical to those
stored in the main data pool. This provides a significant advantage since we
can leverage the bp's checksum when reading from the l2arc to determine if the
contents are valid. If the compressed arc is disabled, then we must first
transform the read block to look like the physical block in the main data pool
before comparing the checksum and determining it's valid.

OpenZFS-issue: https://www.illumos.org/issues/6950
OpenZFS-commit: openzfs/openzfs@7fc10f0
Issue #5078

Author: grwilson

cmd/zdb/zdb.c

@@ -1297,7 +1297,7 @@ visit_indirect(spa_t *spa, const dnode_phys_t *dnp,
 		}
 		if (!err)
 			ASSERT3U(fill, ==, BP_GET_FILL(bp));
-		(void) arc_buf_remove_ref(buf, &buf);
+		arc_buf_destroy(buf, &buf);
 	}
 
 	return (err);

cmd/ztest/ztest.c

@@ -192,6 +192,7 @@ static const ztest_shared_opts_t ztest_opts_defaults = {
 extern uint64_t metaslab_gang_bang;
 extern uint64_t metaslab_df_alloc_threshold;
 extern int metaslab_preload_limit;
+extern boolean_t zfs_compressed_arc_enabled;
 
 static ztest_shared_opts_t *ztest_shared_opts;
 static ztest_shared_opts_t ztest_opts;
@@ -5880,6 +5881,12 @@ ztest_resume_thread(void *arg)
 		if (spa_suspended(spa))
 			ztest_resume(spa);
 		(void) poll(NULL, 0, 100);
+
+		/*
+		 * Periodically change the zfs_compressed_arc_enabled setting.
+		 */
+		if (ztest_random(10) == 0)
+			zfs_compressed_arc_enabled = ztest_random(2);
 	}
 
 	thread_exit();

include/sys/arc.h

@@ -44,12 +44,24 @@ extern "C" {
  */
 #define	ARC_EVICT_ALL	-1ULL
 
+#define	HDR_SET_LSIZE(hdr, x) do { \
+	ASSERT(IS_P2ALIGNED(x, 1U << SPA_MINBLOCKSHIFT)); \
+	(hdr)->b_lsize = ((x) >> SPA_MINBLOCKSHIFT); \
+_NOTE(CONSTCOND) } while (0)
+
+#define	HDR_SET_PSIZE(hdr, x) do { \
+	ASSERT(IS_P2ALIGNED((x), 1U << SPA_MINBLOCKSHIFT)); \
+	(hdr)->b_psize = ((x) >> SPA_MINBLOCKSHIFT); \
+_NOTE(CONSTCOND) } while (0)
+
+#define	HDR_GET_LSIZE(hdr)	((hdr)->b_lsize << SPA_MINBLOCKSHIFT)
+#define	HDR_GET_PSIZE(hdr)	((hdr)->b_psize << SPA_MINBLOCKSHIFT)
+
 typedef struct arc_buf_hdr arc_buf_hdr_t;
 typedef struct arc_buf arc_buf_t;
 typedef struct arc_prune arc_prune_t;
 typedef void arc_done_func_t(zio_t *zio, arc_buf_t *buf, void *private);
 typedef void arc_prune_func_t(int64_t bytes, void *private);
-typedef int arc_evict_func_t(void *private);
 
 /* Shared module parameters */
 extern int zfs_arc_average_blocksize;
@@ -58,6 +70,8 @@ extern int zfs_arc_average_blocksize;
 arc_done_func_t arc_bcopy_func;
 arc_done_func_t arc_getbuf_func;
 
+extern int zfs_arc_num_sublists_per_state;
+
 /* generic arc_prune_func_t wrapper for callbacks */
 struct arc_prune {
 	arc_prune_func_t	*p_pfunc;
@@ -77,37 +91,54 @@ typedef enum arc_flags
 	/*
 	 * Public flags that can be passed into the ARC by external consumers.
 	 */
-	ARC_FLAG_NONE			= 1 << 0,	/* No flags set */
-	ARC_FLAG_WAIT			= 1 << 1,	/* perform sync I/O */
-	ARC_FLAG_NOWAIT			= 1 << 2,	/* perform async I/O */
-	ARC_FLAG_PREFETCH		= 1 << 3,	/* I/O is a prefetch */
-	ARC_FLAG_CACHED			= 1 << 4,	/* I/O was in cache */
-	ARC_FLAG_L2CACHE		= 1 << 5,	/* cache in L2ARC */
-	ARC_FLAG_L2COMPRESS		= 1 << 6,	/* compress in L2ARC */
-	ARC_FLAG_PREDICTIVE_PREFETCH	= 1 << 7,	/* I/O from zfetch */
+	ARC_FLAG_WAIT			= 1 << 0,	/* perform sync I/O */
+	ARC_FLAG_NOWAIT			= 1 << 1,	/* perform async I/O */
+	ARC_FLAG_PREFETCH		= 1 << 2,	/* I/O is a prefetch */
+	ARC_FLAG_CACHED			= 1 << 3,	/* I/O was in cache */
+	ARC_FLAG_L2CACHE		= 1 << 4,	/* cache in L2ARC */
+	ARC_FLAG_PREDICTIVE_PREFETCH	= 1 << 5,	/* I/O from zfetch */
 
 	/*
 	 * Private ARC flags.  These flags are private ARC only flags that
 	 * will show up in b_flags in the arc_hdr_buf_t. These flags should
 	 * only be set by ARC code.
 	 */
-	ARC_FLAG_IN_HASH_TABLE		= 1 << 8,	/* buffer is hashed */
-	ARC_FLAG_IO_IN_PROGRESS		= 1 << 9,	/* I/O in progress */
-	ARC_FLAG_IO_ERROR		= 1 << 10,	/* I/O failed for buf */
-	ARC_FLAG_FREED_IN_READ		= 1 << 11,	/* freed during read */
-	ARC_FLAG_BUF_AVAILABLE		= 1 << 12,	/* block not in use */
-	ARC_FLAG_INDIRECT		= 1 << 13,	/* indirect block */
+	ARC_FLAG_IN_HASH_TABLE		= 1 << 6,	/* buffer is hashed */
+	ARC_FLAG_IO_IN_PROGRESS		= 1 << 7,	/* I/O in progress */
+	ARC_FLAG_IO_ERROR		= 1 << 8,	/* I/O failed for buf */
+	ARC_FLAG_INDIRECT		= 1 << 9,	/* indirect block */
 	/* Indicates that block was read with ASYNC priority. */
-	ARC_FLAG_PRIO_ASYNC_READ	= 1 << 14,
-	ARC_FLAG_L2_WRITING		= 1 << 15,	/* write in progress */
-	ARC_FLAG_L2_EVICTED		= 1 << 16,	/* evicted during I/O */
-	ARC_FLAG_L2_WRITE_HEAD		= 1 << 17,	/* head of write list */
+	ARC_FLAG_PRIO_ASYNC_READ	= 1 << 10,
+	ARC_FLAG_L2_WRITING		= 1 << 11,	/* write in progress */
+	ARC_FLAG_L2_EVICTED		= 1 << 12,	/* evicted during I/O */
+	ARC_FLAG_L2_WRITE_HEAD		= 1 << 13,	/* head of write list */
 	/* indicates that the buffer contains metadata (otherwise, data) */
-	ARC_FLAG_BUFC_METADATA		= 1 << 18,
+	ARC_FLAG_BUFC_METADATA		= 1 << 14,
 
 	/* Flags specifying whether optional hdr struct fields are defined */
-	ARC_FLAG_HAS_L1HDR		= 1 << 19,
-	ARC_FLAG_HAS_L2HDR		= 1 << 20,
+	ARC_FLAG_HAS_L1HDR		= 1 << 15,
+	ARC_FLAG_HAS_L2HDR		= 1 << 16,
+
+	/*
+	 * Indicates the arc_buf_hdr_t's b_pdata matches the on-disk data.
+	 * This allows the l2arc to use the blkptr's checksum to verify
+	 * the data without having to store the checksum in the hdr.
+	 */
+	ARC_FLAG_COMPRESSED_ARC		= 1 << 17,
+	ARC_FLAG_SHARED_DATA		= 1 << 18,
+
+	/*
+	 * The arc buffer's compression mode is stored in the top 7 bits of the
+	 * flags field, so these dummy flags are included so that MDB can
+	 * interpret the enum properly.
+	 */
+	ARC_FLAG_COMPRESS_0		= 1 << 24,
+	ARC_FLAG_COMPRESS_1		= 1 << 25,
+	ARC_FLAG_COMPRESS_2		= 1 << 26,
+	ARC_FLAG_COMPRESS_3		= 1 << 27,
+	ARC_FLAG_COMPRESS_4		= 1 << 28,
+	ARC_FLAG_COMPRESS_5		= 1 << 29,
+	ARC_FLAG_COMPRESS_6		= 1 << 30
 
 } arc_flags_t;
 
@@ -116,11 +147,10 @@ struct arc_buf {
 	arc_buf_t		*b_next;
 	kmutex_t		b_evict_lock;
 	void			*b_data;
-	arc_evict_func_t	*b_efunc;
-	void			*b_private;
 };
 
 typedef enum arc_buf_contents {
+	ARC_BUFC_INVALID,			/* invalid type */
 	ARC_BUFC_DATA,				/* buffer contains data */
 	ARC_BUFC_METADATA,			/* buffer contains metadata */
 	ARC_BUFC_NUMTYPES
@@ -154,7 +184,7 @@ typedef struct arc_buf_info {
 	arc_state_type_t	abi_state_type;
 	arc_buf_contents_t	abi_state_contents;
 	uint32_t		abi_flags;
-	uint32_t		abi_datacnt;
+	uint32_t		abi_bufcnt;
 	uint64_t		abi_size;
 	uint64_t		abi_spa;
 	uint64_t		abi_access;
@@ -171,21 +201,19 @@ typedef struct arc_buf_info {
 
 void arc_space_consume(uint64_t space, arc_space_type_t type);
 void arc_space_return(uint64_t space, arc_space_type_t type);
-arc_buf_t *arc_buf_alloc(spa_t *spa, uint64_t size, void *tag,
+arc_buf_t *arc_alloc_buf(spa_t *spa, int32_t size, void *tag,
     arc_buf_contents_t type);
 arc_buf_t *arc_loan_buf(spa_t *spa, uint64_t size);
 void arc_return_buf(arc_buf_t *buf, void *tag);
 void arc_loan_inuse_buf(arc_buf_t *buf, void *tag);
-void arc_buf_add_ref(arc_buf_t *buf, void *tag);
-boolean_t arc_buf_remove_ref(arc_buf_t *buf, void *tag);
+void arc_buf_destroy(arc_buf_t *buf, void *tag);
 void arc_buf_info(arc_buf_t *buf, arc_buf_info_t *abi, int state_index);
 uint64_t arc_buf_size(arc_buf_t *buf);
 void arc_release(arc_buf_t *buf, void *tag);
 int arc_released(arc_buf_t *buf);
 void arc_buf_sigsegv(int sig, siginfo_t *si, void *unused);
 void arc_buf_freeze(arc_buf_t *buf);
 void arc_buf_thaw(arc_buf_t *buf);
-boolean_t arc_buf_eviction_needed(arc_buf_t *buf);
 #ifdef ZFS_DEBUG
 int arc_referenced(arc_buf_t *buf);
 #endif
@@ -194,8 +222,7 @@ int arc_read(zio_t *pio, spa_t *spa, const blkptr_t *bp,
     arc_done_func_t *done, void *private, zio_priority_t priority, int flags,
     arc_flags_t *arc_flags, const zbookmark_phys_t *zb);
 zio_t *arc_write(zio_t *pio, spa_t *spa, uint64_t txg,
-    blkptr_t *bp, arc_buf_t *buf, boolean_t l2arc, boolean_t l2arc_compress,
-    const zio_prop_t *zp,
+    blkptr_t *bp, arc_buf_t *buf, boolean_t l2arc, const zio_prop_t *zp,
     arc_done_func_t *ready, arc_done_func_t *child_ready,
     arc_done_func_t *physdone, arc_done_func_t *done,
     void *private, zio_priority_t priority, int zio_flags,
@@ -205,13 +232,11 @@ arc_prune_t *arc_add_prune_callback(arc_prune_func_t *func, void *private);
 void arc_remove_prune_callback(arc_prune_t *p);
 void arc_freed(spa_t *spa, const blkptr_t *bp);
 
-void arc_set_callback(arc_buf_t *buf, arc_evict_func_t *func, void *private);
-boolean_t arc_clear_callback(arc_buf_t *buf);
-
 void arc_flush(spa_t *spa, boolean_t retry);
 void arc_tempreserve_clear(uint64_t reserve);
 int arc_tempreserve_space(uint64_t reserve, uint64_t txg);
 
+uint64_t arc_max_bytes(void);
 void arc_init(void);
 void arc_fini(void);
 

include/sys/arc_impl.h

@@ -74,7 +74,7 @@ typedef struct arc_state {
 	/*
 	 * total amount of evictable data in this state
 	 */
-	uint64_t arcs_lsize[ARC_BUFC_NUMTYPES];
+	refcount_t arcs_esize[ARC_BUFC_NUMTYPES];
 	/*
 	 * total amount of data in this state; this includes: evictable,
 	 * non-evictable, ARC_BUFC_DATA, and ARC_BUFC_METADATA.
@@ -140,11 +140,13 @@ struct arc_write_callback {
  */
 typedef struct l1arc_buf_hdr {
 	kmutex_t		b_freeze_lock;
+	zio_cksum_t		*b_freeze_cksum;
 
 	arc_buf_t		*b_buf;
-	uint32_t		b_datacnt;
+	uint32_t		b_bufcnt;
 	/* for waiting on writes to complete */
 	kcondvar_t		b_cv;
+	uint8_t			b_byteswap;
 
 
 	/* protected by arc state mutex */
@@ -163,8 +165,7 @@ typedef struct l1arc_buf_hdr {
 	refcount_t		b_refcnt;
 
 	arc_callback_t		*b_acb;
-	/* temporary buffer holder for in-flight compressed data */
-	void			*b_tmp_cdata;
+	void			*b_pdata;
 } l1arc_buf_hdr_t;
 
 typedef struct l2arc_dev {
@@ -185,10 +186,7 @@ typedef struct l2arc_buf_hdr {
 	/* protected by arc_buf_hdr mutex */
 	l2arc_dev_t		*b_dev;		/* L2ARC device */
 	uint64_t		b_daddr;	/* disk address, offset byte */
-	/* real alloc'd buffer size depending on b_compress applied */
 	uint32_t		b_hits;
-	int32_t			b_asize;
-	uint8_t			b_compress;
 
 	list_node_t		b_l2node;
 } l2arc_buf_hdr_t;
@@ -202,20 +200,37 @@ struct arc_buf_hdr {
 	/* protected by hash lock */
 	dva_t			b_dva;
 	uint64_t		b_birth;
-	/*
-	 * Even though this checksum is only set/verified when a buffer is in
-	 * the L1 cache, it needs to be in the set of common fields because it
-	 * must be preserved from the time before a buffer is written out to
-	 * L2ARC until after it is read back in.
-	 */
-	zio_cksum_t		*b_freeze_cksum;
 
+	arc_buf_contents_t	b_type;
 	arc_buf_hdr_t		*b_hash_next;
 	arc_flags_t		b_flags;
 
-	/* immutable */
-	int32_t			b_size;
-	uint64_t		b_spa;
+	/*
+	 * This field stores the size of the data buffer after
+	 * compression, and is set in the arc's zio completion handlers.
+	 * It is in units of SPA_MINBLOCKSIZE (e.g. 1 == 512 bytes).
+	 *
+	 * While the block pointers can store up to 32MB in their psize
+	 * field, we can only store up to 32MB minus 512B. This is due
+	 * to the bp using a bias of 1, whereas we use a bias of 0 (i.e.
+	 * a field of zeros represents 512B in the bp). We can't use a
+	 * bias of 1 since we need to reserve a psize of zero, here, to
+	 * represent holes and embedded blocks.
+	 *
+	 * This isn't a problem in practice, since the maximum size of a
+	 * buffer is limited to 16MB, so we never need to store 32MB in
+	 * this field. Even in the upstream illumos code base, the
+	 * maximum size of a buffer is limited to 16MB.
+	 */
+	uint16_t		b_psize;
+
+	/*
+	 * This field stores the size of the data buffer before
+	 * compression, and cannot change once set. It is in units
+	 * of SPA_MINBLOCKSIZE (e.g. 2 == 1024 bytes)
+	 */
+	uint16_t		b_lsize;	/* immutable */
+	uint64_t		b_spa;		/* immutable */
 
 	/* L2ARC fields. Undefined when not in L2ARC. */
 	l2arc_buf_hdr_t		b_l2hdr;

include/sys/dbuf.h

@@ -36,6 +36,7 @@
 #include <sys/zfs_context.h>
 #include <sys/refcount.h>
 #include <sys/zrlock.h>
+#include <sys/multilist.h>
 
 #ifdef	__cplusplus
 extern "C" {
@@ -228,6 +229,11 @@ typedef struct dmu_buf_impl {
 	 */
 	avl_node_t db_link;
 
+	/*
+	 * Link in dbuf_cache.
+	 */
+	multilist_node_t db_cache_link;
+
 	/* Data which is unique to data (leaf) blocks: */
 
 	/* User callback information. */
@@ -303,8 +309,7 @@ void dmu_buf_write_embedded(dmu_buf_t *dbuf, void *data,
     bp_embedded_type_t etype, enum zio_compress comp,
     int uncompressed_size, int compressed_size, int byteorder, dmu_tx_t *tx);
 
-void dbuf_clear(dmu_buf_impl_t *db);
-void dbuf_evict(dmu_buf_impl_t *db);
+void dbuf_destroy(dmu_buf_impl_t *db);
 
 void dbuf_unoverride(dbuf_dirty_record_t *dr);
 void dbuf_sync_list(list_t *list, int level, dmu_tx_t *tx);
@@ -342,10 +347,6 @@ boolean_t dbuf_is_metadata(dmu_buf_impl_t *db);
 	(dbuf_is_metadata(_db) &&					\
 	((_db)->db_objset->os_secondary_cache == ZFS_CACHE_METADATA)))
 
-#define	DBUF_IS_L2COMPRESSIBLE(_db)					\
-	((_db)->db_objset->os_compress != ZIO_COMPRESS_OFF ||		\
-	(dbuf_is_metadata(_db) && zfs_mdcomp_disable == B_FALSE))
-
 #ifdef ZFS_DEBUG
 
 /*

include/sys/refcount.h

@@ -70,6 +70,7 @@ int64_t refcount_remove(refcount_t *rc, void *holder_tag);
 int64_t refcount_add_many(refcount_t *rc, uint64_t number, void *holder_tag);
 int64_t refcount_remove_many(refcount_t *rc, uint64_t number, void *holder_tag);
 void refcount_transfer(refcount_t *dst, refcount_t *src);
+void refcount_transfer_ownership(refcount_t *, void *, void *);
 
 void refcount_init(void);
 void refcount_fini(void);
@@ -97,6 +98,7 @@ typedef struct refcount {
 	atomic_add_64(&(src)->rc_count, -__tmp); \
 	atomic_add_64(&(dst)->rc_count, __tmp); \
 }
+#define	refcount_transfer_ownership(rc, current_holder, new_holder)
 
 #define	refcount_init()
 #define	refcount_fini()

include/sys/spa.h

@@ -135,6 +135,8 @@ _NOTE(CONSTCOND) } while (0)
 #define	SPA_PSIZEBITS		16	/* PSIZE up to 32M (2^16 * 512)	*/
 #define	SPA_ASIZEBITS		24	/* ASIZE up to 64 times larger	*/
 
+#define	SPA_COMPRESSBITS	7
+
 /*
  * All SPA data is represented by 128-bit data virtual addresses (DVAs).
  * The members of the dva_t should be considered opaque outside the SPA.
@@ -363,8 +365,10 @@ _NOTE(CONSTCOND) } while (0)
 	    16, SPA_PSIZEBITS, SPA_MINBLOCKSHIFT, 1, x); \
 _NOTE(CONSTCOND) } while (0)
 
-#define	BP_GET_COMPRESS(bp)		BF64_GET((bp)->blk_prop, 32, 7)
-#define	BP_SET_COMPRESS(bp, x)		BF64_SET((bp)->blk_prop, 32, 7, x)
+#define	BP_GET_COMPRESS(bp)		\
+	BF64_GET((bp)->blk_prop, 32, SPA_COMPRESSBITS)
+#define	BP_SET_COMPRESS(bp, x)		\
+	BF64_SET((bp)->blk_prop, 32, SPA_COMPRESSBITS, x)
 
 #define	BP_IS_EMBEDDED(bp)		BF64_GET((bp)->blk_prop, 39, 1)
 #define	BP_SET_EMBEDDED(bp, x)		BF64_SET((bp)->blk_prop, 39, 1, x)