Re: Linux 2.4.33-rc1

Willy Tarreau
2006-06-20T22:31:37Z

On Mon, Jun 19, 2006 at 08:45:06PM -0300, Marcelo Tosatti wrote:
> >
> > ---- from here ----
> >
> >
> > > + inode = dentry->d_inode;
> > > + if (inode)
> > > + atomic_inc(&inode->i_count);
> > > error = vfs_unlink(nd.dentry->d_inode, dentry);
> > > exit2:
> > > dput(dentry);
> > > }
> > > up(&nd.dentry->d_inode->i_sem);
> > > + if (inode)
> > > + iput(inode);
> >
> > ---- to here ----
> >
> > I believe that nd.dentry->d_inode cannot vanish because it is protected by the
> > down(->i_sem) before and the up(->i_sem) after. Am I right or am I missing
> > something important ?
>
> Indeed it can't, but dentry->d_inode will be set to NULL by
> nfs_unlink->nfs_safe_remove->d_delete. Thus the problem.

What puzzles me is how are we supposed to up(&nd.dentry->d_inode->i_sem) if
dentry->d_inode can become NULL ? simply by keeping a copy of it ? I thought
that the down() protected the whole thing, but may be that's stupid anyway.
I've been running rc1 without this patch for a few hours and during kernel
compiles without a problem, so I'm not sure about what to think about the
other changes which were apparently harmless too :-/

Well, if I resume it right, we only need to merge your patch and mine and
it *should* be OK.

BTW, I've been reviewing the PaX patch and found *at least* one patch
that should be merged (fix for oops). I'll send it separately, and it's
queued in -upstream.

Cheers,
Willy