From 6eacc9f8eef32095ca1ac182845a927b134f297b Mon Sep 17 00:00:00 2001 From: Andriy Gapon Date: Thu, 28 May 2015 20:39:29 +0200 Subject: [PATCH] [RFC, discussion, feedback] account for ashift when choosing buffers to be written to l2arc device If we don't account for that, then we might end up overwriting disk area of buffers that have not been evicted yet, because l2arc_evict operates in terms of disk addresses. The discrepancy between the write size calculation and the actual increment to l2ad_hand was introduced in commit e14bb3258d05c1b1077e2db7cf77088924e56919 Also, consistently use asize / a_sz for the allocated size, psize / p_sz for the physical size. Where the latter accounts for possible size reduction because of compression, whereas the former accounts for possible size expansion because of alignment requirements. The code still assumes that either underlying storage subsystems or hardware is able to do read-modify-write when an L2ARC buffer size is not a multiple of a disk's block size. This is true for 4KB sector disks that provide 512B sector emulation, but may not be true in general. In other words, we currently do not have any code to make sure that an L2ARC buffer, whether compressed or not, which is used for physical I/O has a suitable size. --- module/zfs/arc.c | 29 ++++++++++++++++++++--------- 1 file changed, 20 insertions(+), 9 deletions(-) diff --git a/module/zfs/arc.c b/module/zfs/arc.c index 1699ea7e7e7a..ee6b16ce9940 100644 --- a/module/zfs/arc.c +++ b/module/zfs/arc.c @@ -5021,6 +5021,7 @@ l2arc_write_buffers(spa_t *spa, l2arc_dev_t *dev, uint64_t target_sz, l2arc_buf_hdr_t *l2hdr; kmutex_t *hash_lock; uint64_t buf_sz; + uint64_t buf_a_sz; if (arc_warm == B_FALSE) ab_prev = list_next(list, ab); @@ -5049,7 +5050,15 @@ l2arc_write_buffers(spa_t *spa, l2arc_dev_t *dev, uint64_t target_sz, continue; } - if ((write_sz + ab->b_size) > target_sz) { + /* + * Assume that the buffer is not going to be compressed + * and could take more space on disk because of a larger + * disk block size. + */ + buf_sz = ab->b_size; + buf_a_sz = vdev_psize_to_asize(dev->l2ad_vdev, buf_sz); + + if ((write_asize + buf_a_sz) > target_sz) { full = B_TRUE; mutex_exit(hash_lock); break; @@ -5094,7 +5103,6 @@ l2arc_write_buffers(spa_t *spa, l2arc_dev_t *dev, uint64_t target_sz, l2hdr->b_tmp_cdata = ab->b_buf->b_data; l2hdr->b_hits = 0; - buf_sz = ab->b_size; ab->b_l2hdr = l2hdr; list_insert_head(dev->l2ad_buflist, ab); @@ -5109,6 +5117,7 @@ l2arc_write_buffers(spa_t *spa, l2arc_dev_t *dev, uint64_t target_sz, mutex_exit(hash_lock); write_sz += buf_sz; + write_asize += buf_a_sz; } mutex_exit(list_lock); @@ -5130,6 +5139,7 @@ l2arc_write_buffers(spa_t *spa, l2arc_dev_t *dev, uint64_t target_sz, * and work backwards, retracing the course of the buffer selector * loop above. */ + write_asize = 0; for (ab = list_prev(dev->l2ad_buflist, head); ab; ab = list_prev(dev->l2ad_buflist, ab)) { l2arc_buf_hdr_t *l2hdr; @@ -5172,7 +5182,7 @@ l2arc_write_buffers(spa_t *spa, l2arc_dev_t *dev, uint64_t target_sz, /* Compression may have squashed the buffer to zero length. */ if (buf_sz != 0) { - uint64_t buf_p_sz; + uint64_t buf_a_sz; wzio = zio_write_phys(pio, dev->l2ad_vdev, dev->l2ad_hand, buf_sz, buf_data, ZIO_CHECKSUM_OFF, @@ -5183,13 +5193,14 @@ l2arc_write_buffers(spa_t *spa, l2arc_dev_t *dev, uint64_t target_sz, zio_t *, wzio); (void) zio_nowait(wzio); - write_asize += buf_sz; + write_psize += buf_sz; + /* * Keep the clock hand suitably device-aligned. */ - buf_p_sz = vdev_psize_to_asize(dev->l2ad_vdev, buf_sz); - write_psize += buf_p_sz; - dev->l2ad_hand += buf_p_sz; + buf_a_sz = vdev_psize_to_asize(dev->l2ad_vdev, buf_sz); + write_asize += buf_a_sz; + dev->l2ad_hand += buf_a_sz; } } @@ -5199,8 +5210,8 @@ l2arc_write_buffers(spa_t *spa, l2arc_dev_t *dev, uint64_t target_sz, ARCSTAT_BUMP(arcstat_l2_writes_sent); ARCSTAT_INCR(arcstat_l2_write_bytes, write_asize); ARCSTAT_INCR(arcstat_l2_size, write_sz); - ARCSTAT_INCR(arcstat_l2_asize, write_asize); - vdev_space_update(dev->l2ad_vdev, write_asize, 0, 0); + ARCSTAT_INCR(arcstat_l2_asize, write_psize); + vdev_space_update(dev->l2ad_vdev, write_psize, 0, 0); /* * Bump device hand to the device start if it is approaching the end.