Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190628091528.17059-1-duyuyang@gmail.com> <20190628091528.17059-18-duyuyang@gmail.com>
 <20190710051830.GB14490@tardis>
In-Reply-To: <20190710051830.GB14490@tardis>
From:   Yuyang Du <duyuyang@gmail.com>
Date:   Thu, 11 Jul 2019 13:02:14 +0800
Message-ID: <CAHttsrbedG9aJibhXBJZwKtt2ABX+TDU16dOWN+RP9yJ5OcbWA@mail.gmail.com>
Subject: Re: [PATCH v3 17/30] locking/lockdep: Add read-write type for a lock dependency
To:     Boqun Feng <boqun.feng@gmail.com>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Will Deacon <will.deacon@arm.com>,
        Ingo Molnar <mingo@kernel.org>,
        Bart Van Assche <bvanassche@acm.org>, ming.lei@redhat.com,
        Frederic Weisbecker <frederic@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        LKML <linux-kernel@vger.kernel.org>,
        Waiman Long <longman@redhat.com>, paulmck@linux.vnet.ibm.com
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Thanks for review.

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