mirror of
https://git.proxmox.com/git/mirror_zfs.git
synced 2026-05-22 18:40:43 +03:00
OpenZFS 9403 - assertion failed in arc_buf_destroy()
Assertion failed in arc_buf_destroy() when concurrently reading block with checksum error. Porting notes: * The ability to zinject decompression errors has been added, but this only works at the zio_decompress() level, where we have all of the info we need to match against the user's zinject options. * The decompress_fault test has been added to test the new zinject functionality * We attempted to set zio_decompress_fail_fraction to (1 << 18) in ztest for further test coverage. Although this did uncover a few low priority issues, this unfortuantely also causes ztest to ASSERT in many locations where the code is working correctly since it is designed to fail on IO errors. Developers can manually set this variable with the '-o' option to find and debug issues. Authored by: Matt Ahrens <mahrens@delphix.com> Reviewed by: George Wilson <george.wilson@delphix.com> Reviewed by: Paul Dagnelie <pcd@delphix.com> Reviewed by: Pavel Zakharov <pavel.zakharov@delphix.com> Reviewed by: Brian Behlendorf <behlendorf1@llnl.gov> Approved by: Matt Ahrens <mahrens@delphix.com> Ported-by: Tom Caputi <tcaputi@datto.com> OpenZFS-issue: https://illumos.org/issues/9403 OpenZFS-commit: https://github.com/openzfs/openzfs/commit/fa98e487a9 Closes #7822
This commit is contained in:
committed by
Brian Behlendorf
parent
47ab01a18f
commit
c3bd3fb4ac
+41
-10
@@ -21,7 +21,7 @@
|
||||
/*
|
||||
* Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
|
||||
* Copyright (c) 2012, Joyent, Inc. All rights reserved.
|
||||
* Copyright (c) 2011, 2017 by Delphix. All rights reserved.
|
||||
* Copyright (c) 2011, 2018 by Delphix. All rights reserved.
|
||||
* Copyright (c) 2014 by Saso Kiselkov. All rights reserved.
|
||||
* Copyright 2015 Nexenta Systems, Inc. All rights reserved.
|
||||
*/
|
||||
@@ -5784,10 +5784,12 @@ arc_getbuf_func(zio_t *zio, const zbookmark_phys_t *zb, const blkptr_t *bp,
|
||||
arc_buf_t **bufp = arg;
|
||||
|
||||
if (buf == NULL) {
|
||||
ASSERT(zio == NULL || zio->io_error != 0);
|
||||
*bufp = NULL;
|
||||
} else {
|
||||
ASSERT(zio == NULL || zio->io_error == 0);
|
||||
*bufp = buf;
|
||||
ASSERT(buf->b_data);
|
||||
ASSERT(buf->b_data != NULL);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -5915,12 +5917,6 @@ arc_read_done(zio_t *zio)
|
||||
&acb->acb_zb, acb->acb_private, acb->acb_encrypted,
|
||||
acb->acb_compressed, acb->acb_noauth, B_TRUE,
|
||||
&acb->acb_buf);
|
||||
if (error != 0) {
|
||||
(void) remove_reference(hdr, hash_lock,
|
||||
acb->acb_private);
|
||||
arc_buf_destroy_impl(acb->acb_buf);
|
||||
acb->acb_buf = NULL;
|
||||
}
|
||||
|
||||
/*
|
||||
* Assert non-speculative zios didn't fail because an
|
||||
@@ -5943,9 +5939,34 @@ arc_read_done(zio_t *zio)
|
||||
}
|
||||
}
|
||||
|
||||
if (zio->io_error == 0)
|
||||
if (error != 0) {
|
||||
/*
|
||||
* Decompression or decryption failed. Set
|
||||
* io_error so that when we call acb_done
|
||||
* (below), we will indicate that the read
|
||||
* failed. Note that in the unusual case
|
||||
* where one callback is compressed and another
|
||||
* uncompressed, we will mark all of them
|
||||
* as failed, even though the uncompressed
|
||||
* one can't actually fail. In this case,
|
||||
* the hdr will not be anonymous, because
|
||||
* if there are multiple callbacks, it's
|
||||
* because multiple threads found the same
|
||||
* arc buf in the hash table.
|
||||
*/
|
||||
zio->io_error = error;
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* If there are multiple callbacks, we must have the hash lock,
|
||||
* because the only way for multiple threads to find this hdr is
|
||||
* in the hash table. This ensures that if there are multiple
|
||||
* callbacks, the hdr is not anonymous. If it were anonymous,
|
||||
* we couldn't use arc_buf_destroy() in the error case below.
|
||||
*/
|
||||
ASSERT(callback_cnt < 2 || hash_lock != NULL);
|
||||
|
||||
hdr->b_l1hdr.b_acb = NULL;
|
||||
arc_hdr_clear_flags(hdr, ARC_FLAG_IO_IN_PROGRESS);
|
||||
if (callback_cnt == 0)
|
||||
@@ -5987,7 +6008,17 @@ arc_read_done(zio_t *zio)
|
||||
|
||||
/* execute each callback and free its structure */
|
||||
while ((acb = callback_list) != NULL) {
|
||||
if (acb->acb_done) {
|
||||
if (acb->acb_done != NULL) {
|
||||
if (zio->io_error != 0 && acb->acb_buf != NULL) {
|
||||
/*
|
||||
* If arc_buf_alloc_impl() fails during
|
||||
* decompression, the buf will still be
|
||||
* allocated, and needs to be freed here.
|
||||
*/
|
||||
arc_buf_destroy(acb->acb_buf,
|
||||
acb->acb_private);
|
||||
acb->acb_buf = NULL;
|
||||
}
|
||||
acb->acb_done(zio, &zio->io_bookmark, zio->io_bp,
|
||||
acb->acb_buf, acb->acb_private);
|
||||
}
|
||||
|
||||
+20
-17
@@ -21,7 +21,7 @@
|
||||
/*
|
||||
* Copyright (c) 2005, 2010, Oracle and/or its affiliates. All rights reserved.
|
||||
* Copyright 2011 Nexenta Systems, Inc. All rights reserved.
|
||||
* Copyright (c) 2012, 2017 by Delphix. All rights reserved.
|
||||
* Copyright (c) 2012, 2018 by Delphix. All rights reserved.
|
||||
* Copyright (c) 2013 by Saso Kiselkov. All rights reserved.
|
||||
* Copyright (c) 2014 Spectra Logic Corporation, All rights reserved.
|
||||
*/
|
||||
@@ -1191,25 +1191,26 @@ dbuf_read_done(zio_t *zio, const zbookmark_phys_t *zb, const blkptr_t *bp,
|
||||
ASSERT(refcount_count(&db->db_holds) > 0);
|
||||
ASSERT(db->db_buf == NULL);
|
||||
ASSERT(db->db.db_data == NULL);
|
||||
if (db->db_level == 0 && db->db_freed_in_flight) {
|
||||
/* we were freed in flight; disregard any error */
|
||||
if (buf == NULL) {
|
||||
buf = arc_alloc_buf(db->db_objset->os_spa,
|
||||
db, DBUF_GET_BUFC_TYPE(db), db->db.db_size);
|
||||
}
|
||||
if (buf == NULL) {
|
||||
/* i/o error */
|
||||
ASSERT(zio == NULL || zio->io_error != 0);
|
||||
ASSERT(db->db_blkid != DMU_BONUS_BLKID);
|
||||
ASSERT3P(db->db_buf, ==, NULL);
|
||||
db->db_state = DB_UNCACHED;
|
||||
} else if (db->db_level == 0 && db->db_freed_in_flight) {
|
||||
/* freed in flight */
|
||||
ASSERT(zio == NULL || zio->io_error == 0);
|
||||
arc_release(buf, db);
|
||||
bzero(buf->b_data, db->db.db_size);
|
||||
arc_buf_freeze(buf);
|
||||
db->db_freed_in_flight = FALSE;
|
||||
dbuf_set_data(db, buf);
|
||||
db->db_state = DB_CACHED;
|
||||
} else if (buf != NULL) {
|
||||
} else {
|
||||
/* success */
|
||||
ASSERT(zio == NULL || zio->io_error == 0);
|
||||
dbuf_set_data(db, buf);
|
||||
db->db_state = DB_CACHED;
|
||||
} else {
|
||||
ASSERT(db->db_blkid != DMU_BONUS_BLKID);
|
||||
ASSERT3P(db->db_buf, ==, NULL);
|
||||
db->db_state = DB_UNCACHED;
|
||||
}
|
||||
cv_broadcast(&db->db_changed);
|
||||
dbuf_rele_and_unlock(db, NULL, B_FALSE);
|
||||
@@ -2847,6 +2848,13 @@ dbuf_prefetch_indirect_done(zio_t *zio, const zbookmark_phys_t *zb,
|
||||
ASSERT3S(dpa->dpa_zb.zb_level, <, dpa->dpa_curlevel);
|
||||
ASSERT3S(dpa->dpa_curlevel, >, 0);
|
||||
|
||||
if (abuf == NULL) {
|
||||
ASSERT(zio == NULL || zio->io_error != 0);
|
||||
kmem_free(dpa, sizeof (*dpa));
|
||||
return;
|
||||
}
|
||||
ASSERT(zio == NULL || zio->io_error == 0);
|
||||
|
||||
/*
|
||||
* The dpa_dnode is only valid if we are called with a NULL
|
||||
* zio. This indicates that the arc_read() returned without
|
||||
@@ -2879,11 +2887,6 @@ dbuf_prefetch_indirect_done(zio_t *zio, const zbookmark_phys_t *zb,
|
||||
dbuf_rele(db, FTAG);
|
||||
}
|
||||
|
||||
if (abuf == NULL) {
|
||||
kmem_free(dpa, sizeof (*dpa));
|
||||
return;
|
||||
}
|
||||
|
||||
dpa->dpa_curlevel--;
|
||||
uint64_t nextblkid = dpa->dpa_zb.zb_blkid >>
|
||||
(dpa->dpa_epbs * (dpa->dpa_curlevel - dpa->dpa_zb.zb_level));
|
||||
|
||||
@@ -389,6 +389,9 @@ zio_decompress(zio_t *zio, abd_t *data, uint64_t size)
|
||||
zio->io_abd, tmp, zio->io_size, size);
|
||||
abd_return_buf_copy(data, tmp, size);
|
||||
|
||||
if (zio_injection_enabled && ret == 0)
|
||||
ret = zio_handle_fault_injection(zio, EINVAL);
|
||||
|
||||
if (ret != 0)
|
||||
zio->io_error = SET_ERROR(EIO);
|
||||
}
|
||||
|
||||
@@ -28,7 +28,7 @@
|
||||
*/
|
||||
|
||||
/*
|
||||
* Copyright (c) 2013, 2016 by Delphix. All rights reserved.
|
||||
* Copyright (c) 2013, 2018 by Delphix. All rights reserved.
|
||||
*/
|
||||
|
||||
#include <sys/zfs_context.h>
|
||||
@@ -37,6 +37,12 @@
|
||||
#include <sys/zio.h>
|
||||
#include <sys/zio_compress.h>
|
||||
|
||||
/*
|
||||
* If nonzero, every 1/X decompression attempts will fail, simulating
|
||||
* an undetected memory error.
|
||||
*/
|
||||
unsigned long zio_decompress_fail_fraction = 0;
|
||||
|
||||
/*
|
||||
* Compression vectors.
|
||||
*/
|
||||
@@ -148,5 +154,15 @@ zio_decompress_data(enum zio_compress c, abd_t *src, void *dst,
|
||||
int ret = zio_decompress_data_buf(c, tmp, dst, s_len, d_len);
|
||||
abd_return_buf(src, tmp, s_len);
|
||||
|
||||
/*
|
||||
* Decompression shouldn't fail, because we've already verifyied
|
||||
* the checksum. However, for extra protection (e.g. against bitflips
|
||||
* in non-ECC RAM), we handle this error (and test it).
|
||||
*/
|
||||
ASSERT0(ret);
|
||||
if (zio_decompress_fail_fraction != 0 &&
|
||||
spa_get_random(zio_decompress_fail_fraction) == 0)
|
||||
ret = SET_ERROR(EINVAL);
|
||||
|
||||
return (ret);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user