On Sat, 2020-12-19 at 06:13 +0000, Matthew Wilcox wrote: > On Thu, Dec 17, 2020 at 10:00:37AM -0500, Jeff Layton wrote: > > Overlayfs's volatile mounts want to be able to sample an error for their > > own purposes, without preventing a later opener from potentially seeing > > the error. > > umm ... can't they just copy the errseq_t they're interested in, followed > by calling errseq_check() later? > They don't want the sampling for the volatile mount to prevent later openers from seeing an error that hasn't yet been reported. If they copy the errseq_t (or just do an errseq_sample), and then follow it with a errseq_check_and_advance then the SEEN bit will end up being set and a later opener wouldn't see the error. Aside from that though, I think this patch clarifies things a bit since the SEEN flag currently means two different things: 1/ do I need to increment the counter when recording another error? 2/ do I need to report this error to new samplers (at open time) That was ok before, since we those conditions were always changed together, but with the overlayfs volatile mount use-case, it no longer does. > actually, isn't errseq_check() buggy in the face of multiple > watchers? consider this: > > worker.es starts at 0 > t2.es = errseq_sample(&worker.es) > errseq_set(&worker.es, -EIO) > t1.es = errseq_sample(&worker.es) > t2.err = errseq_check_and_advance(&es, t2.es) > ** this sets ERRSEQ_SEEN ** > t1.err = errseq_check(&worker.es, t1.es) > ** reports an error, even though the only change is that > ERRSEQ_SEEN moved **. > > i think errseq_check() should be: > > if (likely(cur | ERRSEQ_SEEN) == (since | ERRSEQ_SEEN)) > return 0; > > i'm not yet convinced other changes are needed to errseq. but i am > having great trouble understanding exactly what overlayfs is trying to do. I think you're right on errseq_check. I'll plan to do a patch to fix that up as well. I too am having a bit of trouble understanding all of the nuances here. My current understanding is that it depends on the "volatility" of the mount: normal (non-volatile): they basically want to be able to track errors as if the files were being opened on the upper layer. For this case I think they should aim to just do all of the error checking against the upper sb and ignore the overlayfs s_wb_err field. This does mean pushing the errseq_check_and_advance down into the individual filesystems in some fashion though. volatile: they want to sample at mount time and always return an error to syncfs if there has been another error since the original sample point. This sampling should also not affect later openers on the upper layer (or on other overlayfs mounts). I'm not 100% clear on whether I understand both use-cases correctly though. -- Jeff Layton <jlayton@xxxxxxxxxx>
