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

[NeilBrown]

On Sat, 21 Dec 2024, Hillf Danton wrote:
> On Fri, 20 Dec 2024 13:54:26 +1100 NeilBrown <neilb@xxxxxxx>
> > 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