mirror of
				https://git.proxmox.com/git/mirror_zfs.git
				synced 2025-10-26 18:05:04 +03:00 
			
		
		
		
	Improve dbuf_read() error reporting
Previous code reported non-ZIO errors only via return value, but not via parent ZIO. It could cause NULL-dereference panics due to dmu_buf_hold_array_by_dnode() ignoring the return value, relying solely on parent ZIO status. Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov> Reviewed-by: Ameer Hamza <ahamza@ixsystems.com> Reported by: Ameer Hamza <ahamza@ixsystems.com> Signed-off-by: Alexander Motin <mav@FreeBSD.org> Sponsored by: iXsystems, Inc. Closes #16042
This commit is contained in:
		
							parent
							
								
									39993c3dfe
								
							
						
					
					
						commit
						d5fb6abd36
					
				@ -1542,17 +1542,14 @@ dbuf_read_verify_dnode_crypt(dmu_buf_impl_t *db, uint32_t flags)
 | 
			
		||||
 * returning.
 | 
			
		||||
 */
 | 
			
		||||
static int
 | 
			
		||||
dbuf_read_impl(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags,
 | 
			
		||||
dbuf_read_impl(dmu_buf_impl_t *db, dnode_t *dn, zio_t *zio, uint32_t flags,
 | 
			
		||||
    db_lock_type_t dblt, const void *tag)
 | 
			
		||||
{
 | 
			
		||||
	dnode_t *dn;
 | 
			
		||||
	zbookmark_phys_t zb;
 | 
			
		||||
	uint32_t aflags = ARC_FLAG_NOWAIT;
 | 
			
		||||
	int err, zio_flags;
 | 
			
		||||
	blkptr_t bp, *bpp;
 | 
			
		||||
 | 
			
		||||
	DB_DNODE_ENTER(db);
 | 
			
		||||
	dn = DB_DNODE(db);
 | 
			
		||||
	ASSERT(!zfs_refcount_is_zero(&db->db_holds));
 | 
			
		||||
	ASSERT(MUTEX_HELD(&db->db_mtx));
 | 
			
		||||
	ASSERT(db->db_state == DB_UNCACHED || db->db_state == DB_NOFILL);
 | 
			
		||||
@ -1627,8 +1624,6 @@ dbuf_read_impl(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags,
 | 
			
		||||
	if (err != 0)
 | 
			
		||||
		goto early_unlock;
 | 
			
		||||
 | 
			
		||||
	DB_DNODE_EXIT(db);
 | 
			
		||||
 | 
			
		||||
	db->db_state = DB_READ;
 | 
			
		||||
	DTRACE_SET_STATE(db, "read issued");
 | 
			
		||||
	mutex_exit(&db->db_mtx);
 | 
			
		||||
@ -1653,12 +1648,11 @@ dbuf_read_impl(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags,
 | 
			
		||||
	 * parent's rwlock, which would be a lock ordering violation.
 | 
			
		||||
	 */
 | 
			
		||||
	dmu_buf_unlock_parent(db, dblt, tag);
 | 
			
		||||
	(void) arc_read(zio, db->db_objset->os_spa, bpp,
 | 
			
		||||
	return (arc_read(zio, db->db_objset->os_spa, bpp,
 | 
			
		||||
	    dbuf_read_done, db, ZIO_PRIORITY_SYNC_READ, zio_flags,
 | 
			
		||||
	    &aflags, &zb);
 | 
			
		||||
	return (err);
 | 
			
		||||
	    &aflags, &zb));
 | 
			
		||||
 | 
			
		||||
early_unlock:
 | 
			
		||||
	DB_DNODE_EXIT(db);
 | 
			
		||||
	mutex_exit(&db->db_mtx);
 | 
			
		||||
	dmu_buf_unlock_parent(db, dblt, tag);
 | 
			
		||||
	return (err);
 | 
			
		||||
@ -1743,7 +1737,7 @@ dbuf_fix_old_data(dmu_buf_impl_t *db, uint64_t txg)
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
int
 | 
			
		||||
dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)
 | 
			
		||||
dbuf_read(dmu_buf_impl_t *db, zio_t *pio, uint32_t flags)
 | 
			
		||||
{
 | 
			
		||||
	int err = 0;
 | 
			
		||||
	boolean_t prefetch;
 | 
			
		||||
@ -1759,7 +1753,7 @@ dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)
 | 
			
		||||
	dn = DB_DNODE(db);
 | 
			
		||||
 | 
			
		||||
	prefetch = db->db_level == 0 && db->db_blkid != DMU_BONUS_BLKID &&
 | 
			
		||||
	    (flags & DB_RF_NOPREFETCH) == 0 && dn != NULL;
 | 
			
		||||
	    (flags & DB_RF_NOPREFETCH) == 0;
 | 
			
		||||
 | 
			
		||||
	mutex_enter(&db->db_mtx);
 | 
			
		||||
	if (flags & DB_RF_PARTIAL_FIRST)
 | 
			
		||||
@ -1806,13 +1800,13 @@ dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)
 | 
			
		||||
 | 
			
		||||
		db_lock_type_t dblt = dmu_buf_lock_parent(db, RW_READER, FTAG);
 | 
			
		||||
 | 
			
		||||
		if (zio == NULL && (db->db_state == DB_NOFILL ||
 | 
			
		||||
		if (pio == NULL && (db->db_state == DB_NOFILL ||
 | 
			
		||||
		    (db->db_blkptr != NULL && !BP_IS_HOLE(db->db_blkptr)))) {
 | 
			
		||||
			spa_t *spa = dn->dn_objset->os_spa;
 | 
			
		||||
			zio = zio_root(spa, NULL, NULL, ZIO_FLAG_CANFAIL);
 | 
			
		||||
			pio = zio_root(spa, NULL, NULL, ZIO_FLAG_CANFAIL);
 | 
			
		||||
			need_wait = B_TRUE;
 | 
			
		||||
		}
 | 
			
		||||
		err = dbuf_read_impl(db, zio, flags, dblt, FTAG);
 | 
			
		||||
		err = dbuf_read_impl(db, dn, pio, flags, dblt, FTAG);
 | 
			
		||||
		/*
 | 
			
		||||
		 * dbuf_read_impl has dropped db_mtx and our parent's rwlock
 | 
			
		||||
		 * for us
 | 
			
		||||
@ -1833,9 +1827,10 @@ dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)
 | 
			
		||||
		 */
 | 
			
		||||
		if (need_wait) {
 | 
			
		||||
			if (err == 0)
 | 
			
		||||
				err = zio_wait(zio);
 | 
			
		||||
				err = zio_wait(pio);
 | 
			
		||||
			else
 | 
			
		||||
				VERIFY0(zio_wait(zio));
 | 
			
		||||
				(void) zio_wait(pio);
 | 
			
		||||
			pio = NULL;
 | 
			
		||||
		}
 | 
			
		||||
	} else {
 | 
			
		||||
		/*
 | 
			
		||||
@ -1862,7 +1857,7 @@ dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)
 | 
			
		||||
				ASSERT(db->db_state == DB_READ ||
 | 
			
		||||
				    (flags & DB_RF_HAVESTRUCT) == 0);
 | 
			
		||||
				DTRACE_PROBE2(blocked__read, dmu_buf_impl_t *,
 | 
			
		||||
				    db, zio_t *, zio);
 | 
			
		||||
				    db, zio_t *, pio);
 | 
			
		||||
				cv_wait(&db->db_changed, &db->db_mtx);
 | 
			
		||||
			}
 | 
			
		||||
			if (db->db_state == DB_UNCACHED)
 | 
			
		||||
@ -1871,6 +1866,13 @@ dbuf_read(dmu_buf_impl_t *db, zio_t *zio, uint32_t flags)
 | 
			
		||||
		}
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	if (pio && err != 0) {
 | 
			
		||||
		zio_t *zio = zio_null(pio, pio->io_spa, NULL, NULL, NULL,
 | 
			
		||||
		    ZIO_FLAG_CANFAIL);
 | 
			
		||||
		zio->io_error = err;
 | 
			
		||||
		zio_nowait(zio);
 | 
			
		||||
	}
 | 
			
		||||
 | 
			
		||||
	return (err);
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
 | 
			
		||||
		Loading…
	
		Reference in New Issue
	
	Block a user