Re: [PATCH v13 05/10] fuse: Handle asynchronous read and write in passthrough

[Miklos Szeredi <miklos@szeredi.hu>]

On Tue, 22 Aug 2023 at 12:18, Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Mon, Aug 21, 2023 at 6:27 PM Amir Goldstein <amir73il@gmail.com> wrote:

> > Getting back to this.
> > Did you mean something like that? (only compile tested)
> >
> > https://github.com/amir73il/linux/commits/backing_fs
> >
> > If yes, then I wonder:
> > 1. Is the difference between FUSE_IOCB_MASK and OVL_IOCB_MASK
> >     (i.e. the APPEND flag) intentional?

Setting IOCB_APPEND on the backing file doesn't make a difference as
long as the backing file is not modified during the write.

In overlayfs the case of the backing file being modified is not
defined, so I guess that's the reason to omit it.  However I don't see
a problem with setting it on the backing file either, the file
size/position is synchronized after the write, so nothing bad should
happen if the backing file was modified.

> > 2. What would be the right way to do ovl_copyattr() on io completion?
> >     Pass another completion handler to read/write helpers?
> >     This seems a bit ugly. Do you have a nicer idea?
> >

Ugh, I missed that little detail.   I don't have a better idea than to
use a callback function.

>
> Hmm. Looking closer, ovl_copyattr() in ovl_aio_cleanup_handler()
> seems a bit racy as it is not done under inode_lock().
>
> I wonder if it is enough to fix that by adding the lock or if we need
> to resort to a more complicated scheme like FUSE_I_SIZE_UNSTABLE
> for overlayfs aio?

Quite recently rename didn't take inode lock on source, so
ovl_aio_cleanup_handler() wasn't the only unlocked instance.

I don't see a strong reason to always lock the inode before
ovl_copyattr(), but I could be wrong.

Thanks,
Miklos