On Tuesday 24 June 2008 00:08, Roland Dreier wrote:
> > +static struct inode * xrc_fd2inode(unsigned int fd)
> > +{
> > + struct file * f = fget(fd);
> > +
> > + if (!f)
> > + return NULL;
> > +
> > + return f->f_dentry->d_inode;
> > +}
>
> Am I missing something, or is there no fput() that matches this fget()?
You're right. I'm adding this after I grab the inode and in the error path
in procedure ib_uverbs_open_xrc_domain().
> Is there some reason why we don't need an igrab() of the inode we're
> using here? (If we do need it then a corresponding iput() when we're
> done with the inode is required of course)
We do need igrab/iput. The algorithm here is to use the fd to get to the inode,
and then grab the inode (to guarantee that it is not deleted out from under).
Once I have grabbed the inode, I don't need the fd anymore (so that I can
fput()).
I am using igrab/iput in the patch (easy to miss):
+static int insert_xrcd(struct ib_device *dev, struct inode *i_n,
+ struct ib_xrcd *xrcd)
+{
+ int ret;
+
+ ret = xrcd_table_insert(dev, i_n, xrcd);
+ if (!ret)
+ igrab(i_n);
+
+ return ret;
+}
...
+static void xrcd_table_delete(struct ib_device *dev,
+ struct inode *i_n)
+{
+ struct xrcd_table_entry *entry = xrcd_table_search(dev, i_n);
+
+ if (entry) {
+ iput(i_n);
+ rb_erase(&entry->node, &dev->ib_uverbs_xrcd_table);
+ kfree(entry);
+ }
+}
At second look, its still a bit messy. I can get rid of insert_xrcd entirely,
and just move the igrab to xrcd_table_insert.
(as is done for xrcd_table_delete/iput).
- Jack
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
