On Mon, Dec 28, 2020 at 03:56:18PM +0000, Matthew Wilcox wrote: > On Mon, Dec 28, 2020 at 08:25:50AM -0500, Jeff Layton wrote: > > To be clear, the main thing you'll lose with the method above is the > > ability to see an unseen error on a newly opened fd, if there was an > > overlayfs mount using the same upper sb before your open occurred. > > > > IOW, consider two overlayfs mounts using the same upper layer sb: > > > > ovlfs1 ovlfs2 > > ---------------------------------------------------------------------- > > mount > > open fd1 > > write to fd1 > > <writeback fails> > > mount (upper errseq_t SEEN flag marked) > > open fd2 > > syncfs(fd2) > > syncfs(fd1) > > > > > > On a "normal" (non-overlay) fs, you'd get an error back on both syncfs > > calls. The first one has a sample from before the error occurred, and > > the second one has a sample of 0, due to the fact that the error was > > unseen at open time. > > > > On overlayfs, with the intervening mount of ovlfs2, syncfs(fd1) will > > return an error and syncfs(fd2) will not. If we split the SEEN flag into > > two, then we can ensure that they both still get an error in this > > situation. > > But do we need to? If the inode has been evicted we also lose the errno. That's for the case of fsync(), right? For the case of syncfs() we will not lose error as its stored in super_block. Even for the case of fsync(), inode can be evicted only if no other fd is opened for the file. So in above example, fd1 is opened so inode can't be evicted, that means we will see error on syncfs(fd2) and not lose it. So if we start consuming upper fs on overlay mount(), it will be a change of behavior for applications using same upper fs. So far overlay mount() does not consume unseen error and even if an fd is opened after the error, application will see error on super block. If we consume error on mount(), we change behavior. I am not saying that's necessarily bad, I am just trying to point out that its a user space visible behavior change and worried if somebody starts calling it a regression. Anyway, I looks like two problems got mixed into same thread. One problem we need to solve is that syncfs() on overlayfs should report back writeback errors (as well as other errors) to applications. And that's what this patch series is solving. And then second issue is detecting writeback errors over remount for volatile mounts. And that's where this question comes whether we should split seen flag or we should simply consume error on mount. So this can be further discussed when patches for this changes are posted again. For now, I will focus on trying to fix first issue and post patches for that again after more testing. Vivek > The guarantee we provide is that a fd that was open before the error > occurred will see the error. An fd that's opened after the error occurred > may or may not see the error.
