Improve arc_read() error reporting

Debugging reported NULL de-reference panic in dnode_hold_impl() I found
that for certain types of errors arc_read() may only return error code,
but not properly report it via done and pio arguments.  Lack of done
calls may result in reference and/or memory leaks in higher level code.
Lack of error reporting via pio may result in unnoticed errors there.
For example, dbuf_read(), where dbuf_read_impl() ignores arc_read()
return, relies completely on the pio mechanism and missed the errors.

This patch makes arc_read() to always call done callback and always
propagate errors to parent zio, if either is provided.

Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Reviewed-by: Brian Atkinson <batkinson@lanl.gov>
Signed-off-by:	Alexander Motin <mav@FreeBSD.org>
Sponsored by:	iXsystems, Inc.
Closes #14454
This commit is contained in:
Alexander Motin 2023-02-13 16:21:53 -05:00 committed by GitHub
parent 7883ea2234
commit 87a4dfa561
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -5958,6 +5958,7 @@ arc_read(zio_t *pio, spa_t *spa, const blkptr_t *bp,
(zio_flags & ZIO_FLAG_RAW_ENCRYPT) != 0; (zio_flags & ZIO_FLAG_RAW_ENCRYPT) != 0;
boolean_t embedded_bp = !!BP_IS_EMBEDDED(bp); boolean_t embedded_bp = !!BP_IS_EMBEDDED(bp);
boolean_t no_buf = *arc_flags & ARC_FLAG_NO_BUF; boolean_t no_buf = *arc_flags & ARC_FLAG_NO_BUF;
arc_buf_t *buf = NULL;
int rc = 0; int rc = 0;
ASSERT(!embedded_bp || ASSERT(!embedded_bp ||
@ -5987,7 +5988,7 @@ top:
if (!zfs_blkptr_verify(spa, bp, zio_flags & ZIO_FLAG_CONFIG_WRITER, if (!zfs_blkptr_verify(spa, bp, zio_flags & ZIO_FLAG_CONFIG_WRITER,
BLK_VERIFY_LOG)) { BLK_VERIFY_LOG)) {
rc = SET_ERROR(ECKSUM); rc = SET_ERROR(ECKSUM);
goto out; goto done;
} }
if (!embedded_bp) { if (!embedded_bp) {
@ -6008,14 +6009,13 @@ top:
if (hdr != NULL && HDR_HAS_L1HDR(hdr) && (HDR_HAS_RABD(hdr) || if (hdr != NULL && HDR_HAS_L1HDR(hdr) && (HDR_HAS_RABD(hdr) ||
(hdr->b_l1hdr.b_pabd != NULL && !encrypted_read))) { (hdr->b_l1hdr.b_pabd != NULL && !encrypted_read))) {
boolean_t is_data = !HDR_ISTYPE_METADATA(hdr); boolean_t is_data = !HDR_ISTYPE_METADATA(hdr);
arc_buf_t *buf = NULL;
if (HDR_IO_IN_PROGRESS(hdr)) { if (HDR_IO_IN_PROGRESS(hdr)) {
if (*arc_flags & ARC_FLAG_CACHED_ONLY) { if (*arc_flags & ARC_FLAG_CACHED_ONLY) {
mutex_exit(hash_lock); mutex_exit(hash_lock);
ARCSTAT_BUMP(arcstat_cached_only_in_progress); ARCSTAT_BUMP(arcstat_cached_only_in_progress);
rc = SET_ERROR(ENOENT); rc = SET_ERROR(ENOENT);
goto out; goto done;
} }
zio_t *head_zio = hdr->b_l1hdr.b_acb->acb_zio_head; zio_t *head_zio = hdr->b_l1hdr.b_acb->acb_zio_head;
@ -6144,9 +6144,7 @@ top:
ARCSTAT_CONDSTAT(!(*arc_flags & ARC_FLAG_PREFETCH), ARCSTAT_CONDSTAT(!(*arc_flags & ARC_FLAG_PREFETCH),
demand, prefetch, is_data, data, metadata, hits); demand, prefetch, is_data, data, metadata, hits);
*arc_flags |= ARC_FLAG_CACHED; *arc_flags |= ARC_FLAG_CACHED;
goto done;
if (done)
done(NULL, zb, bp, buf, private);
} else { } else {
uint64_t lsize = BP_GET_LSIZE(bp); uint64_t lsize = BP_GET_LSIZE(bp);
uint64_t psize = BP_GET_PSIZE(bp); uint64_t psize = BP_GET_PSIZE(bp);
@ -6159,10 +6157,10 @@ top:
int alloc_flags = encrypted_read ? ARC_HDR_ALLOC_RDATA : 0; int alloc_flags = encrypted_read ? ARC_HDR_ALLOC_RDATA : 0;
if (*arc_flags & ARC_FLAG_CACHED_ONLY) { if (*arc_flags & ARC_FLAG_CACHED_ONLY) {
rc = SET_ERROR(ENOENT);
if (hash_lock != NULL) if (hash_lock != NULL)
mutex_exit(hash_lock); mutex_exit(hash_lock);
goto out; rc = SET_ERROR(ENOENT);
goto done;
} }
if (hdr == NULL) { if (hdr == NULL) {
@ -6482,6 +6480,16 @@ out:
spa_read_history_add(spa, zb, *arc_flags); spa_read_history_add(spa, zb, *arc_flags);
spl_fstrans_unmark(cookie); spl_fstrans_unmark(cookie);
return (rc); return (rc);
done:
if (done)
done(NULL, zb, bp, buf, private);
if (pio && rc != 0) {
zio_t *zio = zio_null(pio, spa, NULL, NULL, NULL, zio_flags);
zio->io_error = rc;
zio_nowait(zio);
}
goto out;
} }
arc_prune_t * arc_prune_t *