| Message ID | 20140124204700.532BA31C1B6@corp2gmr1-1.hot.corp.google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 01/24/2014 02:46 PM, akpm@linux-foundation.org wrote: > From: Younger Liu <younger.liucn@gmail.com> > Subject: ocfs2: fix ocfs2_sync_file() if filesystem is readonly > > If filesystem is readonly, there is no need to flush drive's caches or > force any uncommitted transactions. An ocfs2 filesystem can be set to read-only because of an error, in which case, you should return -EROFS. Nak.
On Fri, Jan 24, 2014 at 12:46:59PM -0800, akpm@linux-foundation.org wrote: > From: Younger Liu <younger.liucn@gmail.com> > Subject: ocfs2: fix ocfs2_sync_file() if filesystem is readonly > > If filesystem is readonly, there is no need to flush drive's caches or > force any uncommitted transactions. > > Signed-off-by: Younger Liu <younger.liucn@gmail.com> > Cc: Joel Becker <jlbec@evilplan.org> > Cc: Mark Fasheh <mfasheh@suse.com> > Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Looks good, thanks for this. Signed-off-by: Mark Fasheh <mfasheh@suse.de> --Mark -- Mark Fasheh
On Fri, Jan 24, 2014 at 04:02:09PM -0600, Goldwyn Rodrigues wrote: > > > On 01/24/2014 02:46 PM, akpm@linux-foundation.org wrote: > > From: Younger Liu <younger.liucn@gmail.com> > > Subject: ocfs2: fix ocfs2_sync_file() if filesystem is readonly > > > > If filesystem is readonly, there is no need to flush drive's caches or > > force any uncommitted transactions. > > An ocfs2 filesystem can be set to read-only because of an error, in > which case, you should return -EROFS. > > Nak. Goldwyn's right actually - disregard my sign off for the last one. Basically the patch does this: if (we're in some readonly state) return 0; What we want, at the top of ocfs2_sync_file() is a return of -EROFS. This will satisfy Goldwyn's requirement that we bubble -EROFS up the stack but at the same time avoiding the extra work of trying to sync on a RO fs. So the new version of the patch would be: if (we're in some readonly state) return -EROFS; Thanks, --Mark -- Mark Fasheh
diff -puN fs/ocfs2/file.c~ocfs2-fix-ocfs2_sync_file-if-filesystem-is-readonly fs/ocfs2/file.c --- a/fs/ocfs2/file.c~ocfs2-fix-ocfs2_sync_file-if-filesystem-is-readonly +++ a/fs/ocfs2/file.c @@ -185,6 +185,9 @@ static int ocfs2_sync_file(struct file * file->f_path.dentry->d_name.name, (unsigned long long)datasync); + if (ocfs2_is_hard_readonly(osb) || ocfs2_is_soft_readonly(osb)) + return 0; + err = filemap_write_and_wait_range(inode->i_mapping, start, end); if (err) return err;