On 8/8/2025 3:04 AM, Ackerley Tng wrote: > David Hildenbrand <david@xxxxxxxxxx> writes: > >> On 13.07.25 19:43, Shivank Garg wrote: >>> From: Ackerley Tng <ackerleytng@xxxxxxxxxx> >>> >>> + 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. >>> + 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. > >>> + >>> >> >> 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
