On Thu 13-03-14 19:15:06, Ted Tso wrote:
> On Thu, Mar 13, 2014 at 04:28:23PM +0000, Steven Whitehouse wrote:
> > I guess the same is true for other file systems which are mounted ro
> > too. So maybe a check for MS_RDONLY before doing the sync in those
> > cases?
> My original patch moved the sync_filesystem into the check for
> MS_RDONLY in the core VFS code. The objection was raised that there
> might be some file system out there that might depend on this
> behaviour. I can't imagine why, but I suppose it's at least
> theoretically possible.
Well, syncfs() in RO->RW transition clearly isn't needed. In RW->RO
transition basically everybody needs it. What is disputable is the case of
RW->RW remounts and I could imagine someone relying on syncfs() there
although 99% of filesystems don't need it. So I agree in principle with
moving the responsibility for syncfs() to ->remount so that each filesystem
> So the idea is that this particular patch is *guaranteed* not to make
> any difference. That way there can be no question about the patch'es
> I'm going to follow up with a patch for ext4 that does exactly that,
> but the idea is to allow each file system maintainer to do that for
> their own file system.
> I could do that as well for file systems that are "obviously"
> read-only, but then I'll find out that there's some wierd case where
> the file system can be used in a read-write fashion. (Example: UDF is
> normally used for DVD's, but at least in theory it can be used
> read/write --- I'm told that Windows supports read-write UDF file
> systems on USB sticks, and at least in theory it could be used as a
> inter-OS exchange format in situations where VFAT and exFAT might not
> be appropriate for various reasons.)
Yes, some people do use UDF for USB sticks - Linux supports it in
read-write mode as well (although with somewhat limited set of features).
But the filesystem's I've named are those which clearly either bail with
error if RW mount is attempted or silently change it to RO mount. In these
cases, I'd just patch away the unnecessary sync_fs().
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR