Re: [PATCH 3/5] f2fs: report readonly status in ->fsync

[Jaegeuk Kim <jaegeuk@kernel.org>]

Hi Chao,

Thanks Marc for your report.

On Mon, Dec 28, 2015 at 05:59:19PM +0800, Chao Yu wrote:
> > -----Original Message-----
> > From: Marc Lehmann [mailto:schmorp@schmorp.de]
> > Sent: Saturday, December 26, 2015 2:33 PM
> > To: Chao Yu
> > Cc: linux-f2fs-devel@lists.sourceforge.net
> > Subject: Re: [f2fs-dev] [PATCH 3/5] f2fs: report readonly status in ->fsync
> > 
> > On Thu, Dec 24, 2015 at 06:07:25PM +0800, Chao Yu <chao2.yu@samsung.com> wrote:
> > > Report readonly status of filesystem during fsync.
> > 
> > If this means that fsync returns EROFS when used on a file on a read-only
> > filesystem., then this looks buggy - EROFS is not documented for
> > fsync (only as "fd is bound to a special file which does not support
> > synchronization") for this condition, and I don't think other filesystems
> > do that (just tried xfs).
> 
> Yes.
> 
> > 
> > It also doesn't quite make sense - fsync should only fail when the file
> > couldn't be synced, but in most (probably all) cases, the file is already
> > synchronised, otherwise the filesystem shouldn't be RO.
> 
> Agreed,
> 
> One situation here is if '.' or '..' in dentry page of directory inode was
> missing (corrupted directory), once fsck found this kind of inode exist in
> the image, it will mark inode with F2FS_INLINE_DOTS flag, after that f2fs
> will try to recover it to normal one if user touch such inode in ->lookup,
> so the dirty data was generated even in a readonly fs.
> 
> Actually even for above case, it would be better to avoid generating dirty
> data rather than stop user fsync in a readonly fs.
> 
> Hi Jaegeuk, could you help to drop this patch?

Okay.

> 
> And one more thing is how do you think of moving inline dot recovery into
> fsck?

Hmm, we can't move that part entirely into fsck regarding to backward
compatibility. We can add that in fsck, but it needs another block
allocation part which we need to handle -ENOSPC in fsck as well.

Thanks,

> 
> Thanks,
> 
> > 
> > I only looked at the diff, so if I missed the context and my reasoning
> > doesn't apply, forgive and ignore me :)
> > 
> > --
> >                 The choice of a       Deliantra, the free code+content MORPG
> >       -----==-     _GNU_              http://www.deliantra.net
> >       ----==-- _       generation
> >       ---==---(_)__  __ ____  __      Marc Lehmann
> >       --==---/ / _ \/ // /\ \/ /      schmorp@schmorp.de
> >       -=====/_/_//_/\_,_/ /_/\_\

------------------------------------------------------------------------------