On 2020-04-21 08:41:01 [-0700], Paul E. McKenney wrote: > > Invoking queue_work() is not a problem locking wise. It does however > > extend the length of the critical section (which is not yet needed). > > I am guessing that by "which is not yet needed" you were meaning that > it is not absolutely necessary to extend the length of the critical > section. Please correct me if my guess is wrong. By changing the lock type you extend the atomic section from "only" queue_work() (if invoked) to everything kfree_rcu() does plus queue_work(). > In the meantime, plunging ahead... > > One approach might be to move queue_work() and friends out of the critical > section, but only enable interrupts between the two critical sections > under CONFIG_PREEMPT_RT_FULL. Would that be a reasonable approach? Yes but why do we do this raw_spinlock_t here? It is not yet needed on v5.6-RT as I *did* check. It also complicates the code for !RT but nobody responded to that part but… That said: the current memory allocation is the problem here. The remaining part is fine. The part under the lock is small enough so it should not cause the trouble if it invokes queue_work() which will "only" enqueue the timer. Side question: Is there any real-life workloads that benefits from this? I'm asking because rcuperf allocates the kfree_rcu() the pointer right away. The chances are high that the pointer are fed from the same page. SLUB's build_detached_freelist() scans the page of RCU's pointers to ensure that they are in the same page and then slab_free() them in one go. There is lookahead = 3 so it finds three different pages it stops further scanning and does slab_free() with what it found so far. Which means if your kfree_rcu() collects random pointer from the system, they may belong to different pages (especially if they are part of different "types"). > Thanx, Paul Sebastian
