On Sat, Nov 04, 2023, Xu Yilun wrote:
> > +KVM_SET_USER_MEMORY_REGION2 is an extension to KVM_SET_USER_MEMORY_REGION that
> > +allows mapping guest_memfd memory into a guest. All fields shared with
> > +KVM_SET_USER_MEMORY_REGION identically. Userspace can set KVM_MEM_PRIVATE in
> > +flags to have KVM bind the memory region to a given guest_memfd range of
> > +[guest_memfd_offset, guest_memfd_offset + memory_size]. The target guest_memfd
> ^
> The range end should be exclusive, is it?
Yes, that should be a ')', not a ']'.
> > +static int __kvm_gmem_create(struct kvm *kvm, loff_t size, u64 flags)
> > +{
> > + const char *anon_name = "[kvm-gmem]";
> > + struct kvm_gmem *gmem;
> > + struct inode *inode;
> > + struct file *file;
> > + int fd, err;
> > +
> > + fd = get_unused_fd_flags(0);
> > + if (fd < 0)
> > + return fd;
> > +
> > + gmem = kzalloc(sizeof(*gmem), GFP_KERNEL);
> > + if (!gmem) {
> > + err = -ENOMEM;
> > + goto err_fd;
> > + }
> > +
> > + /*
> > + * Use the so called "secure" variant, which creates a unique inode
> > + * instead of reusing a single inode. Each guest_memfd instance needs
> > + * its own inode to track the size, flags, etc.
> > + */
> > + file = anon_inode_getfile_secure(anon_name, &kvm_gmem_fops, gmem,
> > + O_RDWR, NULL);
> > + if (IS_ERR(file)) {
> > + err = PTR_ERR(file);
> > + goto err_gmem;
> > + }
> > +
> > + file->f_flags |= O_LARGEFILE;
> > +
> > + inode = file->f_inode;
> > + WARN_ON(file->f_mapping != inode->i_mapping);
>
> Just curious, why should we check the mapping fields which is garanteed in
> other subsystem?
Mostly to document the behavior. The vast majority of folks that read this code
will be KVM developers, not file systems developers, and will likely have no clue
about the relationship between f_mapping and i_mapping. And in the extremely
unlikely scenario that anon_inode_getfile_secure() no longer sets f_mapping, a
WARN detects the issue whereas a comment does not.
