Re: [PATCH 1/9] xfs: dump xfiles for debugging purposes

Matthew Wilcox
2024-01-01T00:02:59Z

On Sun, Dec 31, 2023 at 12:13:49PM -0800, Darrick J. Wong wrote:
> + error = xfile_stat(xf, &sb);
> + if (error)
> + return error;
> +
> + printk(KERN_ALERT "xfile ino 0x%lx isize 0x%llx dump:", inode->i_ino,
> + sb.size);
> + pflags = memalloc_nofs_save();

Hm, why? What makes it a bad idea to call back into the filesysteam at
this point?

> + page = shmem_read_mapping_page_gfp(mapping,
> + datapos >> PAGE_SHIFT, __GFP_NOWARN);

This GFP flag looks wrong. Why can't we use GFP_KERNEL here?

I'm also not thrilled about the use of page APIs instead of folio APIs,
but given how long this patchset has been in development, I understand why
you didn't start out with folio APIs. It's not a blocker by any means.

I can come through and convert it later when I decide that it's finally
time to get rid of shmem_read_mapping_page_gfp(), which is going to take
a big gulp because it now means touching GPU drivers ...