On Fri, Jul 09, 2021 at 01:50:13PM +0000, Wei Liu wrote: > On Fri, Jul 09, 2021 at 02:02:04PM +0100, Matthew Wilcox wrote: > > On Fri, Jul 09, 2021 at 11:43:38AM +0000, Wei Liu wrote: > > > +static long > > > +mshv_partition_ioctl_create_device(struct mshv_partition *partition, > > > + void __user *user_args) > > > +{ > > [...] > > > + mshv_partition_get(partition); > > > + r = anon_inode_getfd(ops->name, &mshv_device_fops, dev, O_RDWR | O_CLOEXEC); > > > + if (r < 0) { > > > + mshv_partition_put_no_destroy(partition); > > > + list_del(&dev->partition_node); > > > + ops->destroy(dev); > > > + goto out; > > > + } > > > + > > > + cd->fd = r; > > > + r = 0; > > > > Why return the fd in memory instead of returning the fd as the return > > value from the ioctl? > > > > > + if (copy_to_user(user_args, &tmp, sizeof(tmp))) { > > > + r = -EFAULT; > > > + goto out; > > > + } > > > > ... this could then disappear. > > Thanks for your comment, Matthew. > > This is intentionally because I didn't want to deviate from KVM's API. > The fewer differences the better. Then don't define your own structure. Use theirs.