On Thu, 28 Feb 2008, Roland Dreier wrote:
> > If we let the caller call fd_install(), then it may be messed up WRT
> > cleanup (fd, file, inode).
>
> Yes, that is a tiny bit tricky (need to call put_unused_fd() if you
> don't install the fd).
>
> > How about removing the inode pointer handout altogether, and *doing*
> > fd_install() inside anon_inode_getfd() like:
> >
> > if (pfile != NULL) {
> > get_file(file);
> > *pfile = file;
> > }
> > fd_install(fd, file);
> >
> > In this way, if the caller want the file* back, he gets the reference
> > bumped before fd_install().
>
> 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()?
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;
}
Hmm...?
- Davide
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
