Re: [PATCH 9/9] xfs: connect in-memory btrees to xfiles

Darrick J. Wong
2024-01-03T02:04:50Z

On Mon, Jan 01, 2024 at 12:18:48AM +0000, Matthew Wilcox wrote:
> On Sun, Dec 31, 2023 at 12:15:54PM -0800, Darrick J. Wong wrote:
> > +/* Ensure that there is storage backing the given range. */
> > +int
> > +xfile_prealloc(
> > + struct xfile *xf,
> > + loff_t pos,
> > + u64 count)
> > +{
> > + struct inode *inode = file_inode(xf->file);
> > + struct address_space *mapping = inode->i_mapping;
> > + const struct address_space_operations *aops = mapping->a_ops;
> > + struct page *page = NULL;
> > + unsigned int pflags;
> > + int error = 0;
> > +
> > + if (count > MAX_RW_COUNT)
> > + return -E2BIG;
> > + if (inode->i_sb->s_maxbytes - pos < count)
> > + return -EFBIG;
> > +
> > + trace_xfile_prealloc(xf, pos, count);
> > +
> > + pflags = memalloc_nofs_save();
> > + while (count > 0) {
> > + void *fsdata = NULL;
> > + unsigned int len;
> > + int ret;
> > +
> > + len = min_t(ssize_t, count, PAGE_SIZE - offset_in_page(pos));
> > +
> > + /*
> > + * We call write_begin directly here to avoid all the freezer
> > + * protection lock-taking that happens in the normal path.
> > + * shmem doesn't support fs freeze, but lockdep doesn't know
> > + * that and will trip over that.
> > + */
> > + error = aops->write_begin(NULL, mapping, pos, len, &page,
> > + &fsdata);
> > + if (error)
> > + break;
> > +
> > + /*
> > + * xfile pages must never be mapped into userspace, so we skip
> > + * the dcache flush. If the page is not uptodate, zero it to
> > + * ensure we never go lacking for space here.
> > + */
> > + if (!PageUptodate(page)) {
> > + void *kaddr = kmap_local_page(page);
> > +
> > + memset(kaddr, 0, PAGE_SIZE);
> > + SetPageUptodate(page);
> > + kunmap_local(kaddr);
> > + }
>
> Does the xfiles implementation prevent THPs from being created?
> If not, this could lead to an entire THP being marked uptodate even
> though we've only zeroed one page of it.

No. How does one prevent THPs from being created for a specific tmpfs
file? It's probably time you and I burned a 1x1 on straightening out
some of xfile.c's folio-idiocy. ;)

--D