On Thu, 27 Sep 2007, Andrew Morton wrote:
> On Thu, 27 Sep 2007 09:54:47 -0700 (PDT) Davide Libenzi <davidel@xxxxxxxxxxxxxxx> wrote:
>
> > On Thu, 27 Sep 2007, Davide Libenzi wrote:
> >
> > > On Wed, 26 Sep 2007, Andrew Morton wrote:
> > >
> > > > On Thu, 27 Sep 2007 10:30:50 +0800 "Yan Zheng" <yanzheng@xxxxxxxx> wrote:
> > > >
> > > > > Hello,
> > > > >
> > > > > igrab return NULL on error.
> > > > >
> > > > > Signed-off-by: Yan Zheng<yanzheng@xxxxxxxx>
> > > > > ---
> > > > > diff -ur linux-2.6.23-rc8/fs/anon_inodes.c linux/fs/anon_inodes.c
> > > > > --- linux-2.6.23-rc8/fs/anon_inodes.c 2007-09-27 10:05:07.000000000 +0800
> > > > > +++ linux/fs/anon_inodes.c 2007-09-27 10:18:26.000000000 +0800
> > > > > @@ -87,8 +87,8 @@
> > > > > return -ENFILE;
> > > > >
> > > > > inode = igrab(anon_inode_inode);
> > > > > - if (IS_ERR(inode)) {
> > > > > - error = PTR_ERR(inode);
> > > > > + if (!inode) {
> > > > > + error = -ENOENT;
> > > > > goto err_put_filp;
> > > > > }
> > > >
> > > > hm, does that code actually need to exist? igrab() is pretty expensive and
> > > > that fs is permanently mounted anyway...
> > >
> > > yes. The inode gets attached the the file*, and on its way out an iput()
> > > is done.
> >
> > Wait. You mean "is it worth checking the igrab() return code at all"?
>
> Well I was more thinking "can we just leave that inode's refcount alone all
> the time". I guess that would be tricky, but I think we can at least use
> the much less expensive __iget(), or an open-coded atomic_inc.
>
> inode_lock is a heavily-used singleton. I remain surprised that it hasn't
> bitten us in the ass yet.
Well, __iget() requires the inode lock to be held. Wrapping the anon
inodes code with a lock would be even worse.
Relying on the fact that the count will never go to zero and call __iget()
w/out held lock (to expoit the __iget fast path), is kinda nasty IMO.
Open coded atomic_inc()? Hmm, dunno...
- 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
