On Mon, 6 Dec 2021 at 23:29, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
>
> On Fri, Dec 03, 2021 at 12:05:34AM +0000, Eric Wong wrote:
> > Hi all, I'm working on a new multi-threaded FS using the
> > libfuse3 fuse_lowlevel.h API. It looks to me like the kernel
> > already performs the necessary locking on a per-inode basis to
> > save me some work in userspace.
> >
> > In particular, I originally thought I'd need pthreads mutexes on
> > a per-inode (fuse_ino_t) basis to protect userspace data
> > structures between the .setattr (truncate), .fsync, and
> > .write_buf userspace callbacks.
> >
> > However upon reading the kernel, I can see fuse_fsync,
> > fuse_{cache,direct}_write_iter in fs/fuse/file.c all use
> > inode_lock. do_truncate also uses inode_lock in fs/open.c.
> >
> > So it's look like implementing extra locking in userspace would
> > do nothing useful in my case, right?
>
> I guess it probably is a good idea to implement proper locking
> in multi-threaded fs and not rely on what kind of locking
> kernel is doing. If kernel locking changes down the line, your
> implementation will be broken.
Thing is, some fuse filesystem implementations already do rely on
kernel locking. So while it shouldn't hurt to have an extra layer of
locking (except complexity and performance) it's not necessary.
See for example FUSE_PARALLEL_DIROPS which was added due to kernel
locking changes to avoid breaking backward compatibility.
