On 8/8/2025 3:04 AM, Ackerley Tng wrote:

David Hildenbrand [off-list ref] writes:

quoted

On 13.07.25 19:43, Shivank Garg wrote:

quoted

From: Ackerley Tng <redacted>

quoted

quoted

+	ctx->ops = &kvm_gmem_super_operations;

Curious, why is that required? (secretmem doesn't have it, so I wonder)

Good point! pseudo_fs_fill_super() fills in a struct super_operations
which already does simple_statfs, so guest_memfd doesn't need this.

Right, simple_statfs isn't strictly needed in this patch, but the
super_operations is required for the subsequent patches in
the series which add custom alloc_inode, destroy_inode, and free_inode
callback.

quoted

quoted

+	if (!try_module_get(kvm_gmem_fops.owner))
+		goto err;

Curious, shouldn't there be a module_put() somewhere after this function 
returned a file?

This was interesting indeed, but IIUC this is correct.

I think this flow was basically copied from __anon_inode_getfile(),
which does this try_module_get().

The corresponding module_put() is in __fput(), which calls fops_put()
and calls module_put() on the owner.

quoted

quoted

+

Nothing else jumped at me.

Thanks for the review!

Since we're going to submit this patch through Shivank's mempolicy
support series, I'll follow up soon by sending a replacement patch in
reply to this series so Shivank could build on top of that?

yes, I'll post the V10 soon.

Thanks,
Shivank