Re: [kvm-devel] [PATCH/RFC 1/2] anon-inodes: Remove fd_install()from anon_inode_getfd()

[Avi Kivity]

Davide Libenzi wrote:
> > I think that may be a bit cleaner than Al's approach, but it still
> > leaves the same trap that create_vcpu_fd() falls into.  The current
> > code is:
> >
> > static int create_vcpu_fd(struct kvm_vcpu *vcpu)
> > {
> > 	int fd, r;
> > 	struct inode *inode;
> > 	struct file *file;
> >
> > 	r = anon_inode_getfd(&fd, &inode, &file,
> > 			     "kvm-vcpu", &kvm_vcpu_fops, vcpu);
> > 	if (r)
> > 		return r;
> > 	atomic_inc(&vcpu->kvm->filp->f_count);
> > 	return fd;
> > }
> >
> > and with your proposal, the natural way to write that becomes:
> >
> > static int create_vcpu_fd(struct kvm_vcpu *vcpu)
> > {
> > 	int fd, r;
> >
> > 	r = anon_inode_getfd(&fd, NULL,
> > 			     "kvm-vcpu", &kvm_vcpu_fops, vcpu);
> > 	if (r)
> > 		return r;
> > 	atomic_inc(&vcpu->kvm->filp->f_count);
> > 	return fd;
> > }
> >     
>
> I don't know KVM code, but can't the "private_data" setup be completed 
> before calling anon_inode_getfd()?
>   

Creating the fd is the last thing done when creating a vcpu.

> Or ...
>
> static int create_vcpu_fd(struct kvm_vcpu *vcpu)
> {
> 	int fd, r;
>
> 	get_file(vcpu->kvm->filp);
> 	r = anon_inode_getfd(&fd, NULL,
> 			     "kvm-vcpu", &kvm_vcpu_fops, vcpu);
> 	if (r) {
> 		fput(vcpu->kvm->filp);
> 		return r;
> 	}
> 	return fd;
> }
>   

This seems reasonable.

--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/