On Mon, Nov 27, 2023 at 02:56:31PM -0800, Darrick J. Wong wrote: > On Fri, Nov 24, 2023 at 09:53:38PM -0800, Christoph Hellwig wrote: > > > @@ -480,7 +500,7 @@ xfs_btree_bload_node( > > > > > > ASSERT(!xfs_btree_ptr_is_null(cur, child_ptr)); > > > > > > - ret = xfs_btree_get_buf_block(cur, child_ptr, &child_block, > > > + ret = xfs_btree_read_buf_block(cur, child_ptr, 0, &child_block, > > > &child_bp); > > > > How is this (and making xfs_btree_read_buf_block outside of xfs_buf.c) > > related to the dirty limit? > > Oh! Looking through my notes, I wanted the /new/ btree block buffers > have the same lru reference count the old ones did. I probably should > have exported xfs_btree_set_refs instead of reading whatever is on disk > into a buffer, only to blow away the contents anyway. And now that I've dug further through my notes, I've realized that there's a better reason for this unexplained _get_buf -> _read_buf transition and the setting of XBF_DONE in _delwri_queue_here. This patch introduces the behavior that we flush the delwri list to disk every 256k. Flushing the buffers releases them, which means that reclaim could free the buffer before xfs_btree_bload_node needs it again to build the next level up. If that's the case, then _get_buf will get us a !DONE buffer with zeroes instead of reading the (freshly written) buffer back in from disk. We'll then end up formatting garbage keys into the node block, which is bad. /* * Read the lower-level block in case the buffer for it has * been reclaimed. LRU refs will be set on the block, which is * desirable if the new btree commits. */ ret = xfs_btree_read_buf_block(cur, child_ptr, 0, &child_block, &child_bp); The behavior of setting XBF_DONE in xfs_buf_delwri_queue_here is an optimization if _delwri_submit releases the buffer and it is /not/ reclaimed. In that case, xfs_btree_read_buf_block will find the buffer without the DONE flag set and reread the contents from disk, which is unnecessary. I'll split those changes into separate patches with fuller explanations of what's going on. --D > --D >
