Re: [PATCH v2 15/19] locking/lockdep: Remove __cq_empty()

From: Bart Van Assche
Date: Tue Mar 19 2019 - 12:54:49 EST


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.