Author: mav Date: Sat Oct 3 07:45:12 2015 New Revision: 288557 URL: https://svnweb.freebsd.org/changeset/base/288557
Log: MFC r286598: 5701 zpool list reports incorrect "alloc" value for cache devices Modified: stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c Directory Properties: stable/10/ (props changed) Modified: stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c ============================================================================== --- stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c Sat Oct 3 07:43:33 2015 (r288556) +++ stable/10/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/arc.c Sat Oct 3 07:45:12 2015 (r288557) @@ -965,6 +965,16 @@ uint64_t zfs_crc64_table[256]; #define L2ARC_FEED_SECS 1 /* caching interval secs */ #define L2ARC_FEED_MIN_MS 200 /* min caching interval ms */ +/* + * Used to distinguish headers that are being process by + * l2arc_write_buffers(), but have yet to be assigned to a l2arc disk + * address. This can happen when the header is added to the l2arc's list + * of buffers to write in the first stage of l2arc_write_buffers(), but + * has not yet been written out which happens in the second stage of + * l2arc_write_buffers(). + */ +#define L2ARC_ADDR_UNSET ((uint64_t)(-1)) + #define l2arc_writes_sent ARCSTAT(arcstat_l2_writes_sent) #define l2arc_writes_done ARCSTAT(arcstat_l2_writes_done) @@ -1048,12 +1058,12 @@ struct l2arc_dev { uint64_t l2ad_hand; /* next write location */ uint64_t l2ad_start; /* first addr on device */ uint64_t l2ad_end; /* last addr on device */ - uint64_t l2ad_evict; /* last addr eviction reached */ boolean_t l2ad_first; /* first sweep through */ boolean_t l2ad_writing; /* currently writing */ kmutex_t l2ad_mtx; /* lock for buffer list */ list_t l2ad_buflist; /* buffer list */ list_node_t l2ad_node; /* device list node */ + refcount_t l2ad_alloc; /* allocated bytes */ }; static list_t L2ARC_dev_list; /* device list */ @@ -1425,6 +1435,7 @@ arc_hdr_realloc(arc_buf_hdr_t *hdr, kmem buf_hash_remove(hdr); bcopy(hdr, nhdr, HDR_L2ONLY_SIZE); + if (new == hdr_full_cache) { nhdr->b_flags |= ARC_FLAG_HAS_L1HDR; /* @@ -1468,6 +1479,20 @@ arc_hdr_realloc(arc_buf_hdr_t *hdr, kmem mutex_exit(&dev->l2ad_mtx); + /* + * Since we're using the pointer address as the tag when + * incrementing and decrementing the l2ad_alloc refcount, we + * must remove the old pointer (that we're about to destroy) and + * add the new pointer to the refcount. Otherwise we'd remove + * the wrong pointer address when calling arc_hdr_destroy() later. + */ + + (void) refcount_remove_many(&dev->l2ad_alloc, + hdr->b_l2hdr.b_asize, hdr); + + (void) refcount_add_many(&dev->l2ad_alloc, + nhdr->b_l2hdr.b_asize, nhdr); + buf_discard_identity(hdr); hdr->b_freeze_cksum = NULL; kmem_cache_free(old, hdr); @@ -2219,6 +2244,57 @@ arc_buf_destroy(arc_buf_t *buf, boolean_ } static void +arc_hdr_l2hdr_destroy(arc_buf_hdr_t *hdr) +{ + l2arc_buf_hdr_t *l2hdr = &hdr->b_l2hdr; + l2arc_dev_t *dev = l2hdr->b_dev; + + ASSERT(MUTEX_HELD(&dev->l2ad_mtx)); + ASSERT(HDR_HAS_L2HDR(hdr)); + + list_remove(&dev->l2ad_buflist, hdr); + + /* + * We don't want to leak the b_tmp_cdata buffer that was + * allocated in l2arc_write_buffers() + */ + arc_buf_l2_cdata_free(hdr); + + /* + * If the l2hdr's b_daddr is equal to L2ARC_ADDR_UNSET, then + * this header is being processed by l2arc_write_buffers() (i.e. + * it's in the first stage of l2arc_write_buffers()). + * Re-affirming that truth here, just to serve as a reminder. If + * b_daddr does not equal L2ARC_ADDR_UNSET, then the header may or + * may not have its HDR_L2_WRITING flag set. (the write may have + * completed, in which case HDR_L2_WRITING will be false and the + * b_daddr field will point to the address of the buffer on disk). + */ + IMPLY(l2hdr->b_daddr == L2ARC_ADDR_UNSET, HDR_L2_WRITING(hdr)); + + /* + * If b_daddr is equal to L2ARC_ADDR_UNSET, we're racing with + * l2arc_write_buffers(). Since we've just removed this header + * from the l2arc buffer list, this header will never reach the + * second stage of l2arc_write_buffers(), which increments the + * accounting stats for this header. Thus, we must be careful + * not to decrement them for this header either. + */ + if (l2hdr->b_daddr != L2ARC_ADDR_UNSET) { + ARCSTAT_INCR(arcstat_l2_asize, -l2hdr->b_asize); + ARCSTAT_INCR(arcstat_l2_size, -hdr->b_size); + + vdev_space_update(dev->l2ad_vdev, + -l2hdr->b_asize, 0, 0); + + (void) refcount_remove_many(&dev->l2ad_alloc, + l2hdr->b_asize, hdr); + } + + hdr->b_flags &= ~ARC_FLAG_HAS_L2HDR; +} + +static void arc_hdr_destroy(arc_buf_hdr_t *hdr) { if (HDR_HAS_L1HDR(hdr)) { @@ -2231,31 +2307,29 @@ arc_hdr_destroy(arc_buf_hdr_t *hdr) ASSERT(!HDR_IN_HASH_TABLE(hdr)); if (HDR_HAS_L2HDR(hdr)) { - l2arc_buf_hdr_t *l2hdr = &hdr->b_l2hdr; - boolean_t buflist_held = MUTEX_HELD(&l2hdr->b_dev->l2ad_mtx); - - if (!buflist_held) { - mutex_enter(&l2hdr->b_dev->l2ad_mtx); - l2hdr = &hdr->b_l2hdr; - } + l2arc_dev_t *dev = hdr->b_l2hdr.b_dev; + boolean_t buflist_held = MUTEX_HELD(&dev->l2ad_mtx); - trim_map_free(l2hdr->b_dev->l2ad_vdev, l2hdr->b_daddr, - l2hdr->b_asize, 0); - list_remove(&l2hdr->b_dev->l2ad_buflist, hdr); + if (!buflist_held) + mutex_enter(&dev->l2ad_mtx); /* - * We don't want to leak the b_tmp_cdata buffer that was - * allocated in l2arc_write_buffers() - */ - arc_buf_l2_cdata_free(hdr); - - ARCSTAT_INCR(arcstat_l2_size, -hdr->b_size); - ARCSTAT_INCR(arcstat_l2_asize, -l2hdr->b_asize); + * Even though we checked this conditional above, we + * need to check this again now that we have the + * l2ad_mtx. This is because we could be racing with + * another thread calling l2arc_evict() which might have + * destroyed this header's L2 portion as we were waiting + * to acquire the l2ad_mtx. If that happens, we don't + * want to re-destroy the header's L2 portion. + */ + if (HDR_HAS_L2HDR(hdr)) { + trim_map_free(dev->l2ad_vdev, hdr->b_l2hdr.b_daddr, + hdr->b_l2hdr.b_asize, 0); + arc_hdr_l2hdr_destroy(hdr); + } if (!buflist_held) - mutex_exit(&l2hdr->b_dev->l2ad_mtx); - - hdr->b_flags &= ~ARC_FLAG_HAS_L2HDR; + mutex_exit(&dev->l2ad_mtx); } if (!BUF_EMPTY(hdr)) @@ -4274,23 +4348,23 @@ arc_release(arc_buf_t *buf, void *tag) ASSERT(refcount_count(&hdr->b_l1hdr.b_refcnt) > 0); if (HDR_HAS_L2HDR(hdr)) { - ARCSTAT_INCR(arcstat_l2_asize, -hdr->b_l2hdr.b_asize); - ARCSTAT_INCR(arcstat_l2_size, -hdr->b_size); - mutex_enter(&hdr->b_l2hdr.b_dev->l2ad_mtx); - trim_map_free(hdr->b_l2hdr.b_dev->l2ad_vdev, - hdr->b_l2hdr.b_daddr, hdr->b_l2hdr.b_asize, 0); - list_remove(&hdr->b_l2hdr.b_dev->l2ad_buflist, hdr); /* - * We don't want to leak the b_tmp_cdata buffer that was - * allocated in l2arc_write_buffers() + * We have to recheck this conditional again now that + * we're holding the l2ad_mtx to prevent a race with + * another thread which might be concurrently calling + * l2arc_evict(). In that case, l2arc_evict() might have + * destroyed the header's L2 portion as we were waiting + * to acquire the l2ad_mtx. */ - arc_buf_l2_cdata_free(hdr); + if (HDR_HAS_L2HDR(hdr)) { + trim_map_free(hdr->b_l2hdr.b_dev->l2ad_vdev, + hdr->b_l2hdr.b_daddr, hdr->b_l2hdr.b_asize, 0); + arc_hdr_l2hdr_destroy(hdr); + } mutex_exit(&hdr->b_l2hdr.b_dev->l2ad_mtx); - - hdr->b_flags &= ~ARC_FLAG_HAS_L2HDR; } /* @@ -5358,6 +5432,10 @@ l2arc_write_done(zio_t *zio) ARCSTAT_INCR(arcstat_l2_asize, -hdr->b_l2hdr.b_asize); ARCSTAT_INCR(arcstat_l2_size, -hdr->b_size); + + bytes_dropped += hdr->b_l2hdr.b_asize; + (void) refcount_remove_many(&dev->l2ad_alloc, + hdr->b_l2hdr.b_asize, hdr); } /* @@ -5514,7 +5592,6 @@ l2arc_evict(l2arc_dev_t *dev, uint64_t d arc_buf_hdr_t *hdr, *hdr_prev; kmutex_t *hash_lock; uint64_t taddr; - int64_t bytes_evicted = 0; buflist = &dev->l2ad_buflist; @@ -5599,21 +5676,11 @@ top: hdr->b_flags |= ARC_FLAG_L2_EVICTED; } - /* Tell ARC this no longer exists in L2ARC. */ - ARCSTAT_INCR(arcstat_l2_asize, -hdr->b_l2hdr.b_asize); - ARCSTAT_INCR(arcstat_l2_size, -hdr->b_size); - hdr->b_flags &= ~ARC_FLAG_HAS_L2HDR; - list_remove(buflist, hdr); - - /* This may have been leftover after a failed write. */ - hdr->b_flags &= ~ARC_FLAG_L2_WRITING; + arc_hdr_l2hdr_destroy(hdr); } mutex_exit(hash_lock); } mutex_exit(&dev->l2ad_mtx); - - vdev_space_update(dev->l2ad_vdev, -bytes_evicted, 0, 0); - dev->l2ad_evict = taddr; } /* @@ -5773,6 +5840,28 @@ l2arc_write_buffers(spa_t *spa, l2arc_de hdr->b_l2hdr.b_asize = hdr->b_size; hdr->b_l1hdr.b_tmp_cdata = hdr->b_l1hdr.b_buf->b_data; + /* + * Explicitly set the b_daddr field to a known + * value which means "invalid address". This + * enables us to differentiate which stage of + * l2arc_write_buffers() the particular header + * is in (e.g. this loop, or the one below). + * ARC_FLAG_L2_WRITING is not enough to make + * this distinction, and we need to know in + * order to do proper l2arc vdev accounting in + * arc_release() and arc_hdr_destroy(). + * + * Note, we can't use a new flag to distinguish + * the two stages because we don't hold the + * header's hash_lock below, in the second stage + * of this function. Thus, we can't simply + * change the b_flags field to denote that the + * IO has been sent. We can change the b_daddr + * field of the L2 portion, though, since we'll + * be holding the l2ad_mtx; which is why we're + * using it to denote the header's state change. + */ + hdr->b_l2hdr.b_daddr = L2ARC_ADDR_UNSET; hdr->b_flags |= ARC_FLAG_HAS_L2HDR; list_insert_head(&dev->l2ad_buflist, hdr); @@ -5862,6 +5951,13 @@ l2arc_write_buffers(spa_t *spa, l2arc_de if (!L2ARC_IS_VALID_COMPRESS(HDR_GET_COMPRESS(hdr))) hdr->b_l1hdr.b_tmp_cdata = NULL; + /* + * We need to do this regardless if buf_sz is zero or + * not, otherwise, when this l2hdr is evicted we'll + * remove a reference that was never added. + */ + (void) refcount_add_many(&dev->l2ad_alloc, buf_sz, hdr); + /* Compression may have squashed the buffer to zero length. */ if (buf_sz != 0) { uint64_t buf_a_sz; @@ -5876,6 +5972,7 @@ l2arc_write_buffers(spa_t *spa, l2arc_de (void) zio_nowait(wzio); stats_size += buf_sz; + /* * Keep the clock hand suitably device-aligned. */ @@ -5900,7 +5997,6 @@ l2arc_write_buffers(spa_t *spa, l2arc_de */ if (dev->l2ad_hand >= (dev->l2ad_end - target_sz)) { dev->l2ad_hand = dev->l2ad_start; - dev->l2ad_evict = dev->l2ad_start; dev->l2ad_first = B_FALSE; } @@ -6207,7 +6303,6 @@ l2arc_add_vdev(spa_t *spa, vdev_t *vd) adddev->l2ad_start = VDEV_LABEL_START_SIZE; adddev->l2ad_end = VDEV_LABEL_START_SIZE + vdev_get_min_asize(vd); adddev->l2ad_hand = adddev->l2ad_start; - adddev->l2ad_evict = adddev->l2ad_start; adddev->l2ad_first = B_TRUE; adddev->l2ad_writing = B_FALSE; @@ -6220,6 +6315,7 @@ l2arc_add_vdev(spa_t *spa, vdev_t *vd) offsetof(arc_buf_hdr_t, b_l2hdr.b_l2node)); vdev_space_update(vd, 0, 0, adddev->l2ad_end - adddev->l2ad_hand); + refcount_create(&adddev->l2ad_alloc); /* * Add device to global list @@ -6265,6 +6361,7 @@ l2arc_remove_vdev(vdev_t *vd) l2arc_evict(remdev, 0, B_TRUE); list_destroy(&remdev->l2ad_buflist); mutex_destroy(&remdev->l2ad_mtx); + refcount_destroy(&remdev->l2ad_alloc); kmem_free(remdev, sizeof (l2arc_dev_t)); } _______________________________________________ svn-src-all@freebsd.org mailing list https://lists.freebsd.org/mailman/listinfo/svn-src-all To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"