On Mon, Apr 12 2021 at 12:56, Mel Gorman wrote:
> On Fri, Apr 09, 2021 at 08:55:39PM +0200, Peter Zijlstra wrote:
> I'll update the changelog and comment accordingly. I'll decide later
> whether to leave it or move the location of the lock at the end of the
> series. If the patch is added, it'll either incur the double lookup (not
> that expensive, might be optimised by the compiler) or come up with a
> helper that takes the lock and returns the per-cpu structure. The double
> lookup probably makes more sense initially because there are multiple
> potential users of a helper that says "pin to CPU, lookup, lock and return
> a per-cpu structure" for both IRQ-safe and IRQ-unsafe variants with the
> associated expansion of the local_lock API. It might be better to introduce
> such a helper with multiple users converted at the same time and there are
> other local_lock users in preempt-rt that could do with upstreaming first.
We had such helpers in RT a while ago but it turned into an helper
explosion pretty fast. But that was one of the early versions of local
locks which could not be embedded into a per CPU data structure due to
raisins (my stupidity).
But with the more thought out approach of today we can have (+/- the
obligatory naming bikeshedding):
--- a/include/linux/local_lock.h
+++ b/include/linux/local_lock.h
@@ -51,4 +51,35 @@
#define local_unlock_irqrestore(lock, flags) \
__local_unlock_irqrestore(lock, flags)
+/**
+ * local_lock_get_cpu_ptr - Acquire a per CPU local lock and return
+ * a pointer to the per CPU data which
+ * contains the local lock.
+ * @pcp: Per CPU data structure
+ * @lock: The local lock member of @pcp
+ */
+#define local_lock_get_cpu_ptr(pcp, lock) \
+ __local_lock_get_cpu_ptr(pcp, typeof(*(pcp)), lock)
+
+/**
+ * local_lock_irq_get_cpu_ptr - Acquire a per CPU local lock, disable
+ * interrupts and return a pointer to the
+ * per CPU data which contains the local lock.
+ * @pcp: Per CPU data structure
+ * @lock: The local lock member of @pcp
+ */
+#define local_lock_irq_get_cpu_ptr(pcp, lock) \
+ __local_lock_irq_get_cpu_ptr(pcp, typeof(*(pcp)), lock)
+
+/**
+ * local_lock_irqsave_get_cpu_ptr - Acquire a per CPU local lock, save and
+ * disable interrupts and return a pointer to
+ * the CPU data which contains the local lock.
+ * @pcp: Per CPU data structure
+ * @lock: The local lock member of @pcp
+ * @flags: Storage for interrupt flags
+ */
+#define local_lock_irqsave_get_cpu_ptr(pcp, lock, flags) \
+ __local_lock_irqsave_get_cpu_ptr(pcp, typeof(*(pcp)), lock, flags)
+
#endif
--- a/include/linux/local_lock_internal.h
+++ b/include/linux/local_lock_internal.h
@@ -91,3 +91,33 @@ static inline void local_lock_release(lo
local_lock_release(this_cpu_ptr(lock)); \
local_irq_restore(flags); \
} while (0)
+
+#define __local_lock_get_cpu_ptr(pcp, type, lock) \
+ ({ \
+ type *__pcp; \
+ \
+ preempt_disable(); \
+ __pcp = this_cpu_ptr(pcp); \
+ local_lock_acquire(&__pcp->lock); \
+ __pcp; \
+ })
+
+#define __local_lock_irq_get_cpu_ptr(pcp, type, lock) \
+ ({ \
+ type *__pcp; \
+ \
+ local_irq_disable(); \
+ __pcp = this_cpu_ptr(pcp); \
+ local_lock_acquire(&__pcp->lock); \
+ __pcp; \
+ })
+
+#define __local_lock_irqsave_get_cpu_ptr(pcp, type, lock, flags)\
+ ({ \
+ type *__pcp; \
+ \
+ local_irq_save(flags); \
+ __pcp = this_cpu_ptr(pcp); \
+ local_lock_acquire(&__pcp->lock); \
+ __pcp; \
+ })
and RT will then change that to:
--- a/include/linux/local_lock_internal.h
+++ b/include/linux/local_lock_internal.h
@@ -96,7 +96,7 @@ static inline void local_lock_release(lo
({ \
type *__pcp; \
\
- preempt_disable(); \
+ ll_preempt_disable(); \
__pcp = this_cpu_ptr(pcp); \
local_lock_acquire(&__pcp->lock); \
__pcp; \
@@ -106,7 +106,7 @@ static inline void local_lock_release(lo
({ \
type *__pcp; \
\
- local_irq_disable(); \
+ ll_local_irq_disable(); \
__pcp = this_cpu_ptr(pcp); \
local_lock_acquire(&__pcp->lock); \
__pcp; \
@@ -116,7 +116,7 @@ static inline void local_lock_release(lo
({ \
type *__pcp; \
\
- local_irq_save(flags); \
+ ll_local_irq_save(flags); \
__pcp = this_cpu_ptr(pcp); \
local_lock_acquire(&__pcp->lock); \
__pcp; \
where ll_xxx is defined as xxx for non-RT and on RT all of them
get mapped to migrate_disable().
Thoughts?
Thanks,
tglx
