Re: [PATCH v5 07/15] locking/lockdep: Free lock classes that are no longer in use

[Bart Van Assche]

On Thu, 2019-01-10 at 20:44 +-0100, Peter Zijlstra wrote:
+AD4 On Thu, Jan 10, 2019 at 10:51:27AM -0800, Bart Van Assche wrote:
+AD4 +AD4 On Thu, 2019-01-10 at 16:24 +-0100, Peter Zijlstra wrote:
+AD4 +AD4 +AD4  /+ACo
+AD4 +AD4 +AD4   +ACo A data structure for delayed freeing of data structures that may be
+AD4 +AD4 +AD4 - +ACo accessed by RCU readers at the time these were freed. The size of the array
+AD4 +AD4 +AD4 - +ACo is a compromise between minimizing the amount of memory used by this array
+AD4 +AD4 +AD4 - +ACo and minimizing the number of wait+AF8-event() calls by get+AF8-pending+AF8-free+AF8-lock().
+AD4 +AD4 +AD4 +- +ACo accessed by RCU readers at the time these were freed.
+AD4 +AD4 +AD4   +ACo-/
+AD4 +AD4 +AD4  static struct pending+AF8-free +AHs
+AD4 +AD4 +AD4 -	struct list+AF8-head zapped+AF8-classes+ADs
+AD4 +AD4 +AD4  	struct rcu+AF8-head	 rcu+AF8-head+ADs
+AD4 +AD4 +AD4 +-	int		 index+ADs
+AD4 +AD4 +AD4  	int		 pending+ADs
+AD4 +AD4 +AD4 -+AH0 pending+AF8-free+AFs-2+AF0AOw
+AD4 +AD4 +AD4 -static DECLARE+AF8-WAIT+AF8-QUEUE+AF8-HEAD(rcu+AF8-cb)+ADs
+AD4 +AD4 +AD4 +-	struct list+AF8-head zapped+AFs-2+AF0AOw
+AD4 +AD4 +AD4 +-+AH0 pending+AF8-free+ADs
+AD4 +AD4 
+AD4 +AD4 Hi Peter,
+AD4 +AD4 
+AD4 +AD4 If the zapped+AFsAXQ array only has two elements there is no guarantee that an
+AD4 +AD4 element will be free when zap+AF8-class() is called. I think we need at least
+AD4 +AD4 num+AF8-online+AF8-cpus() elements to guarantee that at least one element is free
+AD4 +AD4 when zap+AF8-class() is called. So removing the wait loop from
+AD4 +AD4 get+AF8-pending+AF8-free+AF8-lock() seems wrong to me. Have you tried to run a workload
+AD4 +AD4 that keeps all CPUs busy and that triggers get+AF8-pending+AF8-free+AF8-lock()
+AD4 +AD4 frequently?
+AD4 
+AD4 I have not ran (yet)+ADs but I do not quite follow your argument. There is
+AD4 only a single rcu+AF8-head, yes? Thereby only a single list can be pending
+AD4 at any one time, and the other list is free to be appended to during
+AD4 this time -- all is serialized by the graph lock after all.
+AD4 
+AD4 When the rcu callback happens, we flush the list we started the QS for,
+AD4 which then becomes empty and if the open list contains entries, we
+AD4 flip the lot and requeue the rcu+AF8-head for another QS.
+AD4 
+AD4 Therefore we only ever need 2 lists+ADs 1 closed with entries waiting for
+AD4 the callback, 1 open, to which we can append all newly freed entries.

Hi Peter,

Now that I had a closer look at your patch I think the approach of your patch
is fine. Sorry for the confusion.

Bart.