Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id ; Sun, 6 Jan 2002 22:55:05 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id ; Sun, 6 Jan 2002 22:54:56 -0500 Received: from vasquez.zip.com.au ([203.12.97.41]:39954 "EHLO vasquez.zip.com.au") by vger.kernel.org with ESMTP id ; Sun, 6 Jan 2002 22:54:43 -0500 Message-ID: <3C391AB1.21F8F48C@zip.com.au> Date: Sun, 06 Jan 2002 19:49:05 -0800 From: Andrew Morton X-Mailer: Mozilla 4.77 [en] (X11; U; Linux 2.4.17-pre8 i686) X-Accept-Language: en MIME-Version: 1.0 To: Andrea Arcangeli CC: Alexander Viro , Linus Torvalds , torrey.hoffman@myrio.com, linux-kernel@vger.kernel.org, Marcelo Tosatti , "Stephen C. Tweedie" , Trond Myklebust Subject: Re: ramdisk corruption problems - was: RE: pivot_root and initrd kern el panic woes In-Reply-To: <3C2EB208.B2BA7CBF@zip.com.au> , ; <20011231010537.K1356@athlon.random> <3C36E6E8.628BF0BF@zip.com.au>, <3C36E6E8.628BF0BF@zip.com.au>; from akpm@zip.com.au on Sat, Jan 05, 2002 at 03:43:36AM -0800 <20020107040828.H1561@athlon.random> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org X-Mailing-List: linux-kernel@vger.kernel.org Andrea Arcangeli wrote: > > > + ret = filemap_sync(vma, start, end-start, flags); > > > > - if (!error && (flags & MS_SYNC)) { > > + if (flags & (MS_SYNC|MS_ASYNC)) { > > ok, it cannot fail but I prefer you either avoid the 'ret = > filemap_sync' and you make filemap_sync return void, or you also left > '(!err' over here. OK, let's leave the test in place - filemap_sync() has global scope. > > struct inode * inode = file->f_dentry->d_inode; > > + > > down(&inode->i_sem); > > - filemap_fdatasync(inode->i_mapping); > > - if (file->f_op && file->f_op->fsync) > > - error = file->f_op->fsync(file, file->f_dentry, 1); > > - filemap_fdatawait(inode->i_mapping); > > + ret = filemap_fdatasync(inode->i_mapping); > > + if (flags & MS_SYNC) { > > + int err; > > + > > + if (file->f_op && file->f_op->fsync) { > > + err = file->f_op->fsync(file, file->f_dentry, 1); > > + if (err && !ret) > > + ret = err; > > + } > > + err = filemap_fdatawait(inode->i_mapping); > > + if (err && !ret) > > + ret = err; > > + } > > sounds right (not something I'd love to do in 2.4 but it's > strightforward enough so I'll take the risk). It works OK, but in practice makes little difference compared to MS_ASYNC. We run out of request slots very easily with this sort of thing. Another option would be to make MS_ASYNC behave the same as MS_SYNC, which is probably better than just ignoring it as we do at present. I'll leave it as shown above unless there be objections. > > you forgot return ret here. Whoop, thanks. It was returning `err'. > > --- linux-2.4.18-pre1/fs/nfs/file.c Wed Dec 26 11:47:41 2001 > > +++ linux-akpm/fs/nfs/file.c Sat Jan 5 03:21:07 2002 > > @@ -244,6 +244,7 @@ nfs_lock(struct file *filp, int cmd, str > > { > > struct inode * inode = filp->f_dentry->d_inode; > > int status = 0; > > + int status2; > > > > dprintk("NFS: nfs_lock(f=%4x/%ld, t=%x, fl=%x, r=%Ld:%Ld)\n", > > inode->i_dev, inode->i_ino, > > @@ -278,11 +279,18 @@ nfs_lock(struct file *filp, int cmd, str > > * Flush all pending writes before doing anything > > * with locks.. > > */ > > - filemap_fdatasync(inode->i_mapping); > > + /* > > + * Shouldn't filemap_fdatasync/wait be inside i_sem? > > + */ > > I think it's not necessary, because the list browse it's serialized by > the pagecache_lock and writepage can run outside the i_sem. Yup. I've changed this part of the patch based on some discussion with Trond. I'll wait until Marcelo looks like he has his head above water and then send out the final version of the ramdisk, truncate and fsync/msync patches, cc'ed to yourself and lkml. Thanks. - 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/