On Sat, Nov 30, 2019 at 03:53:10PM +0800, yukuai (C) wrote:
> On 2019/11/30 11:43, Matthew Wilcox wrote:
> > On Sat, Nov 30, 2019 at 10:02:23AM +0800, yu kuai wrote:
> > > However, a single 'DENTRY_D_LOCK_NESTED' may not be enough if more than
> > > two dentry are involed. So, add in 'DENTRY_D_LOCK_NESTED_TWICE'.
> >
> > No. These need meaningful names. Indeed, I think D_LOCK_NESTED is
> > a terrible name.
> >
> > The exception is __d_move() where I think we should actually name the
> > different lock classes instead of using a bare '2' and '3'. Something
> > like this, perhaps:
>
> Thanks for looking into this, do you mind if I replace your patch with the
> first two patches in the patchset?
That's fine by me, but I think we should wait for Al to give his approval
before submitting a new version.
I'm also not entirely content with the explanation I wrote last night.
Maybe this instead ...
/*
- * dentry->d_lock spinlock nesting subclasses:
+ * dentry->d_lock spinlock nesting subclasses. Always taken in increasing
+ * order although some subclasses may be skipped. If one dentry is the
+ * ancestor of another, then the ancestor's d_lock is taken before the
+ * descendent. If NORMAL and PARENT_2 do not have a hierarchical relationship
+ * then you must hold the s_vfs_rename_mutex to prevent another thread taking
+ * the locks in the opposite order, or NORMAL and PARENT_2 becoming
+ * hierarchical through a rename operation.
*
* 0: normal
- * 1: nested
+ * 1: either a descendent of "normal" or a cousin.
+ * 2: child of the "normal" dentry
+ * 3: child of the "parent2" dentry
*/
enum dentry_d_lock_class
{
- DENTRY_D_LOCK_NORMAL, /* implicitly used by plain spin_lock() APIs. */
- DENTRY_D_LOCK_NESTED
+ DENTRY_D_LOCK_NORMAL, /* implicitly used by plain spin_lock() APIs */
+ DENTRY_D_LOCK_PARENT_2, /* not an ancestor of normal */
+ DENTRY_D_LOCK_CHILD, /* nests under parent's lock */
+ DENTRY_D_LOCK_CHILD_2, /* PARENT_2's child */
};
