On Sun, Dec 31, 2023 at 12:15:54PM -0800, Darrick J. Wong wrote: > From: Darrick J. Wong <djwong@xxxxxxxxxx> > > Add to our stubbed-out in-memory btrees the ability to connect them with > an actual in-memory backing file (aka xfiles) and the necessary pieces > to track free space in the xfile and flush dirty xfbtree buffers on > demand, which we'll need for online repair. I guess this is is split from the last patch because of the size of the changes? Because it feels they relong belong together. Maybe with my patches for the diet and splitting out the new helpers outside the btree code these could now become a single commit? > +#ifdef CONFIG_XFS_BTREE_IN_XFILE > +static inline unsigned long > +xfbtree_ino( > + struct xfbtree *xfbt) > +{ > + return file_inode(xfbt->target->bt_xfile->file)->i_ino; > +} > +#endif /* CONFIG_XFS_BTREE_IN_XFILE */ This should probably move to xfile.h? > + /* Make sure we actually can write to the block before we return it. */ > + pos = xfo_to_b(bt_xfoff); > + error = xfile_prealloc(xfbtree_xfile(xfbt), pos, xfo_to_b(1)); > + if (error) > + return error; IFF we stick to always backing the buffers directly by the shmem pages this won't be needed - the btree code does a buf_get right after calling into ->alloc_blocks that will allocate the page. > +int > +xfbtree_free_block( > + struct xfs_btree_cur *cur, > + struct xfs_buf *bp) > +{ > + struct xfbtree *xfbt = cur->bc_mem.xfbtree; > + xfileoff_t bt_xfoff, bt_xflen; > + > + ASSERT(cur->bc_flags & XFS_BTREE_IN_XFILE); > + > + bt_xfoff = xfs_daddr_to_xfot(xfs_buf_daddr(bp)); > + bt_xflen = xfs_daddr_to_xfot(bp->b_length); > + > + trace_xfbtree_free_block(xfbt, cur, bt_xfoff); > + > + return xfboff_bitmap_set(&xfbt->freespace, bt_xfoff, bt_xflen); Any reason this doesn't actually remove the page from shmem? > +int > +xfbtree_trans_commit( > + struct xfbtree *xfbt, > + struct xfs_trans *tp) > +{ > + LIST_HEAD(buffer_list); > + struct xfs_log_item *lip, *n; > + bool corrupt = false; > + bool tp_dirty = false; Can we have some sort of flag on the xfs_trans structure that marks it as fake for xfbtree, and assert it gets fed here, and add ansother assert it desn't get fed to xfs_trans_commit/cancel? > +/* Discard pages backing a range of the xfile. */ > +void > +xfile_discard( > + struct xfile *xf, > + loff_t pos, > + u64 count) > +{ > + trace_xfile_discard(xf, pos, count); > + shmem_truncate_range(file_inode(xf->file), pos, pos + count - 1); > +} This doesn't end up being used. > +/* Ensure that there is storage backing the given range. */ > +int > +xfile_prealloc( > + struct xfile *xf, > + loff_t pos, > + u64 count) If we end up needing this somewhere else in the end (and it really should be a separate patch), we should be able to replace it with a simple xfile_get_page/xfile_put_page pair.