![]() | |||||||||||||
Messages in this thread |
On Sat, Jun 17, 2006 at 04:24:00PM +1000, Grant Coady wrote: > On Sat, 17 Jun 2006 07:13:57 +0200, Willy Tarreau <w@1wt.eu> wrote: > > >Hi Grant, > > > >On Sat, Jun 17, 2006 at 09:24:02AM +1000, Grant Coady wrote: > >> On Sat, 17 Jun 2006 00:38:10 +0200, "Michal Piotrowski" <michal.k.k.piotrowski@gmail.com> wrote: > >> > >> >Can you revert 42cce987d63d3048595b8b00d74be786707d5e5d commit? > >> > >> Only if you point me at the patch, that string seems not to be > >> in the patch-2.4.33-rc1.gz file. > > > >First, revert this one : > > > > http://kernel.org/git/?p=linux/kernel/git/marcelo/linux-2.4.git;a=commitdiff_plain;h=efc95599c0261dd7ab3a1d9071024ca140b4c644;hp=6601095e2de35f00325a33c8be6b548f81fe76d5 > > > >--- a/fs/namei.c > >+++ b/fs/namei.c > >@@ -1479,19 +1479,20 @@ int vfs_unlink(struct inode *dir, struct > > { > > int error; > > > >- double_down(&dir->i_zombie, &dentry->d_inode->i_zombie); > > error = may_delete(dir, dentry, 0); > >- if (!error) { > >- error = -EPERM; > >- if (dir->i_op && dir->i_op->unlink) { > >- DQUOT_INIT(dir); > >- if (d_mountpoint(dentry)) > >- error = -EBUSY; > >- else { > >- lock_kernel(); > >- error = dir->i_op->unlink(dir, dentry); > >- unlock_kernel(); > >- } > >+ if (error) > >+ return error; > >+ > >+ double_down(&dir->i_zombie, &dentry->d_inode->i_zombie); > >+ error = -EPERM; > >+ if (dir->i_op && dir->i_op->unlink) { > >+ DQUOT_INIT(dir); > >+ if (d_mountpoint(dentry)) > >+ error = -EBUSY; > >+ else { > >+ lock_kernel(); > >+ error = dir->i_op->unlink(dir, dentry); > >+ unlock_kernel(); > > } > > } > > double_up(&dir->i_zombie, &dentry->d_inode->i_zombie); > > > > > >It will put you back to the state where all your machines hanged at boot > >with -hf32.5, then revert this one : > > > > http://kernel.org/git/?p=linux/kernel/git/marcelo/linux-2.4.git;a=commitdiff_plain;h=f41e0ce901260d3d1ae5bd8bae34266891b4a65d;hp=925c7ce0a2d9a676cd8e4a2baf411b23cf6762d6 > > > >--- a/fs/namei.c > >+++ b/fs/namei.c > >@@ -1479,7 +1479,7 @@ int vfs_unlink(struct inode *dir, struct > > { > > int error; > > > >- down(&dir->i_zombie); > >+ double_down(&dir->i_zombie, &dentry->d_inode->i_zombie); > > error = may_delete(dir, dentry, 0); > > if (!error) { > > error = -EPERM; > >@@ -1491,14 +1491,14 @@ int vfs_unlink(struct inode *dir, struct > > lock_kernel(); > > error = dir->i_op->unlink(dir, dentry); > > unlock_kernel(); > >- if (!error) > >- d_delete(dentry); > > } > > } > > } > >- up(&dir->i_zombie); > >- if (!error) > >+ double_up(&dir->i_zombie, &dentry->d_inode->i_zombie); > >+ if (!error) { > >+ d_delete(dentry); > > inode_dir_notify(dir, DN_DELETE); > >+ } > > return error; > > } > > > >@@ -1607,18 +1607,19 @@ int vfs_link(struct dentry *old_dentry, > > struct inode *inode; > > int error; > > > >- down(&dir->i_zombie); > > error = -ENOENT; > > inode = old_dentry->d_inode; > > if (!inode) > >- goto exit_lock; > >- > >- error = may_create(dir, new_dentry); > >- if (error) > >- goto exit_lock; > >+ goto exit; > > > > error = -EXDEV; > > if (dir->i_dev != inode->i_dev) > >+ goto exit; > >+ > >+ double_down(&dir->i_zombie, &old_dentry->d_inode->i_zombie); > >+ > >+ error = may_create(dir, new_dentry); > >+ if (error) > > goto exit_lock; > > > > /* > >@@ -1636,9 +1637,10 @@ int vfs_link(struct dentry *old_dentry, > > unlock_kernel(); > > > > exit_lock: > >- up(&dir->i_zombie); > >+ double_up(&dir->i_zombie, &old_dentry->d_inode->i_zombie); > > if (!error) > > inode_dir_notify(dir, DN_CREATE); > >+exit: > > return error; > > } > > > > > >And you will have something equivalent to -hf32.6 (you remember, I only > >reverted the fix in -hf32.6, and did not merge Marcelo's fix for the fix). > >Then we will have to decide whether we can fix it again or revert it > >completely. I would have liked to Cc: Vadim Egorov, but I cannot find his > >email. > > > > Hi Willy, this worked. OK Fine. However, I don't understand. From you oops, it looks like dentry->d_inode suddenly gets NULL before calling this line : double_up(&dir->i_zombie, &dentry->d_inode->i_zombie); I'm wondering if the unlink(dir, dentry) above can produce this. If this is the case, the Vadim's fix is terribly broken. I also think that in vfs_link(), we can encounter the same problem that Marcelo had to fix, because double_down() is performed without prior checking that old_dentry->d_inode is valid. Marcelo, do you have Vadim's email somewhere ? I think he should help us on this otherwise it would be better to revert his fix, as it has introduced lots of sensible changes in the error path. > Grant. Thanks for the tests, Grant. Willy - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ | ||||||||||||
| Last update: 2006-06-17 09:14 [from the cache] ©2003-2008 | |||||||||||||