On Tue, Aug 15, 2023 at 10:07 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > > On Tue, 15 Aug 2023 at 17:59, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > > > What occurs to me is why are we bothering with getting write access on > > > the internal upper mnt each time. Seems to me it's a historical thing > > > without a good reason. Upper mnt is never changed from R/W to R/O. > > > > > > So the only thing we need to do is grab the upper mount write access > > > on superblock creation and do the sb_start_write/end_write() thing > > > which can't fail. If upper mnt is read-only, we effectively have a > > > read-only filesystem, and can handle it that way (sb->s_flags |= > > > SB_RDONLY). > > > > > > There's still the possibility that we do some changes to upper even > > > for non-modify operations. But with careful review we can remove a > > > most (possibly all) error handling cases from ovl_want_write() > > > callsites when we do know that we have write access on upper. And > > > WARN_ON(__mnt_is_readonly(ovl_upper_mnt(ofs))) should ensure that we > > > catch any mistakes. > > > > > > Hmm? > > > > > > > I was thinking the same thing myself, before I went on this journey. > > I reached the conclusion that doing only sb_start_write() would not be > > safe against emergency remount rdonly of the upper sb. > > > > I guess if upper sb is emergency mounted rdonly, then overlayfs > > sb would also be emergency remounted rdonly, but for example > > ext4 sb can become rdonly on internal errors. > > But maybe that is not the responsibility of vfs or ovl to care about? > > Consider the case of a writable open file: the mount write access is > only checked on open. So not having fine grained mnt write access > checks is not without precedent. > That's true. I see that at least ext4_sync_file() and ext4_do_writepages() test the EXT4_MF_FS_ABORTED case specifically to return EROFS. > I'm not sure, but the number of added lines in this particular patch > makes me think that at least during copy-up we could separate the mnt > and the sb write locks. > Yeh, I think that makes a lot of sense. > > Christian, is there also an API to set the sb rdonly when private > > writable mounts (i.e. ovl_upper_mnt) exist? > > You mean notifying overlayfs about rdonly remount of upper mnt? No, > that doesn't exist today. > What I meant is, except from emergency remount rdonly and fs specific cases like ext4 remount-ro on error, is there a way via new mount API that users can request remount of the upper SB rdonly, despite the fact that this sb has private writable mount clones? even if ovl has elevated mnt_writers of upper_mnt? Thanks, Amir.
