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;
Message-ID: <1553014484.65329.10.camel@acm.org>
Subject: Re: [PATCH v2 15/19] locking/lockdep: Remove __cq_empty()
From:   Bart Van Assche <bvanassche@acm.org>
To:     Yuyang Du <duyuyang@gmail.com>, peterz@infradead.org,
        will.deacon@arm.com, mingo@kernel.org
Cc:     ming.lei@redhat.com, linux-kernel@vger.kernel.org
Date:   Tue, 19 Mar 2019 09:54:44 -0700
In-Reply-To: <20190318085733.3143-16-duyuyang@gmail.com>
References: <20190318085733.3143-1-duyuyang@gmail.com>
         <20190318085733.3143-16-duyuyang@gmail.com>
Content-Type: text/plain; charset="UTF-7"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Mon, 2019-03-18 at 16:57 +-0800, Yuyang Du wrote:
+AD4 +AF8AXw-cq+AF8-empty() can be embeded in +AF8AXw-cq+AF8-dequeue(), removing it. We get slightly
+AD4 simpler code. No functional change.

Does inlining +AF8AXw-cq+AF8-empty() really improve readability of the lockdep code?

+AD4 -static inline int +AF8AXw-cq+AF8-dequeue(struct circular+AF8-queue +ACo-cq, struct lock+AF8-list +ACoAKg-elem)
+AD4 +-/+ACo
+AD4 +- +ACo Dequeue an element from the circular+AF8-queue, return the lock if the queue
+AD4 +- +ACo is not empty, or NULL if otherwise
+AD4 +- +ACo-/
+AD4 +-static inline struct lock+AF8-list +ACo +AF8AXw-cq+AF8-dequeue(struct circular+AF8-queue +ACo-cq)
+AD4  +AHs
+AD4 -       if (+AF8AXw-cq+AF8-empty(cq))
+AD4 -               return -1+ADs
+AD4 +-       struct lock+AF8-list +ACo lock+ADs
+AD4  
+AD4 -       +ACo-elem +AD0 cq-+AD4-element+AFs-cq-+AD4-front+AF0AOw
+AD4 +-       /+ACo
+AD4 +-        +ACo Is the circular+AF8-queue empty?
+AD4 +-        +ACo-/
+AD4 +-       if (cq-+AD4-front +AD0APQ cq-+AD4-rear)
+AD4 +-               return NULL+ADs
+AD4 +-
+AD4 +-       lock +AD0 cq-+AD4-element+AFs-cq-+AD4-front+AF0AOw
+AD4         cq-+AD4-front +AD0 (cq-+AD4-front +- 1) +ACY CQ+AF8-MASK+ADs
+AD4 -       return 0+ADs
+AD4 +-
+AD4 +-       return lock+ADs
+AD4  +AH0
+AD4  
+AD4  static inline unsigned int  +AF8AXw-cq+AF8-get+AF8-elem+AF8-count(struct circular+AF8-queue +ACo-cq)
+AD4 +AEAAQA -1376,6 +-1381,7 +AEAAQA static int +AF8AXw-bfs(struct lock+AF8-list +ACo-source+AF8-entry,
+AD4                  int forward)
+AD4  +AHs
+AD4         struct lock+AF8-list +ACo-entry+ADs
+AD4 +-       struct lock+AF8-list +ACo-lock+ADs
+AD4         struct list+AF8-head +ACo-head+ADs
+AD4         struct circular+AF8-queue +ACo-cq +AD0 +ACY-lock+AF8-cq+ADs
+AD4         int ret +AD0 1+ADs
+AD4 +AEAAQA -1397,10 +-1403,7 +AEAAQA static int +AF8AXw-bfs(struct lock+AF8-list +ACo-source+AF8-entry,
+AD4         +AF8AXw-cq+AF8-init(cq)+ADs
+AD4         +AF8AXw-cq+AF8-enqueue(cq, source+AF8-entry)+ADs
+AD4  
+AD4 -       while (+ACEAXwBf-cq+AF8-empty(cq)) +AHs
+AD4 -               struct lock+AF8-list +ACo-lock+ADs
+AD4 -
+AD4 -               +AF8AXw-cq+AF8-dequeue(cq, +ACY-lock)+ADs
+AD4 +-       while ((lock +AD0 +AF8AXw-cq+AF8-dequeue(cq))) +AHs
+AD4  
+AD4                 if (+ACE-lock-+AD4-class) +AHs
+AD4                         ret +AD0 -2+ADs

This is the most important change in this patch. Using the title +ACI-Remove +AF8AXw-cq+AF8-empty()+ACI
for this patch is misleading because the above patch does something else, namely changing
the return type of +AF8AXw-cq+AF8-dequeue() from int into a pointer. Should this patch perhaps be
split or should the +AF8AXw-cq+AF8-empty() removal be left out from this patch?

Bart.