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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 11 Jul 2019 13:02:14 +0800
From:   Yuyang Du <duyuyang@...il.com>
To:     Boqun Feng <boqun.feng@...il.com>
Cc:     Peter Zijlstra <peterz@...radead.org>,
        Will Deacon <will.deacon@....com>,
        Ingo Molnar <mingo@...nel.org>,
        Bart Van Assche <bvanassche@....org>, ming.lei@...hat.com,
        Frederic Weisbecker <frederic@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        LKML <linux-kernel@...r.kernel.org>,
        Waiman Long <longman@...hat.com>, paulmck@...ux.vnet.ibm.com
Subject: Re: [PATCH v3 17/30] locking/lockdep: Add read-write type for a lock dependency

Thanks for review.

On Wed, 10 Jul 2019 at 13:18, Boqun Feng <boqun.feng@...il.com> 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@...il.com>
> > ---
> >  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