mirror of
				https://git.proxmox.com/git/mirror_zfs.git
				synced 2025-10-26 18:05:04 +03:00 
			
		
		
		
	Trust ARC_BUF_SHARED() more
In my understanding ARC_BUF_SHARED() and arc_buf_is_shared() should return identical results, except the second also asserts it deeper. The first is much cheaper though, saving few pointer dereferences. Replace production arc_buf_is_shared() calls with ARC_BUF_SHARED(), and call arc_buf_is_shared() in random assertions, while making it even more strict. On my tests this in half reduces arc_buf_destroy_impl() time, that noticeably reduces hash_lock congestion under heavy dbuf eviction. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: George Wilson <george.wilson@delphix.com> Signed-off-by: Alexander Motin <mav@FreeBSD.org> Sponsored by: iXsystems, Inc. Closes #15397
This commit is contained in:
		
							parent
							
								
									79f7de5752
								
							
						
					
					
						commit
						6e41aca519
					
				| @ -1364,7 +1364,7 @@ arc_buf_is_shared(arc_buf_t *buf) | ||||
| 	    abd_is_linear(buf->b_hdr->b_l1hdr.b_pabd) && | ||||
| 	    buf->b_data == abd_to_buf(buf->b_hdr->b_l1hdr.b_pabd)); | ||||
| 	IMPLY(shared, HDR_SHARED_DATA(buf->b_hdr)); | ||||
| 	IMPLY(shared, ARC_BUF_SHARED(buf)); | ||||
| 	EQUIV(shared, ARC_BUF_SHARED(buf)); | ||||
| 	IMPLY(shared, ARC_BUF_COMPRESSED(buf) || ARC_BUF_LAST(buf)); | ||||
| 
 | ||||
| 	/*
 | ||||
| @ -1998,7 +1998,7 @@ arc_buf_fill(arc_buf_t *buf, spa_t *spa, const zbookmark_phys_t *zb, | ||||
| 	IMPLY(encrypted, HDR_ENCRYPTED(hdr)); | ||||
| 	IMPLY(encrypted, ARC_BUF_ENCRYPTED(buf)); | ||||
| 	IMPLY(encrypted, ARC_BUF_COMPRESSED(buf)); | ||||
| 	IMPLY(encrypted, !ARC_BUF_SHARED(buf)); | ||||
| 	IMPLY(encrypted, !arc_buf_is_shared(buf)); | ||||
| 
 | ||||
| 	/*
 | ||||
| 	 * If the caller wanted encrypted data we just need to copy it from | ||||
| @ -2066,7 +2066,9 @@ arc_buf_fill(arc_buf_t *buf, spa_t *spa, const zbookmark_phys_t *zb, | ||||
| 	} | ||||
| 
 | ||||
| 	if (hdr_compressed == compressed) { | ||||
| 		if (!arc_buf_is_shared(buf)) { | ||||
| 		if (ARC_BUF_SHARED(buf)) { | ||||
| 			ASSERT(arc_buf_is_shared(buf)); | ||||
| 		} else { | ||||
| 			abd_copy_to_buf(buf->b_data, hdr->b_l1hdr.b_pabd, | ||||
| 			    arc_buf_size(buf)); | ||||
| 		} | ||||
| @ -2078,7 +2080,7 @@ arc_buf_fill(arc_buf_t *buf, spa_t *spa, const zbookmark_phys_t *zb, | ||||
| 		 * If the buf is sharing its data with the hdr, unlink it and | ||||
| 		 * allocate a new data buffer for the buf. | ||||
| 		 */ | ||||
| 		if (arc_buf_is_shared(buf)) { | ||||
| 		if (ARC_BUF_SHARED(buf)) { | ||||
| 			ASSERT(ARC_BUF_COMPRESSED(buf)); | ||||
| 
 | ||||
| 			/* We need to give the buf its own b_data */ | ||||
| @ -2090,6 +2092,8 @@ arc_buf_fill(arc_buf_t *buf, spa_t *spa, const zbookmark_phys_t *zb, | ||||
| 			/* Previously overhead was 0; just add new overhead */ | ||||
| 			ARCSTAT_INCR(arcstat_overhead_size, HDR_GET_LSIZE(hdr)); | ||||
| 		} else if (ARC_BUF_COMPRESSED(buf)) { | ||||
| 			ASSERT(!arc_buf_is_shared(buf)); | ||||
| 
 | ||||
| 			/* We need to reallocate the buf's b_data */ | ||||
| 			arc_free_data_buf(hdr, buf->b_data, HDR_GET_PSIZE(hdr), | ||||
| 			    buf); | ||||
| @ -2217,7 +2221,7 @@ arc_evictable_space_increment(arc_buf_hdr_t *hdr, arc_state_t *state) | ||||
| 
 | ||||
| 	for (arc_buf_t *buf = hdr->b_l1hdr.b_buf; buf != NULL; | ||||
| 	    buf = buf->b_next) { | ||||
| 		if (arc_buf_is_shared(buf)) | ||||
| 		if (ARC_BUF_SHARED(buf)) | ||||
| 			continue; | ||||
| 		(void) zfs_refcount_add_many(&state->arcs_esize[type], | ||||
| 		    arc_buf_size(buf), buf); | ||||
| @ -2256,7 +2260,7 @@ arc_evictable_space_decrement(arc_buf_hdr_t *hdr, arc_state_t *state) | ||||
| 
 | ||||
| 	for (arc_buf_t *buf = hdr->b_l1hdr.b_buf; buf != NULL; | ||||
| 	    buf = buf->b_next) { | ||||
| 		if (arc_buf_is_shared(buf)) | ||||
| 		if (ARC_BUF_SHARED(buf)) | ||||
| 			continue; | ||||
| 		(void) zfs_refcount_remove_many(&state->arcs_esize[type], | ||||
| 		    arc_buf_size(buf), buf); | ||||
| @ -2481,7 +2485,7 @@ arc_change_state(arc_state_t *new_state, arc_buf_hdr_t *hdr) | ||||
| 				 * add to the refcount if the arc_buf_t is | ||||
| 				 * not shared. | ||||
| 				 */ | ||||
| 				if (arc_buf_is_shared(buf)) | ||||
| 				if (ARC_BUF_SHARED(buf)) | ||||
| 					continue; | ||||
| 
 | ||||
| 				(void) zfs_refcount_add_many( | ||||
| @ -2537,7 +2541,7 @@ arc_change_state(arc_state_t *new_state, arc_buf_hdr_t *hdr) | ||||
| 				 * add to the refcount if the arc_buf_t is | ||||
| 				 * not shared. | ||||
| 				 */ | ||||
| 				if (arc_buf_is_shared(buf)) | ||||
| 				if (ARC_BUF_SHARED(buf)) | ||||
| 					continue; | ||||
| 
 | ||||
| 				(void) zfs_refcount_remove_many( | ||||
| @ -3061,9 +3065,10 @@ arc_buf_destroy_impl(arc_buf_t *buf) | ||||
| 		arc_cksum_verify(buf); | ||||
| 		arc_buf_unwatch(buf); | ||||
| 
 | ||||
| 		if (arc_buf_is_shared(buf)) { | ||||
| 		if (ARC_BUF_SHARED(buf)) { | ||||
| 			arc_hdr_clear_flags(hdr, ARC_FLAG_SHARED_DATA); | ||||
| 		} else { | ||||
| 			ASSERT(!arc_buf_is_shared(buf)); | ||||
| 			uint64_t size = arc_buf_size(buf); | ||||
| 			arc_free_data_buf(hdr, buf->b_data, size, buf); | ||||
| 			ARCSTAT_INCR(arcstat_overhead_size, -size); | ||||
| @ -3104,9 +3109,9 @@ arc_buf_destroy_impl(arc_buf_t *buf) | ||||
| 		 */ | ||||
| 		if (lastbuf != NULL && !ARC_BUF_ENCRYPTED(lastbuf)) { | ||||
| 			/* Only one buf can be shared at once */ | ||||
| 			VERIFY(!arc_buf_is_shared(lastbuf)); | ||||
| 			ASSERT(!arc_buf_is_shared(lastbuf)); | ||||
| 			/* hdr is uncompressed so can't have compressed buf */ | ||||
| 			VERIFY(!ARC_BUF_COMPRESSED(lastbuf)); | ||||
| 			ASSERT(!ARC_BUF_COMPRESSED(lastbuf)); | ||||
| 
 | ||||
| 			ASSERT3P(hdr->b_l1hdr.b_pabd, !=, NULL); | ||||
| 			arc_hdr_free_abd(hdr, B_FALSE); | ||||
| @ -6189,7 +6194,7 @@ arc_release(arc_buf_t *buf, const void *tag) | ||||
| 		ASSERT(hdr->b_l1hdr.b_buf != buf || buf->b_next != NULL); | ||||
| 		VERIFY3S(remove_reference(hdr, tag), >, 0); | ||||
| 
 | ||||
| 		if (arc_buf_is_shared(buf) && !ARC_BUF_COMPRESSED(buf)) { | ||||
| 		if (ARC_BUF_SHARED(buf) && !ARC_BUF_COMPRESSED(buf)) { | ||||
| 			ASSERT3P(hdr->b_l1hdr.b_buf, !=, buf); | ||||
| 			ASSERT(ARC_BUF_LAST(buf)); | ||||
| 		} | ||||
| @ -6206,9 +6211,9 @@ arc_release(arc_buf_t *buf, const void *tag) | ||||
| 		 * If the current arc_buf_t and the hdr are sharing their data | ||||
| 		 * buffer, then we must stop sharing that block. | ||||
| 		 */ | ||||
| 		if (arc_buf_is_shared(buf)) { | ||||
| 		if (ARC_BUF_SHARED(buf)) { | ||||
| 			ASSERT3P(hdr->b_l1hdr.b_buf, !=, buf); | ||||
| 			VERIFY(!arc_buf_is_shared(lastbuf)); | ||||
| 			ASSERT(!arc_buf_is_shared(lastbuf)); | ||||
| 
 | ||||
| 			/*
 | ||||
| 			 * First, sever the block sharing relationship between | ||||
| @ -6241,7 +6246,7 @@ arc_release(arc_buf_t *buf, const void *tag) | ||||
| 			 */ | ||||
| 			ASSERT(arc_buf_is_shared(lastbuf) || | ||||
| 			    arc_hdr_get_compress(hdr) != ZIO_COMPRESS_OFF); | ||||
| 			ASSERT(!ARC_BUF_SHARED(buf)); | ||||
| 			ASSERT(!arc_buf_is_shared(buf)); | ||||
| 		} | ||||
| 
 | ||||
| 		ASSERT(hdr->b_l1hdr.b_pabd != NULL || HDR_HAS_RABD(hdr)); | ||||
| @ -6335,9 +6340,10 @@ arc_write_ready(zio_t *zio) | ||||
| 		arc_cksum_free(hdr); | ||||
| 		arc_buf_unwatch(buf); | ||||
| 		if (hdr->b_l1hdr.b_pabd != NULL) { | ||||
| 			if (arc_buf_is_shared(buf)) { | ||||
| 			if (ARC_BUF_SHARED(buf)) { | ||||
| 				arc_unshare_buf(hdr, buf); | ||||
| 			} else { | ||||
| 				ASSERT(!arc_buf_is_shared(buf)); | ||||
| 				arc_hdr_free_abd(hdr, B_FALSE); | ||||
| 			} | ||||
| 		} | ||||
| @ -6636,9 +6642,10 @@ arc_write(zio_t *pio, spa_t *spa, uint64_t txg, | ||||
| 		 * The hdr will remain with a NULL data pointer and the | ||||
| 		 * buf will take sole ownership of the block. | ||||
| 		 */ | ||||
| 		if (arc_buf_is_shared(buf)) { | ||||
| 		if (ARC_BUF_SHARED(buf)) { | ||||
| 			arc_unshare_buf(hdr, buf); | ||||
| 		} else { | ||||
| 			ASSERT(!arc_buf_is_shared(buf)); | ||||
| 			arc_hdr_free_abd(hdr, B_FALSE); | ||||
| 		} | ||||
| 		VERIFY3P(buf->b_data, !=, NULL); | ||||
|  | ||||
		Loading…
	
		Reference in New Issue
	
	Block a user
	 Alexander Motin
						Alexander Motin