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. > > 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. > The patch with separate locks during copy-up is not much smaller but it is a lot nicer IMO: https://github.com/amir73il/linux/commits/ovl_want_write-v3 I shall post these shortly after tests are complete. Thanks, Amir.
