From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932266AbWFSKet (ORCPT ); Mon, 19 Jun 2006 06:34:49 -0400 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S932368AbWFSKes (ORCPT ); Mon, 19 Jun 2006 06:34:48 -0400 Received: from 1wt.eu ([62.212.114.60]:64520 "EHLO 1wt.eu") by vger.kernel.org with ESMTP id S932266AbWFSKes (ORCPT ); Mon, 19 Jun 2006 06:34:48 -0400 Date: Mon, 19 Jun 2006 12:31:14 +0200 From: Willy Tarreau To: Grant Coady Cc: Marcelo Tosatti , linux-kernel@vger.kernel.org, Al Viro Subject: Re: Linux 2.4.33-rc1 Message-ID: <20060619103114.GA3855@1wt.eu> References: <20060618133718.GA2467@dmt> <20060618223736.GA4965@1wt.eu> <20060619040152.GB2678@1wt.eu> <20060619080651.GA3273@1wt.eu> <20060619092426.GC3472@1wt.eu> <9huc9217opa7sd26q5it13nvos9f9gg2in@4ax.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9huc9217opa7sd26q5it13nvos9f9gg2in@4ax.com> User-Agent: Mutt/1.5.11 Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Jun 19, 2006 at 08:27:29PM +1000, Grant Coady wrote: > On Mon, 19 Jun 2006 11:24:26 +0200, Willy Tarreau wrote: > > >On Mon, Jun 19, 2006 at 07:12:22PM +1000, Grant Coady wrote: > >> On Mon, 19 Jun 2006 10:06:51 +0200, Willy Tarreau wrote: > >> > >> >Hi Grant, > >> > > >> >OK, it does *really* crash in vfs_unlink(), during the double_up on > >> >dentry->inode-i_zombie (dentry->inode = NULL). > >> > > >> >I suggest the following fix, I hope that it is correct and is not subject > >> >to any race condition : > >> > > >> >--- ./fs/namei.c.orig 2006-06-19 09:39:52.000000000 +0200 > >> >+++ ./fs/namei.c 2006-06-19 09:51:09.000000000 +0200 > >> >@@ -1478,12 +1478,14 @@ > >> > int vfs_unlink(struct inode *dir, struct dentry *dentry) > >> > { > >> > int error; > >> >+ struct inode *inode; > >> > > >> > error = may_delete(dir, dentry, 0); > >> > if (error) > >> > return error; > >> > > >> >- double_down(&dir->i_zombie, &dentry->d_inode->i_zombie); > >> >+ inode = dentry->d_inode; > >> >+ double_down(&dir->i_zombie, &inode->i_zombie); > >> > error = -EPERM; > >> > if (dir->i_op && dir->i_op->unlink) { > >> > DQUOT_INIT(dir); > >> >@@ -1495,7 +1497,7 @@ > >> > unlock_kernel(); > >> > } > >> > } > >> >- double_up(&dir->i_zombie, &dentry->d_inode->i_zombie); > >> >+ double_up(&dir->i_zombie, &inode->i_zombie); > >> > if (!error) { > >> > d_delete(dentry); > >> > inode_dir_notify(dir, DN_DELETE); > >> > > >> >I think it will *not* oops anymore with this fix, but I'd like someone to > >> >review it to ensure that it is valid. > >> > >> Strangely, the /etc/lilo.conf~ is as expected on a different box, > >> 500MHz Celeron (Coppermine) + PATA HDD okay, whereas the Sempron > >> SktA 2600+ with SATA HDD has something munch a couple chars off > >> a filename during whatever vim does to make its backup file. > > > >I would not suspect the hardware. Instead, you should strace vim when it > >write the file : > > > > # strace -s 1000 -o /tmp/vim.trace vim /etc/lilo.conf > > > >Grep for "lilo.co" in it, I'm fairly sure that you will find "lilo.co~". > > stat64("/etc/lilo.conf", {st_mode=S_IFREG|0644, st_size=778, ...}) = 0 > stat64("/etc/lilo.conf", {st_mode=S_IFREG|0644, st_size=778, ...}) = 0 > stat64("/etc/lilo.conf", {st_mode=S_IFREG|0644, st_size=778, ...}) = 0 > access("/etc/lilo.conf", W_OK) = 0 > open("/etc/lilo.conf", O_RDONLY) = 3 > > ## munch a char: > stat64("/etc/lilo_con.swp", 0xbfffee8c) = -1 ENOENT (No such file or directory) > lstat64("/etc/lilo_con.swp", 0xbfffef0c) = -1 ENOENT (No such file or directory) > lstat64("/etc/lilo_con.swp", 0xbffff38c) = -1 ENOENT (No such file or directory) > open("/etc/lilo_con.swp", O_RDWR|O_CREAT|O_EXCL, 0600) = 4 > > ##munch another: > write(1, "\"/etc/lilo.conf\"", 16) = 16 > stat64("/etc/lilo.conf", {st_mode=S_IFREG|0644, st_size=778, ...}) = 0 > access("/etc/lilo.conf", W_OK) = 0 > lstat64("/etc/lilo.conf", {st_mode=S_IFREG|0644, st_size=778, ...}) = 0 > lstat64("/etc/lilo.conf", {st_mode=S_IFREG|0644, st_size=778, ...}) = 0 > stat64("/etc/lilo.conf", {st_mode=S_IFREG|0644, st_size=778, ...}) = 0 > unlink("/etc/lilo.co~") = 0 > rename("/etc/lilo.conf", "/etc/lilo.co~") = 0 > > > > If you want the whole trace (168k -> 26k gzipped). Not needed, it really seems that your vim does name the file like this on purpose. I see nothing abnormal right here. So possibly the kernel bug is fixed (but I'd like to get some reviewer comments). Thanks, Willy