Re: [PATCH 08/11] VFS: add inode_dir_lock/unlock

NeilBrown
2024-12-23T03:10:16Z

On Sat, 21 Dec 2024, Hillf Danton wrote:
> On Fri, 20 Dec 2024 13:54:26 +1100 NeilBrown <neilb@suse.de>
> > During the transition from providing exclusive locking on the directory
> > for directory modifying operation to providing exclusive locking only on
> > the dentry with a shared lock on the directory - we need an alternate
> > way to provide exclusion on the directory for file systems which haven't
> > been converted. This is provided by inode_dir_lock() and
> > inode_dir_inlock().
> > This uses a bit in i_state for locking, and wait_var_event_spinlock() for
> > waiting.
> >
> Inventing anything like mutex sounds bad.

In general I would agree. But when the cost of adding a mutex exceeds
the cost of using an alternate solution that only requires 2 bits, I
think the alternate solution is justified.

> > --- a/include/linux/fs.h
> > +++ b/include/linux/fs.h
> > @@ -722,6 +722,8 @@ struct inode {
> > void (*free_inode)(struct inode *);
> > };
> > struct file_lock_context *i_flctx;
> > +
> > + struct lockdep_map i_dirlock_map; /* For tracking I_DIR_LOCKED locks */
>
> The cost of this map says no to any attempt inventing mutex in any form.
>

"struct lockdep_map" is size-zero when lockdep is not enabled. And when
it is enabled we accept the cost of larger structures to benefit from
deadlock detection.
So I don't think this is a sound argument.

Thanks for the review,
NeilBrown