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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 22, 2023 at 2:03 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
>
> On Tue, 22 Aug 2023 at 12:18, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> >
> > On Mon, Aug 21, 2023 at 6:27 PM Amir Goldstein <amir73il@xxxxxxxxx> 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.
>

IDK, ovl_copyattr() looks like a textbook example of a race
if not protected by something because it reads a bunch of stuff
from realinode and then writes a bunch of stuff to inode.

Anyway, I guess it wouldn't hurt to wrap it with inode_lock()
in the ovl completion callback.

Thanks,
Amir.




[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux