Re: [PATCH v3 17/30] locking/lockdep: Add read-write type for a lock dependency

[Yuyang Du]

Thanks for review.

On Wed, 10 Jul 2019 at 13:18, Boqun Feng <boqun.feng@xxxxxxxxx> wrote:
>
> On Fri, Jun 28, 2019 at 05:15:15PM +0800, Yuyang Du wrote:
> > Direct dependencies need to keep track of their read-write lock types.
> > Two bit fields, which share the distance field, are added to lock_list
> > struct so the types are stored there.
> >
> > With a dependecy lock1 -> lock2, lock_type1 has the type for lock1 and
> > lock_type2 has the type for lock2, where the values are one of the
> > lock_type enums.
> >
> > Signed-off-by: Yuyang Du <duyuyang@xxxxxxxxx>
> > ---
> >  include/linux/lockdep.h  | 15 ++++++++++++++-
> >  kernel/locking/lockdep.c | 25 +++++++++++++++++++++++--
> >  2 files changed, 37 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
> > index eb26e93..fd619ac 100644
> > --- a/include/linux/lockdep.h
> > +++ b/include/linux/lockdep.h
> > @@ -185,6 +185,8 @@ static inline void lockdep_copy_map(struct lockdep_map *to,
> >               to->class_cache[i] = NULL;
> >  }
> >
> > +#define LOCK_TYPE_BITS       2
> > +
> >  /*
> >   * Every lock has a list of other locks that were taken after or before
> >   * it as lock dependencies. These dependencies constitute a graph, which
> > @@ -207,7 +209,17 @@ struct lock_list {
> >       struct list_head                chains;
> >       struct lock_class               *class[2];
> >       struct lock_trace               trace;
> > -     int                             distance;
> > +
> > +     /*
> > +      * The lock_type fields keep track of the lock type of this
> > +      * dependency.
> > +      *
> > +      * With L1 -> L2, lock_type1 stores the lock type of L1, and
> > +      * lock_type2 stores that of L2.
> > +      */
> > +     unsigned int                    lock_type1 : LOCK_TYPE_BITS,
> > +                                     lock_type2 : LOCK_TYPE_BITS,
>
> Bad names ;-) Maybe fw_dep_type and bw_dep_type? Which seems to be
> aligned with the naming schema other functions.

I think the types are for L1 -> L2 respectively, hence the names in
question. Let me reconsider this anyway and maybe hear from others.

Thanks,
Yuyang