On Fri, Feb 9, 2024 at 5:28 PM Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> On Fri, Feb 9, 2024 at 3:27 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> >
> > On Fri, 9 Feb 2024 at 13:12, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> >
> > > I think this race can happen even if we remove killable_
> >
> > Without _killable, the loop will exit with iocachectr >= 0, hence the
> > FUSE_I_CACHE_IO_MODE will not be cleared.
> >
> > > not sure - anyway, with fuse passthrough there is another error
> > > condition:
> > >
> > > /*
> > > * Check if inode entered passthrough io mode while waiting for parallel
> > > * dio write completion.
> > > */
> > > if (fuse_inode_backing(fi))
> > > err = -ETXTBSY;
> > >
> > > But in this condition, all waiting tasks should abort the wait,
> > > so it does not seem a problem to clean the flag.
> >
> > Ah, this complicates things. But I think it's safe to clear
> > FUSE_I_CACHE_IO_MODE in this case, since other
> > fuse_inode_get_io_cache() calls will also fail.
> >
>
> Right.
>
> > > Anyway, IMO it is better to set the flag before every wait and on
> > > success. Like below.
> >
> > This would still have the race, since there will be a window during
> > which the FUSE_I_CACHE_IO_MODE flag has been cleared and new parallel
> > writes can start, even though there are one or more waiters for cached
> > open.
> >
> > Not that this would be a problem in practice, but I also don't see
> > removing the _killable being a big issue.
>
> ok. Remove killable.
>
> Pushed branches fuse_io_mode-090224 and fuse-backing-fd-090224
> with requested fixes.
>
> Note that I had to update libfuse fuse-backing-fd branch, because when
> removing FOPEN_CACHE_IO, I changed the constant value of
> FOPEN_PASSTHOUGH.
>
> Passes my sanity tests.
> Bernd, please verify that I did not break anything on your end.
>
Miklos,
I see that you decided to drop the waiting for parallel dio logic
in the final version. Decided to simply or found a problem?
Also, FUSE passthrough patch 9/9 ("fuse: auto-invalidate inode
attributes in passthrough mode") was not included in the final version,
so these fstests are failing on non uptodate size/mtime/ctime after
mapped write:
Failures: generic/080 generic/120 generic/207 generic/215
Was it left out by mistake or for a reason?
Thanks,
Amir.
[1] https://lore.kernel.org/linux-fsdevel/20240206142453.1906268-10-amir73il@xxxxxxxxx/
