On Fri, Jul 20, 2007 at 04:16:31AM +0100, Al Viro wrote: > On Fri, Jul 20, 2007 at 12:36:01PM +1000, David Chinner wrote: > > To the context that dropped the last reference. It can't be > > reported to anything else.... > > Oh, for fsck sake... Here is a patch which removes the file_count test from coda_flush. This avoids the race condition described by Al Viro where we may never see the file_count drop to 0 in ->flush, in which case userspace is never notified that a writeback should occur. Signed-off-by: Jan Harkes <jaharkes@xxxxxxxxxx> --- Going from here I can fix the problem in several ways in userspace. The current implementation would end up making useless snapshots as a result of every close, and still leave us with lost updates in the the write-after-close case, but that isn't any worse than what it is now. Seeing every fops->flush does allow us to perform preliminary checks so we can return errors for common issues and we can flag the file so that the final release upcall (where we can no longer return errors) will start the writeback. Really the problem for me is that Coda's semantics (write on last close) are not compatible with UNIX semantics, where writes may occur even after the application has closed all fd's. This is quite similar to having an open fd to a removed file, UNIX obviously doesn't return an error or block the unlink if there is an active reference it just detaches the inode from the namespace. I will try to find a clean way to block the close syscall until fput drops the last reference. However I realize that such an implementation would not be acceptable for other file systems, and there are some interesting unresolved details such as 'which close is going to be the last close'. diff --git a/fs/coda/file.c b/fs/coda/file.c index 7594962..0a300dd 100644 --- a/fs/coda/file.c +++ b/fs/coda/file.c @@ -169,15 +169,10 @@ int coda_flush(struct file *coda_file, fl_owner_t id) unsigned short coda_flags = coda_flags_to_cflags(flags); struct coda_file_info *cfi; struct inode *coda_inode; - int err = 0, fcnt; + int err = 0; lock_kernel(); - /* last close semantics */ - fcnt = file_count(coda_file); - if (fcnt > 1) - goto out; - /* No need to make an upcall when we have not made any modifications * to the file */ if ((coda_file->f_flags & O_ACCMODE) == O_RDONLY) @@ -246,7 +241,7 @@ int coda_release(struct inode *coda_inode, struct file *coda_file) coda_file->private_data = NULL; unlock_kernel(); - return err; + return 0; /* no point in returning err, VFS ignores returned value */ } int coda_fsync(struct file *coda_file, struct dentry *coda_dentry, int datasync) - 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
