[PATCH 00/32] softirq: Per vector masking v2

[PATCH 00/32] softirq: Per vector masking v2

[Frederic Weisbecker]

For those who missed the infinitely invasive and carpal tunnel
unfriendly v1: https://lwn.net/Articles/768157/

Softirqs masking is an all or nothing operation. It's currently not
possible to disable a single vector. Yet some workloads are interested
in deterministic latencies for vectors execution. Reducing their
interdependencies is a first step toward that.

Unlike the previous take, the current APIs aren't changed, as advised
by reviewers. But new APIs have been introduced. An individual vector
can be disabled and that behaviour can self-nest and nest with existing
APIs:

    bh = local_bh_disable_mask(BIT(TASKLET_SOFTIRQ));
	    bh2 = spin_lock_bh_mask(lock, BIT(NET_RX_SOFTIRQ));
	        local_bh_disable();
            [...]
            local_bh_enable();
        spin_unlock_bh_mask(lock, bh2);
    local_bh_enable_mask(bh));

Also mandatory, the new version provides lockdep validation in a per
vector finegrained way.

The next step could be to allow soft-interrupting softirq vectors with
other vectors. We'll need to be careful about stack usage and
interdependencies though. But that could solve issues with long lasting
vectors running at the expense of others.

A few details need improvement:

* We need to set all vectors of local_softirq_enabled() on boot for all
  archs. Only x86 does it for now. Shouldn't be too hard to achieve though.

* Handle multiple usage on lockdep verbose debugging (see patch PATCH 10/32)
  Also easy to fix.

* Restore redundant softirqs on tracking on softirq. See
  "locking/lockdep: Remove redundant softirqs on check". Also shouldn't
  be too hard to fix.

git://git.kernel.org/pub/scm/linux/kernel/git/frederic/linux-dynticks.git
	softirq/soft-interruptible

HEAD: 0d35ad1a4b13b62e135672dfe86d362b49f41abf

Thanks,
	Frederic
---

Frederic Weisbecker (32):
      locking/lockdep: Use expanded masks on find_usage_*() functions
      locking/lockdep: Introduce struct lock_usage
      locking/lockdep: Convert usage_mask to u64
      locking/lockdep: Test all incompatible scenario at once in check_irq_usage()
      locking/lockdep: Prepare valid_state() to handle plain masks
      locking/lockdep: Prepare check_usage_*() to handle plain masks
      locking/lockdep: Prepare state_verbose() to handle all softirqs
      locking/lockdep: Make mark_lock() fastpath to work with multiple usage at once
      locking/lockdep: Save stack trace for each softirq vector involved
      locking/lockdep: Make mark_lock() verbosity aware of vector
      softirq: Macrofy softirq vectors
      locking/lockdep: Define per vector softirq lock usage states
      softirq: Pass softirq vector number to lockdep on vector execution
      x86: Revert "x86/irq: Demote irq_cpustat_t::__softirq_pending to u16"
      arch/softirq: Rename softirq_pending fields to softirq_data
      softirq: Normalize softirq_pending naming scheme
      softirq: Convert softirq_pending_*() to set/clear mask scheme
      softirq: Introduce disabled softirq vectors bits
      softirq: Rename _local_bh_enable() to local_bh_enable_no_softirq()
      softirq: Move vectors bits to bottom_half.h
      x86: Init softirq enabled field
      softirq: Check enabled vectors before processing
      softirq: Remove stale comment
      softirq: Uninline !CONFIG_TRACE_IRQFLAGS __local_bh_disable_ip()
      softirq: Prepare for mixing all/per-vector masking
      softirq: Support per vector masking
      locking/lockdep: Remove redundant softirqs on check
      locking/lockdep: Update check_flags() according to new layout
      locking/lockdep: Branch the new vec-finegrained softirq masking to lockdep
      softirq: Allow to soft interrupt vector-specific masked contexts
      locking: Introduce spin_[un]lock_bh_mask()
      net: Make softirq vector masking finegrained on release_sock()


 arch/arm/include/asm/hardirq.h      |   2 +-
 arch/arm64/include/asm/hardirq.h    |   2 +-
 arch/h8300/kernel/asm-offsets.c     |   2 +-
 arch/ia64/include/asm/hardirq.h     |   2 +-
 arch/ia64/include/asm/processor.h   |   2 +-
 arch/m68k/include/asm/hardirq.h     |   2 +-
 arch/m68k/kernel/asm-offsets.c      |   2 +-
 arch/parisc/include/asm/hardirq.h   |   2 +-
 arch/powerpc/include/asm/hardirq.h  |   2 +-
 arch/s390/include/asm/hardirq.h     |  11 +-
 arch/s390/lib/delay.c               |   2 +-
 arch/sh/include/asm/hardirq.h       |   2 +-
 arch/sparc/include/asm/cpudata_64.h |   2 +-
 arch/sparc/include/asm/hardirq_64.h |   4 +-
 arch/um/include/asm/hardirq.h       |   2 +-
 arch/x86/include/asm/hardirq.h      |   2 +-
 arch/x86/kernel/irq.c               |   5 +-
 drivers/s390/char/sclp.c            |   2 +-
 drivers/s390/cio/cio.c              |   2 +-
 include/asm-generic/hardirq.h       |   2 +-
 include/linux/bottom_half.h         |  41 +++-
 include/linux/interrupt.h           |  87 ++++---
 include/linux/irqflags.h            |  12 +-
 include/linux/lockdep.h             |   5 +-
 include/linux/softirq_vector.h      |  10 +
 include/linux/spinlock.h            |  14 ++
 include/linux/spinlock_api_smp.h    |  26 ++
 include/linux/spinlock_api_up.h     |  13 +
 kernel/locking/lockdep.c            | 465 ++++++++++++++++++++++++------------
 kernel/locking/lockdep_internals.h  |  50 +++-
 kernel/locking/lockdep_proc.c       |   2 +-
 kernel/locking/lockdep_states.h     |   4 +-
 kernel/locking/spinlock.c           |  19 ++
 kernel/softirq.c                    | 159 ++++++++----
 lib/locking-selftest.c              |   4 +-
 net/core/sock.c                     |   6 +-
 36 files changed, 690 insertions(+), 281 deletions(-)

[PATCH 01/32] locking/lockdep: Use expanded masks on find_usage_*() functions

[Frederic Weisbecker]

In order to perform softirq vector-finegrained locking validation we'll
need to be able to check multiple vector usages at once. Prepare the low
level usage mask check functions for that purpose.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/locking/lockdep.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 608f74ed8bb9..6127cef4f8fb 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -1335,9 +1335,9 @@ check_redundant(struct lock_list *root, struct lock_class *target,
  * without creating any illegal irq-safe -> irq-unsafe lock dependency.
  */
 
-static inline int usage_match(struct lock_list *entry, void *bit)
+static inline int usage_match(struct lock_list *entry, void *mask)
 {
-	return entry->class->usage_mask & (1 << (enum lock_usage_bit)bit);
+	return entry->class->usage_mask & *(u64 *)mask;
 }
 
 
@@ -1353,14 +1353,14 @@ static inline int usage_match(struct lock_list *entry, void *bit)
  * Return <0 on error.
  */
 static int
-find_usage_forwards(struct lock_list *root, enum lock_usage_bit bit,
+find_usage_forwards(struct lock_list *root, u64 usage_mask,
 			struct lock_list **target_entry)
 {
 	int result;
 
 	debug_atomic_inc(nr_find_usage_forwards_checks);
 
-	result = __bfs_forwards(root, (void *)bit, usage_match, target_entry);
+	result = __bfs_forwards(root, &usage_mask, usage_match, target_entry);
 
 	return result;
 }
@@ -1376,14 +1376,14 @@ find_usage_forwards(struct lock_list *root, enum lock_usage_bit bit,
  * Return <0 on error.
  */
 static int
-find_usage_backwards(struct lock_list *root, enum lock_usage_bit bit,
+find_usage_backwards(struct lock_list *root, u64 usage_mask,
 			struct lock_list **target_entry)
 {
 	int result;
 
 	debug_atomic_inc(nr_find_usage_backwards_checks);
 
-	result = __bfs_backwards(root, (void *)bit, usage_match, target_entry);
+	result = __bfs_backwards(root, &usage_mask, usage_match, target_entry);
 
 	return result;
 }
@@ -1588,7 +1588,7 @@ check_usage(struct task_struct *curr, struct held_lock *prev,
 	this.parent = NULL;
 
 	this.class = hlock_class(prev);
-	ret = find_usage_backwards(&this, bit_backwards, &target_entry);
+	ret = find_usage_backwards(&this, BIT(bit_backwards), &target_entry);
 	if (ret < 0)
 		return print_bfs_bug(ret);
 	if (ret == 1)
@@ -1596,7 +1596,7 @@ check_usage(struct task_struct *curr, struct held_lock *prev,
 
 	that.parent = NULL;
 	that.class = hlock_class(next);
-	ret = find_usage_forwards(&that, bit_forwards, &target_entry1);
+	ret = find_usage_forwards(&that, BIT(bit_forwards), &target_entry1);
 	if (ret < 0)
 		return print_bfs_bug(ret);
 	if (ret == 1)
@@ -2553,7 +2553,7 @@ print_irq_inversion_bug(struct task_struct *curr,
  */
 static int
 check_usage_forwards(struct task_struct *curr, struct held_lock *this,
-		     enum lock_usage_bit bit, const char *irqclass)
+		     u64 usage_mask, const char *irqclass)
 {
 	int ret;
 	struct lock_list root;
@@ -2561,7 +2561,7 @@ check_usage_forwards(struct task_struct *curr, struct held_lock *this,
 
 	root.parent = NULL;
 	root.class = hlock_class(this);
-	ret = find_usage_forwards(&root, bit, &target_entry);
+	ret = find_usage_forwards(&root, usage_mask, &target_entry);
 	if (ret < 0)
 		return print_bfs_bug(ret);
 	if (ret == 1)
@@ -2577,7 +2577,7 @@ check_usage_forwards(struct task_struct *curr, struct held_lock *this,
  */
 static int
 check_usage_backwards(struct task_struct *curr, struct held_lock *this,
-		      enum lock_usage_bit bit, const char *irqclass)
+		      u64 usage_mask, const char *irqclass)
 {
 	int ret;
 	struct lock_list root;
@@ -2585,7 +2585,7 @@ check_usage_backwards(struct task_struct *curr, struct held_lock *this,
 
 	root.parent = NULL;
 	root.class = hlock_class(this);
-	ret = find_usage_backwards(&root, bit, &target_entry);
+	ret = find_usage_backwards(&root, usage_mask, &target_entry);
 	if (ret < 0)
 		return print_bfs_bug(ret);
 	if (ret == 1)
@@ -2644,7 +2644,7 @@ static inline int state_verbose(enum lock_usage_bit bit,
 }
 
 typedef int (*check_usage_f)(struct task_struct *, struct held_lock *,
-			     enum lock_usage_bit bit, const char *name);
+			     u64 usage_mask, const char *name);
 
 static int
 mark_lock_irq(struct task_struct *curr, struct held_lock *this,
@@ -2676,7 +2676,7 @@ mark_lock_irq(struct task_struct *curr, struct held_lock *this,
 	 * states.
 	 */
 	if ((!read || !dir || STRICT_READ_CHECKS) &&
-			!usage(curr, this, excl_bit, state_name(new_bit & ~LOCK_USAGE_READ_MASK)))
+	    !usage(curr, this, BIT(excl_bit), state_name(new_bit & ~LOCK_USAGE_READ_MASK)))
 		return 0;
 
 	/*
@@ -2687,7 +2687,7 @@ mark_lock_irq(struct task_struct *curr, struct held_lock *this,
 			return 0;
 
 		if (STRICT_READ_CHECKS &&
-			!usage(curr, this, excl_bit + LOCK_USAGE_READ_MASK,
+		    !usage(curr, this, BIT(excl_bit + LOCK_USAGE_READ_MASK),
 				state_name(new_bit + LOCK_USAGE_READ_MASK)))
 			return 0;
 	}
-- 
2.17.1

Re: [PATCH 01/32] locking/lockdep: Use expanded masks on find_usage_*() functions

[Linus Torvalds]

On Tue, Feb 12, 2019 at 9:14 AM Frederic Weisbecker <frederic@kernel.org> wrote:
>
> In order to perform softirq vector-finegrained locking validation we'll
> need to be able to check multiple vector usages at once. Prepare the low
> level usage mask check functions for that purpose.

Why is this using "u64 mask"?

That's not only fairly expensive on 32-bit targets, it wasn't what the
code did before:

> -static inline int usage_match(struct lock_list *entry, void *bit)
> +static inline int usage_match(struct lock_list *entry, void *mask)
>  {
> -       return entry->class->usage_mask & (1 << (enum lock_usage_bit)bit);
> +       return entry->class->usage_mask & *(u64 *)mask;

Note how that was an "int" mask value before, and "usage_mask" itself
is just "unsigned long".

So where does that "u64" come from?

If there is some reason you really want to use a 64-bit value (some
future change?), it should be documented.

                Linus

[PATCH 02/32] locking/lockdep: Introduce struct lock_usage

[Frederic Weisbecker]

In order to support softirq per-vector locking validation, we could
simply iterate over all enabled vectors and perform separate validation
for all of them on every lock event. We can expect that to introduce a
severe performance penalty though.

Therefore, we instead plan to validate the LOCK_USED_IN_*_SOFTIRQ and
LOCK_ENABLED_*_SOFTIRQ events with grouping the involved softirq vector
bits. It implies that we'll rather validate expanded usage mask than
usage bit in the end.

Before the usage mask to be expanded though, we need to play with the
usage bit that defines the nature of the event relevant to a group of
vectors.

Introduce struct lock_usage to implement that. This is made of the
classical lock usage bit that defines the nature of a lock usage
along which we carry the vectors to which that usage applies.

Once high level functions are done dealing with the nature of the lock
usage, lower level functions dealing with validation can expand the
struct lock_usage to a usage mask through lock_usage_mask().

For now, vector is always 0 until we get the proper vector finegrained
informations on softirq usage events.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/locking/lockdep.c           | 121 +++++++++++++++++------------
 kernel/locking/lockdep_internals.h |   6 ++
 2 files changed, 79 insertions(+), 48 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 6127cef4f8fb..1bb955d22eae 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2466,6 +2466,11 @@ print_usage_bug(struct task_struct *curr, struct held_lock *this,
 	return 0;
 }
 
+static u64 lock_usage_mask(struct lock_usage *usage)
+{
+	return BIT(usage->bit);
+}
+
 /*
  * Print out an error if an invalid bit is set:
  */
@@ -2479,7 +2484,7 @@ valid_state(struct task_struct *curr, struct held_lock *this,
 }
 
 static int mark_lock(struct task_struct *curr, struct held_lock *this,
-		     enum lock_usage_bit new_bit);
+		     struct lock_usage *new_usage);
 
 #if defined(CONFIG_TRACE_IRQFLAGS) && defined(CONFIG_PROVE_LOCKING)
 
@@ -2648,11 +2653,14 @@ typedef int (*check_usage_f)(struct task_struct *, struct held_lock *,
 
 static int
 mark_lock_irq(struct task_struct *curr, struct held_lock *this,
-		enum lock_usage_bit new_bit)
+	      struct lock_usage *new_usage)
 {
-	int excl_bit = exclusive_bit(new_bit);
-	int read = new_bit & LOCK_USAGE_READ_MASK;
-	int dir = new_bit & LOCK_USAGE_DIR_MASK;
+	struct lock_usage excl_usage = {
+		.bit = exclusive_bit(new_usage->bit),
+		.vector = new_usage->vector
+	};
+	int read = new_usage->bit & LOCK_USAGE_READ_MASK;
+	int dir = new_usage->bit & LOCK_USAGE_DIR_MASK;
 
 	/*
 	 * mark USED_IN has to look forwards -- to ensure no dependency
@@ -2668,7 +2676,7 @@ mark_lock_irq(struct task_struct *curr, struct held_lock *this,
 	 * Validate that this particular lock does not have conflicting
 	 * usage states.
 	 */
-	if (!valid_state(curr, this, new_bit, excl_bit))
+	if (!valid_state(curr, this, new_usage->bit, excl_usage.bit))
 		return 0;
 
 	/*
@@ -2676,23 +2684,24 @@ mark_lock_irq(struct task_struct *curr, struct held_lock *this,
 	 * states.
 	 */
 	if ((!read || !dir || STRICT_READ_CHECKS) &&
-	    !usage(curr, this, BIT(excl_bit), state_name(new_bit & ~LOCK_USAGE_READ_MASK)))
+	    !usage(curr, this, lock_usage_mask(&excl_usage), state_name(new_usage->bit & ~LOCK_USAGE_READ_MASK)))
 		return 0;
 
 	/*
 	 * Check for read in write conflicts
 	 */
 	if (!read) {
-		if (!valid_state(curr, this, new_bit, excl_bit + LOCK_USAGE_READ_MASK))
+		excl_usage.bit += LOCK_USAGE_READ_MASK;
+		if (!valid_state(curr, this, new_usage->bit, excl_usage.bit))
 			return 0;
 
 		if (STRICT_READ_CHECKS &&
-		    !usage(curr, this, BIT(excl_bit + LOCK_USAGE_READ_MASK),
-				state_name(new_bit + LOCK_USAGE_READ_MASK)))
+		    !usage(curr, this, lock_usage_mask(&excl_usage),
+			   state_name(new_usage->bit + LOCK_USAGE_READ_MASK)))
 			return 0;
 	}
 
-	if (state_verbose(new_bit, hlock_class(this)))
+	if (state_verbose(new_usage->bit, hlock_class(this)))
 		return 2;
 
 	return 1;
@@ -2702,24 +2711,24 @@ mark_lock_irq(struct task_struct *curr, struct held_lock *this,
  * Mark all held locks with a usage bit:
  */
 static int
-mark_held_locks(struct task_struct *curr, enum lock_usage_bit base_bit)
+mark_held_locks(struct task_struct *curr, struct lock_usage *base_usage)
 {
 	struct held_lock *hlock;
 	int i;
 
 	for (i = 0; i < curr->lockdep_depth; i++) {
-		enum lock_usage_bit hlock_bit = base_bit;
+		struct lock_usage hlock_usage = *base_usage;
 		hlock = curr->held_locks + i;
 
 		if (hlock->read)
-			hlock_bit += LOCK_USAGE_READ_MASK;
+			hlock_usage.bit += LOCK_USAGE_READ_MASK;
 
-		BUG_ON(hlock_bit >= LOCK_USAGE_STATES);
+		BUG_ON(hlock_usage.bit >= LOCK_USAGE_STATES);
 
 		if (!hlock->check)
 			continue;
 
-		if (!mark_lock(curr, hlock, hlock_bit))
+		if (!mark_lock(curr, hlock, &hlock_usage))
 			return 0;
 	}
 
@@ -2732,6 +2741,7 @@ mark_held_locks(struct task_struct *curr, enum lock_usage_bit base_bit)
 static void __trace_hardirqs_on_caller(unsigned long ip)
 {
 	struct task_struct *curr = current;
+	struct lock_usage usage = { .bit = LOCK_ENABLED_HARDIRQ };
 
 	/* we'll do an OFF -> ON transition: */
 	curr->hardirqs_enabled = 1;
@@ -2740,16 +2750,18 @@ static void __trace_hardirqs_on_caller(unsigned long ip)
 	 * We are going to turn hardirqs on, so set the
 	 * usage bit for all held locks:
 	 */
-	if (!mark_held_locks(curr, LOCK_ENABLED_HARDIRQ))
+	if (!mark_held_locks(curr, &usage))
 		return;
 	/*
 	 * If we have softirqs enabled, then set the usage
 	 * bit for all held locks. (disabled hardirqs prevented
 	 * this bit from being set before)
 	 */
-	if (curr->softirqs_enabled)
-		if (!mark_held_locks(curr, LOCK_ENABLED_SOFTIRQ))
+	if (curr->softirqs_enabled) {
+		usage.bit = LOCK_ENABLED_SOFTIRQ;
+		if (!mark_held_locks(curr, &usage))
 			return;
+	}
 
 	curr->hardirq_enable_ip = ip;
 	curr->hardirq_enable_event = ++curr->irq_events;
@@ -2832,6 +2844,9 @@ void lockdep_hardirqs_off(unsigned long ip)
 void trace_softirqs_on(unsigned long ip)
 {
 	struct task_struct *curr = current;
+	struct lock_usage usage = {
+		.bit = LOCK_ENABLED_SOFTIRQ,
+	};
 
 	if (unlikely(!debug_locks || current->lockdep_recursion))
 		return;
@@ -2862,7 +2877,7 @@ void trace_softirqs_on(unsigned long ip)
 	 * enabled too:
 	 */
 	if (curr->hardirqs_enabled)
-		mark_held_locks(curr, LOCK_ENABLED_SOFTIRQ);
+		mark_held_locks(curr, &usage);
 	current->lockdep_recursion = 0;
 }
 
@@ -2900,46 +2915,55 @@ void trace_softirqs_off(unsigned long ip)
 
 static int mark_irqflags(struct task_struct *curr, struct held_lock *hlock)
 {
+	struct lock_usage usage = { .vector = 0 };
 	/*
 	 * If non-trylock use in a hardirq or softirq context, then
 	 * mark the lock as used in these contexts:
 	 */
 	if (!hlock->trylock) {
 		if (hlock->read) {
-			if (curr->hardirq_context)
-				if (!mark_lock(curr, hlock,
-						LOCK_USED_IN_HARDIRQ_READ))
+			if (curr->hardirq_context) {
+				usage.bit = LOCK_USED_IN_HARDIRQ_READ;
+				if (!mark_lock(curr, hlock, &usage))
 					return 0;
-			if (curr->softirq_context)
-				if (!mark_lock(curr, hlock,
-						LOCK_USED_IN_SOFTIRQ_READ))
+			}
+			if (curr->softirq_context) {
+				usage.bit = LOCK_USED_IN_SOFTIRQ_READ;
+				if (!mark_lock(curr, hlock, &usage))
 					return 0;
+			}
 		} else {
-			if (curr->hardirq_context)
-				if (!mark_lock(curr, hlock, LOCK_USED_IN_HARDIRQ))
+			if (curr->hardirq_context) {
+				usage.bit = LOCK_USED_IN_HARDIRQ;
+				if (!mark_lock(curr, hlock, &usage))
 					return 0;
-			if (curr->softirq_context)
-				if (!mark_lock(curr, hlock, LOCK_USED_IN_SOFTIRQ))
+			}
+			if (curr->softirq_context) {
+				usage.bit = LOCK_USED_IN_SOFTIRQ;
+				if (!mark_lock(curr, hlock, &usage))
 					return 0;
+			}
 		}
 	}
 	if (!hlock->hardirqs_off) {
 		if (hlock->read) {
-			if (!mark_lock(curr, hlock,
-					LOCK_ENABLED_HARDIRQ_READ))
+			usage.bit = LOCK_ENABLED_HARDIRQ_READ;
+			if (!mark_lock(curr, hlock, &usage))
 				return 0;
-			if (curr->softirqs_enabled)
-				if (!mark_lock(curr, hlock,
-						LOCK_ENABLED_SOFTIRQ_READ))
+			if (curr->softirqs_enabled) {
+				usage.bit = LOCK_ENABLED_SOFTIRQ_READ;
+				if (!mark_lock(curr, hlock, &usage))
 					return 0;
+			}
 		} else {
-			if (!mark_lock(curr, hlock,
-					LOCK_ENABLED_HARDIRQ))
+			usage.bit = LOCK_ENABLED_HARDIRQ;
+			if (!mark_lock(curr, hlock, &usage))
 				return 0;
-			if (curr->softirqs_enabled)
-				if (!mark_lock(curr, hlock,
-						LOCK_ENABLED_SOFTIRQ))
+			if (curr->softirqs_enabled) {
+				usage.bit = LOCK_ENABLED_SOFTIRQ;
+				if (!mark_lock(curr, hlock, &usage))
 					return 0;
+			}
 		}
 	}
 
@@ -2978,7 +3002,7 @@ static int separate_irq_context(struct task_struct *curr,
 
 static inline
 int mark_lock_irq(struct task_struct *curr, struct held_lock *this,
-		enum lock_usage_bit new_bit)
+		  struct lock_usage *new_usage)
 {
 	WARN_ON(1); /* Impossible innit? when we don't have TRACE_IRQFLAG */
 	return 1;
@@ -3007,9 +3031,9 @@ static inline int separate_irq_context(struct task_struct *curr,
  * Mark a lock with a usage bit, and validate the state transition:
  */
 static int mark_lock(struct task_struct *curr, struct held_lock *this,
-			     enum lock_usage_bit new_bit)
+		     struct lock_usage *new_usage)
 {
-	unsigned int new_mask = 1 << new_bit, ret = 1;
+	u64 new_mask = lock_usage_mask(new_usage), ret = 1;
 
 	/*
 	 * If already set then do not dirty the cacheline,
@@ -3030,10 +3054,10 @@ static int mark_lock(struct task_struct *curr, struct held_lock *this,
 
 	hlock_class(this)->usage_mask |= new_mask;
 
-	if (!save_trace(hlock_class(this)->usage_traces + new_bit))
+	if (!save_trace(hlock_class(this)->usage_traces + new_usage->bit))
 		return 0;
 
-	switch (new_bit) {
+	switch (new_usage->bit) {
 #define LOCKDEP_STATE(__STATE)			\
 	case LOCK_USED_IN_##__STATE:		\
 	case LOCK_USED_IN_##__STATE##_READ:	\
@@ -3041,7 +3065,7 @@ static int mark_lock(struct task_struct *curr, struct held_lock *this,
 	case LOCK_ENABLED_##__STATE##_READ:
 #include "lockdep_states.h"
 #undef LOCKDEP_STATE
-		ret = mark_lock_irq(curr, this, new_bit);
+		ret = mark_lock_irq(curr, this, new_usage);
 		if (!ret)
 			return 0;
 		break;
@@ -3061,7 +3085,7 @@ static int mark_lock(struct task_struct *curr, struct held_lock *this,
 	 * We must printk outside of the graph_lock:
 	 */
 	if (ret == 2) {
-		printk("\nmarked lock as {%s}:\n", usage_str[new_bit]);
+		printk("\nmarked lock as {%s}:\n", usage_str[new_usage->bit]);
 		print_lock(this);
 		print_irqtrace_events(curr);
 		dump_stack();
@@ -3185,6 +3209,7 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass,
 {
 	struct task_struct *curr = current;
 	struct lock_class *class = NULL;
+	struct lock_usage usage = { .bit = LOCK_USED };
 	struct held_lock *hlock;
 	unsigned int depth;
 	int chain_head = 0;
@@ -3278,7 +3303,7 @@ static int __lock_acquire(struct lockdep_map *lock, unsigned int subclass,
 		return 0;
 
 	/* mark it as used: */
-	if (!mark_lock(curr, hlock, LOCK_USED))
+	if (!mark_lock(curr, hlock, &usage))
 		return 0;
 
 	/*
diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h
index 2ebb9d0ea91c..e714c823f594 100644
--- a/kernel/locking/lockdep_internals.h
+++ b/kernel/locking/lockdep_internals.h
@@ -26,6 +26,12 @@ enum lock_usage_bit {
 #define LOCK_USAGE_DIR_MASK  2
 #define LOCK_USAGE_STATE_MASK (~(LOCK_USAGE_READ_MASK | LOCK_USAGE_DIR_MASK))
 
+struct lock_usage {
+	enum lock_usage_bit bit;
+	unsigned long vector; /* Softirq vector */
+};
+
+
 /*
  * Usage-state bitmasks:
  */
-- 
2.17.1

Re: [PATCH 02/32] locking/lockdep: Introduce struct lock_usage

[Linus Torvalds]

On Tue, Feb 12, 2019 at 9:14 AM Frederic Weisbecker <frederic@kernel.org> wrote:
>
> +static u64 lock_usage_mask(struct lock_usage *usage)
> +{
> +       return BIT(usage->bit);
> +}

More insane "u64" - and it's *incorrect* too.

    #define BIT(nr)                    (1UL << (nr))

fundamentally means that "BIT()" can only work on up to "unsigned long".

So this odd use of u64 seems to be a disease. It only uses more memory
(and more CPU) for no obvious reason.

u64 is not some "default type". It's expensive and shouldn't be used
unless you have a *reason* for it.

              Linus

Re: [PATCH 02/32] locking/lockdep: Introduce struct lock_usage

[Frederic Weisbecker]

On Tue, Feb 12, 2019 at 09:38:42AM -0800, Linus Torvalds wrote:
> On Tue, Feb 12, 2019 at 9:14 AM Frederic Weisbecker <frederic@kernel.org> wrote:
> >
> > +static u64 lock_usage_mask(struct lock_usage *usage)
> > +{
> > +       return BIT(usage->bit);
> > +}
> 
> More insane "u64" - and it's *incorrect* too.
> 
>     #define BIT(nr)                    (1UL << (nr))
> 
> fundamentally means that "BIT()" can only work on up to "unsigned long".
> 
> So this odd use of u64 seems to be a disease. It only uses more memory
> (and more CPU) for no obvious reason.
> 
> u64 is not some "default type". It's expensive and shouldn't be used
> unless you have a *reason* for it.

Right, I'll simply move "[PATCH 03/32] locking/lockdep: Convert usage_mask to u64"
at the first position and follow up on that to justify its use.

Thanks.

[PATCH 03/32] locking/lockdep: Convert usage_mask to u64

[Frederic Weisbecker]

The usage mask is going to expand to validate softirq related usages in
a per-vector finegrained way.

The current bitmap layout is:

                  LOCK_USED        HARDIRQ bits
                        \            /
                         \          /
                          0  0000  0000
                               |
                               |
                          SOFTIRQ bits

The new one will be:

                                  TIMER_SOFTIRQ
                 LOCK_USED            bits       HARDIRQ bits
                     \                  |            |
                      \                 |            |
                      0   0000  [...]  0000  0000  0000
                            |                  |
                            |                  |
                       RCU_SOFTIRQ        HI_SOFTIRQ bits
                          bits

So we have 4 hardirq bits + NR_SOFTIRQS * 4 + 1 bit (LOCK_USED) = 45
bits. Therefore we need a 64 bits mask.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 include/linux/lockdep.h  | 2 +-
 kernel/locking/lockdep.c | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index c5335df2372f..06669f20a30a 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -83,7 +83,7 @@ struct lock_class {
 	/*
 	 * IRQ/softirq usage tracking bits:
 	 */
-	unsigned long			usage_mask;
+	u64				usage_mask;
 	struct stack_trace		usage_traces[XXX_LOCK_USAGE_STATES];
 
 	/*
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 1bb955d22eae..a977aa5976b7 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -463,9 +463,9 @@ const char * __get_key_name(struct lockdep_subclass_key *key, char *str)
 	return kallsyms_lookup((unsigned long)key, NULL, NULL, NULL, str);
 }
 
-static inline unsigned long lock_flag(enum lock_usage_bit bit)
+static inline u64 lock_flag(enum lock_usage_bit bit)
 {
-	return 1UL << bit;
+	return BIT_ULL(bit);
 }
 
 static char get_usage_char(struct lock_class *class, enum lock_usage_bit bit)
@@ -1400,7 +1400,7 @@ static void print_lock_class_header(struct lock_class *class, int depth)
 	printk(KERN_CONT " {\n");
 
 	for (bit = 0; bit < LOCK_USAGE_STATES; bit++) {
-		if (class->usage_mask & (1 << bit)) {
+		if (class->usage_mask & lock_flag(bit)) {
 			int len = depth;
 
 			len += printk("%*s   %s", depth, "", usage_str[bit]);
@@ -2478,7 +2478,7 @@ static inline int
 valid_state(struct task_struct *curr, struct held_lock *this,
 	    enum lock_usage_bit new_bit, enum lock_usage_bit bad_bit)
 {
-	if (unlikely(hlock_class(this)->usage_mask & (1 << bad_bit)))
+	if (unlikely(hlock_class(this)->usage_mask & lock_flag(bad_bit)))
 		return print_usage_bug(curr, this, bad_bit, new_bit);
 	return 1;
 }
-- 
2.17.1

Re: [PATCH 03/32] locking/lockdep: Convert usage_mask to u64

[Linus Torvalds]

On Tue, Feb 12, 2019 at 9:14 AM Frederic Weisbecker <frederic@kernel.org> wrote:
>
> The usage mask is going to expand to validate softirq related usages in
> a per-vector finegrained way.

So here you start explaining why you wanted that u64. Much too late,
considering that you already changed to u64 earlier.

                 Linus

Re: [PATCH 03/32] locking/lockdep: Convert usage_mask to u64

[Frederic Weisbecker]

On Tue, Feb 12, 2019 at 09:40:09AM -0800, Linus Torvalds wrote:
> On Tue, Feb 12, 2019 at 9:14 AM Frederic Weisbecker <frederic@kernel.org> wrote:
> >
> > The usage mask is going to expand to validate softirq related usages in
> > a per-vector finegrained way.
> 
> So here you start explaining why you wanted that u64. Much too late,
> considering that you already changed to u64 earlier.

Yes indeed, while recutting the set I forgot a few things back. I'll re-introduce
the u64 from that patch on.

Thanks.

[PATCH 04/32] locking/lockdep: Test all incompatible scenario at once in check_irq_usage()

[Frederic Weisbecker]

check_prev_add_irq() tests all incompatible scenarios one after the
other while adding a lock (@next) to a tree dependency (@prev):

	LOCK_USED_IN_HARDIRQ          vs         LOCK_ENABLED_HARDIRQ
	LOCK_USED_IN_HARDIRQ_READ     vs         LOCK_ENABLED_HARDIRQ
	LOCK_USED_IN_SOFTIRQ          vs         LOCK_ENABLED_SOFTIRQ
	LOCK_USED_IN_SOFTIRQ_READ     vs         LOCK_ENABLED_SOFTIRQ

Also for these four scenarios, we must at least iterate the @prev
backward dependency. Then if it matches the relevant LOCK_USED_* bit,
we must also iterate the @next forward dependency.

Therefore in the best case we iterate 4 times, in the worst case 8 times.

Now things are going to become much worse as we plan to add as much
LOCK_USED_IN_{$VEC}_SOFTIRQ[_READ] as we have softirq vectors, currently
10. So the expanded test would be at least 22 loops and at most 44. We
can't really afford that.

So we take a different approach to fix this:

1) Iterate through @prev backward dependencies and accumulate all the IRQ
   uses in a single mask.

2) Iterate through @next forward dependencies and try to find a lock
   whose usage is exclusive to the accumulated usages gathered in the
   previous step. If we find one (call it @lockA), we have found an
   incompatible use.

3) Iterate again through @prev backward dependency and find the lock
   whose usage matches @lockA in term of incompatibility. Call that
   lock @lockB.

4) Report the incompatible usages of @lockA and @lockB

If no incompatible use is found, the verification never goes beyond
step 2 which means at most two iterations.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/locking/lockdep.c | 204 ++++++++++++++++++++++++++-------------
 1 file changed, 137 insertions(+), 67 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index a977aa5976b7..578dd57f70d3 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -1329,6 +1329,14 @@ check_redundant(struct lock_list *root, struct lock_class *target,
 }
 
 #if defined(CONFIG_TRACE_IRQFLAGS) && defined(CONFIG_PROVE_LOCKING)
+
+static inline int usage_accumulate(struct lock_list *entry, void *mask)
+{
+	*(u64 *)mask |= entry->class->usage_mask;
+
+	return 0;
+}
+
 /*
  * Forwards and backwards subgraph searching, for the purposes of
  * proving that two subgraphs can be connected by a new dependency
@@ -1340,8 +1348,6 @@ static inline int usage_match(struct lock_list *entry, void *mask)
 	return entry->class->usage_mask & *(u64 *)mask;
 }
 
-
-
 /*
  * Find a node in the forwards-direction dependency sub-graph starting
  * at @root->class that matches @bit.
@@ -1575,39 +1581,6 @@ print_bad_irq_dependency(struct task_struct *curr,
 	return 0;
 }
 
-static int
-check_usage(struct task_struct *curr, struct held_lock *prev,
-	    struct held_lock *next, enum lock_usage_bit bit_backwards,
-	    enum lock_usage_bit bit_forwards, const char *irqclass)
-{
-	int ret;
-	struct lock_list this, that;
-	struct lock_list *uninitialized_var(target_entry);
-	struct lock_list *uninitialized_var(target_entry1);
-
-	this.parent = NULL;
-
-	this.class = hlock_class(prev);
-	ret = find_usage_backwards(&this, BIT(bit_backwards), &target_entry);
-	if (ret < 0)
-		return print_bfs_bug(ret);
-	if (ret == 1)
-		return ret;
-
-	that.parent = NULL;
-	that.class = hlock_class(next);
-	ret = find_usage_forwards(&that, BIT(bit_forwards), &target_entry1);
-	if (ret < 0)
-		return print_bfs_bug(ret);
-	if (ret == 1)
-		return ret;
-
-	return print_bad_irq_dependency(curr, &this, &that,
-			target_entry, target_entry1,
-			prev, next,
-			bit_backwards, bit_forwards, irqclass);
-}
-
 static const char *state_names[] = {
 #define LOCKDEP_STATE(__STATE) \
 	__stringify(__STATE),
@@ -1638,45 +1611,143 @@ static int exclusive_bit(int new_bit)
 	return state | (dir ^ LOCK_USAGE_DIR_MASK);
 }
 
+static u64 __invert_mask(u64 mask, bool read)
+{
+	u64 excl_mask = 0;
+	int bit = 0;
+
+	while (mask) {
+		long fs = __ffs64(mask) + 1;
+		int excl;
+
+		mask >>= fs;
+		bit += fs;
+		excl = exclusive_bit(bit - 1);
+		excl_mask |= lock_flag(excl);
+		if (read)
+			excl_mask |= lock_flag(excl | LOCK_USAGE_READ_MASK);
+	}
+
+	return excl_mask;
+}
+
+static u64 exclusive_mask(u64 mask)
+{
+	return __invert_mask(mask, false);
+}
+
+/*
+ * Retrieve the _possible_ original mask to which @mask is
+ * exclusive. Ie: this is the opposite of exclusive_mask().
+ * Note that 2 possible original bits can match an exclusive
+ * bit: one has LOCK_USAGE_READ_MASK set, the other has it
+ * cleared. So both are returned for each exclusive bit.
+ */
+static u64 original_mask(u64 mask)
+{
+	return __invert_mask(mask, true);
+}
+
+/*
+ * Find the first pair of bit match between an original
+ * usage mask and an exclusive usage mask.
+ */
+static int find_exclusive_match(u64 mask, u64 excl_mask,
+				enum lock_usage_bit *bit,
+				enum lock_usage_bit *excl_bit)
+{
+	int nr = 0;
+
+	while (mask) {
+		long fs = __ffs64(mask) + 1;
+		int excl;
+
+		mask >>= fs;
+		nr += fs;
+
+		excl = exclusive_bit(nr - 1);
+		if (excl_mask & lock_flag(excl)) {
+			*bit = nr - 1;
+			*excl_bit = excl;
+			return 0;
+		}
+	}
+	return -1;
+}
+
+/*
+ * Prove that the new dependency does not connect a hardirq-safe(-read)
+ * lock with a hardirq-unsafe lock - to achieve this we search
+ * the backwards-subgraph starting at <prev>, and the
+ * forwards-subgraph starting at <next>:
+ */
 static int check_irq_usage(struct task_struct *curr, struct held_lock *prev,
-			   struct held_lock *next, enum lock_usage_bit bit)
+			   struct held_lock *next)
 {
+	enum lock_usage_bit forward_bit = 0, backward_bit = 0;
+	struct lock_list *uninitialized_var(target_entry1);
+	struct lock_list *uninitialized_var(target_entry);
+	u64 usage_mask = 0, forward_mask, backward_mask;
+	struct lock_list this, that;
+	int ret;
+
 	/*
-	 * Prove that the new dependency does not connect a hardirq-safe
-	 * lock with a hardirq-unsafe lock - to achieve this we search
-	 * the backwards-subgraph starting at <prev>, and the
-	 * forwards-subgraph starting at <next>:
+	 * Step 1: gather all hard/soft IRQs usages backward in an
+	 * accumulated usage mask.
 	 */
-	if (!check_usage(curr, prev, next, bit,
-			   exclusive_bit(bit), state_name(bit)))
-		return 0;
+	this.parent = NULL;
+	this.class = hlock_class(prev);
+
+	ret = __bfs_backwards(&this, &usage_mask, usage_accumulate, NULL);
+	if (ret < 0)
+		return print_bfs_bug(ret);
 
-	bit++; /* _READ */
+	usage_mask &= LOCKF_USED_IN_IRQ | LOCKF_USED_IN_IRQ_READ;
+	if (!usage_mask)
+		return 1;
 
 	/*
-	 * Prove that the new dependency does not connect a hardirq-safe-read
-	 * lock with a hardirq-unsafe lock - to achieve this we search
-	 * the backwards-subgraph starting at <prev>, and the
-	 * forwards-subgraph starting at <next>:
+	 * Step 2: find exclusive uses forward that match the previous
+	 * backward accumulated mask.
 	 */
-	if (!check_usage(curr, prev, next, bit,
-			   exclusive_bit(bit), state_name(bit)))
-		return 0;
+	forward_mask = exclusive_mask(usage_mask);
 
-	return 1;
-}
+	that.parent = NULL;
+	that.class = hlock_class(next);
 
-static int
-check_prev_add_irq(struct task_struct *curr, struct held_lock *prev,
-		struct held_lock *next)
-{
-#define LOCKDEP_STATE(__STATE)						\
-	if (!check_irq_usage(curr, prev, next, LOCK_USED_IN_##__STATE))	\
-		return 0;
-#include "lockdep_states.h"
-#undef LOCKDEP_STATE
+	ret = find_usage_forwards(&that, forward_mask, &target_entry1);
+	if (ret < 0)
+		return print_bfs_bug(ret);
+	if (ret == 1)
+		return ret;
+
+	/*
+	 * Step 3: we found a bad match! Now retrieve a lock from the backward
+	 * list whose usage mask matches the exclusive usage mask from the
+	 * lock found on the forward list.
+	 */
+	backward_mask = original_mask(target_entry1->class->usage_mask);
+
+	ret = find_usage_backwards(&this, backward_mask, &target_entry);
+	if (ret < 0)
+		return print_bfs_bug(ret);
+	if (ret == 1)
+		return print_bfs_bug(ret);
+
+	/*
+	 * Step 4: narrow down to a pair of incompatible usage bits
+	 * and report it.
+	 */
+	ret = find_exclusive_match(target_entry->class->usage_mask,
+				   target_entry1->class->usage_mask,
+				   &backward_bit, &forward_bit);
+	WARN_ON_ONCE(ret == -1);
 
-	return 1;
+	return print_bad_irq_dependency(curr, &this, &that,
+			target_entry, target_entry1,
+			prev, next,
+			backward_bit, forward_bit,
+			state_name(backward_bit));
 }
 
 static void inc_chains(void)
@@ -1693,9 +1764,8 @@ static void inc_chains(void)
 
 #else
 
-static inline int
-check_prev_add_irq(struct task_struct *curr, struct held_lock *prev,
-		struct held_lock *next)
+static inline int check_irq_usage(struct task_struct *curr,
+				  struct held_lock *prev, struct held_lock *next)
 {
 	return 1;
 }
@@ -1857,7 +1927,7 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev,
 	else if (unlikely(ret < 0))
 		return print_bfs_bug(ret);
 
-	if (!check_prev_add_irq(curr, prev, next))
+	if (!check_irq_usage(curr, prev, next))
 		return 0;
 
 	/*
-- 
2.17.1

[PATCH 05/32] locking/lockdep: Prepare valid_state() to handle plain masks

[Frederic Weisbecker]

mark_lock_irq() is going to deal with lock usages that gather multiple
softirq vectors at once. Therefore the validation through valid_state()
will need to handle expanded usage masks.

So enhance valid_state() to that purpose.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/locking/lockdep.c | 44 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 39 insertions(+), 5 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 578dd57f70d3..6b625b70598a 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2538,7 +2538,26 @@ print_usage_bug(struct task_struct *curr, struct held_lock *this,
 
 static u64 lock_usage_mask(struct lock_usage *usage)
 {
-	return BIT(usage->bit);
+	int nr = 0;
+	u64 vectors = usage->vector;
+	u64 mask = 0ULL;
+
+	if (!vectors)
+		return lock_flag(usage->bit);
+
+	/* Only softirqs can have non-zero vectors */
+	WARN_ON_ONCE(usage->bit < LOCK_USED_IN_SOFTIRQ ||
+		     usage->bit > LOCK_ENABLED_SOFTIRQ_READ);
+
+	while (vectors) {
+		long fs = __ffs64(vectors) + 1;
+
+		vectors >>= fs;
+		nr += fs;
+		mask |= lock_flag(usage->bit) << (4 * (nr - 1));
+	}
+
+	return mask;
 }
 
 /*
@@ -2546,10 +2565,23 @@ static u64 lock_usage_mask(struct lock_usage *usage)
  */
 static inline int
 valid_state(struct task_struct *curr, struct held_lock *this,
-	    enum lock_usage_bit new_bit, enum lock_usage_bit bad_bit)
+	    u64 new_mask, u64 bad_mask)
 {
-	if (unlikely(hlock_class(this)->usage_mask & lock_flag(bad_bit)))
+	u64 bad_intersec;
+
+	bad_intersec = hlock_class(this)->usage_mask & bad_mask;
+
+	if (unlikely(bad_intersec)) {
+		enum lock_usage_bit new_bit, bad_bit;
+		int err;
+
+		err = find_exclusive_match(new_mask,
+					   bad_intersec, &new_bit, &bad_bit);
+		if (WARN_ON_ONCE(err < 0))
+			return err;
+
 		return print_usage_bug(curr, this, bad_bit, new_bit);
+	}
 	return 1;
 }
 
@@ -2746,7 +2778,8 @@ mark_lock_irq(struct task_struct *curr, struct held_lock *this,
 	 * Validate that this particular lock does not have conflicting
 	 * usage states.
 	 */
-	if (!valid_state(curr, this, new_usage->bit, excl_usage.bit))
+	if (!valid_state(curr, this, lock_usage_mask(new_usage),
+			 lock_usage_mask(&excl_usage)))
 		return 0;
 
 	/*
@@ -2762,7 +2795,8 @@ mark_lock_irq(struct task_struct *curr, struct held_lock *this,
 	 */
 	if (!read) {
 		excl_usage.bit += LOCK_USAGE_READ_MASK;
-		if (!valid_state(curr, this, new_usage->bit, excl_usage.bit))
+		if (!valid_state(curr, this, lock_usage_mask(new_usage),
+				 lock_usage_mask(&excl_usage)))
 			return 0;
 
 		if (STRICT_READ_CHECKS &&
-- 
2.17.1

Re: [PATCH 05/32] locking/lockdep: Prepare valid_state() to handle plain masks

[Linus Torvalds]

On Tue, Feb 12, 2019 at 9:14 AM Frederic Weisbecker <frederic@kernel.org> wrote:
>
> +
> +       while (vectors) {
> +               long fs = __ffs64(vectors) + 1;
> +
> +               vectors >>= fs;

This is wrong.

If "vectors" only has the high hit set, you end up with "fs" having
the value "64".

And then "vectors >>= fs" is undefined and won't actually do anything
at all on x86.

In general, avoid "ffs()", and the stupid pattern of "__ffs(x)+1".

Bit numbering starts at 0. "ffs()" is wrong. And you never *ever* just
add one to a bit number in order to shift by one more bit, exactly
because of overflow issues.

So it may look inefficient, but the correct thing to do is

    long bit = __ffs64(vectors);
    vectors >>= fs;
    vectors >>= 1;

because that actually works.

                     Linus

Re: [PATCH 05/32] locking/lockdep: Prepare valid_state() to handle plain masks

[Frederic Weisbecker]

On Tue, Feb 12, 2019 at 09:45:52AM -0800, Linus Torvalds wrote:
> On Tue, Feb 12, 2019 at 9:14 AM Frederic Weisbecker <frederic@kernel.org> wrote:
> >
> > +
> > +       while (vectors) {
> > +               long fs = __ffs64(vectors) + 1;
> > +
> > +               vectors >>= fs;
> 
> This is wrong.
> 
> If "vectors" only has the high hit set, you end up with "fs" having
> the value "64".
> 
> And then "vectors >>= fs" is undefined and won't actually do anything
> at all on x86.

Oh! ok didn't know that...

> 
> In general, avoid "ffs()", and the stupid pattern of "__ffs(x)+1".
> 
> Bit numbering starts at 0. "ffs()" is wrong. And you never *ever* just
> add one to a bit number in order to shift by one more bit, exactly
> because of overflow issues.
> 
> So it may look inefficient, but the correct thing to do is
> 
>     long bit = __ffs64(vectors);
>     vectors >>= fs;
>     vectors >>= 1;
> 
> because that actually works.

I see, perhaps I should use for_each_set_bit() that should take care about those
details?

Thanks.

Re: [PATCH 05/32] locking/lockdep: Prepare valid_state() to handle plain masks

[Linus Torvalds]

On Wed, Feb 13, 2019 at 7:16 AM Frederic Weisbecker <frederic@kernel.org> wrote:
> >
> > If "vectors" only has the high hit set, you end up with "fs" having
> > the value "64".
> >
> > And then "vectors >>= fs" is undefined and won't actually do anything
> > at all on x86.
>
> Oh! ok didn't know that...

So in general, shift counts >= width of the type (or negative) are undefined.

They can sometimes happen to work (that's the "undefined" part ;), but
it's not reliable or portable.

It's why you occasionally see things like

drivers/block/sx8.c:
        tmp             = (blk_rq_pos(rq) >> 16) >> 16;

to get the upper 32 bits of the value. It is written with that odd
double shift, rather than being written as ">> 32". That way it works
even if the sector type happens to be 32-bit (and the compiler will
just end up turning it into a zero if it's an unsigned 32-bit type
since it's compile-time obvious).

> I see, perhaps I should use for_each_set_bit() that should take care about those
> details?

That would _work_, but don't do that. "for_each_set_bit()" works on
bitmaps in memory, and is slow for a simple word case. In addition to
being slow, it uses the Linux tradition of working on bitmaps that are
comprised of "unsigned long". So it has byte order issues too.

So for_each_set_bit() is useful when you have real arrays of bits and
are using the "set_bit()" etc interfaces.

When you're actually working on just a single variable, your "__ffs()"
model works fine, you just need to be careful to _not_ do the "+1" and
then use it for shifts.

Also, it actually turns out that if you want to be really clever, you
can play tricks if you don't care about the exact bit *number*.

For example, this expression:

       v =  a & (a-1);

will remove the lowest bit set from 'a' very cheaply. So what you can
do is something like this:

    void for_each_bit_in_mask(u64 mask)
    {
        while (mask) {
                u64 newmask = mask & (mask-1);
                u64 onebit = mask ^ newmask;
                mask = newmask;
                do_something_with(onebit);
        }
    }

to do some operation on each bit set, and quite efficiently and
without any undefined behavior or expensive shifts.

But the above trick does require that you are happy to just see the
bit *mask*, not the bit *number*. I'm not sure any of your cases match
that.

                Linus

Re: [PATCH 05/32] locking/lockdep: Prepare valid_state() to handle plain masks

[Frederic Weisbecker]

On Wed, Feb 13, 2019 at 11:47:13AM -0800, Linus Torvalds wrote:
> On Wed, Feb 13, 2019 at 7:16 AM Frederic Weisbecker <frederic@kernel.org> wrote:
> > >
> > > If "vectors" only has the high hit set, you end up with "fs" having
> > > the value "64".
> > >
> > > And then "vectors >>= fs" is undefined and won't actually do anything
> > > at all on x86.
> >
> > Oh! ok didn't know that...
> 
> So in general, shift counts >= width of the type (or negative) are undefined.
> 
> They can sometimes happen to work (that's the "undefined" part ;), but
> it's not reliable or portable.
> 
> It's why you occasionally see things like
> 
> drivers/block/sx8.c:
>         tmp             = (blk_rq_pos(rq) >> 16) >> 16;
> 
> to get the upper 32 bits of the value. It is written with that odd
> double shift, rather than being written as ">> 32". That way it works
> even if the sector type happens to be 32-bit (and the compiler will
> just end up turning it into a zero if it's an unsigned 32-bit type
> since it's compile-time obvious).

Ok, I see.

> 
> > I see, perhaps I should use for_each_set_bit() that should take care about those
> > details?
> 
> That would _work_, but don't do that. "for_each_set_bit()" works on
> bitmaps in memory, and is slow for a simple word case. In addition to
> being slow, it uses the Linux tradition of working on bitmaps that are
> comprised of "unsigned long". So it has byte order issues too.
> 
> So for_each_set_bit() is useful when you have real arrays of bits and
> are using the "set_bit()" etc interfaces.

Yeah I suspected some overhead.

> 
> When you're actually working on just a single variable, your "__ffs()"
> model works fine, you just need to be careful to _not_ do the "+1" and
> then use it for shifts.
> 
> Also, it actually turns out that if you want to be really clever, you
> can play tricks if you don't care about the exact bit *number*.
> 
> For example, this expression:
> 
>        v =  a & (a-1);
> 
> will remove the lowest bit set from 'a' very cheaply. So what you can
> do is something like this:
> 
>     void for_each_bit_in_mask(u64 mask)
>     {
>         while (mask) {
>                 u64 newmask = mask & (mask-1);
>                 u64 onebit = mask ^ newmask;
>                 mask = newmask;
>                 do_something_with(onebit);
>         }
>     }
> 
> to do some operation on each bit set, and quite efficiently and
> without any undefined behavior or expensive shifts.
> 
> But the above trick does require that you are happy to just see the
> bit *mask*, not the bit *number*. I'm not sure any of your cases match
> that.

Nice, I couldn't resist introducing such a headache in my set ;-) unfortunately
I indeed need the bit number itself most of the time. 

So following your 1st advice, I should rather do something along the lines of:

   nr = 0;
   while (mask) {
       fs = __ffs64(mask);
       mask >>= fs;
       mask >>= 1;
       nr += fs + 1;
       process_bit_nr(nr - 1);
   }

And define a for_each_lock_usage_bit(usage_mask) on top of it.

Thanks a lot!

[PATCH 06/32] locking/lockdep: Prepare check_usage_*() to handle plain masks

[Frederic Weisbecker]

mark_lock_irq() is going to deal with lock usages that gather multiple
softirq vectors at once. Therefore the validation through
check_usage_backwards() and check_usage_forwards() will need to handle
expanded usage masks.

So enhance those functions to that purpose.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/locking/lockdep.c | 30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 6b625b70598a..bb66327e5cc3 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2660,9 +2660,12 @@ print_irq_inversion_bug(struct task_struct *curr,
  */
 static int
 check_usage_forwards(struct task_struct *curr, struct held_lock *this,
-		     u64 usage_mask, const char *irqclass)
+		     u64 usage_mask, bool read)
 {
 	int ret;
+	u64 intersec;
+	int intersec_bit;
+	const char *irqclass;
 	struct lock_list root;
 	struct lock_list *uninitialized_var(target_entry);
 
@@ -2674,8 +2677,14 @@ check_usage_forwards(struct task_struct *curr, struct held_lock *this,
 	if (ret == 1)
 		return ret;
 
+	intersec = usage_mask & target_entry->class->usage_mask;
+	intersec_bit = __ffs64(intersec);
+	if (read)
+		intersec_bit += LOCK_USAGE_READ_MASK;
+	irqclass = state_name(intersec_bit);
+
 	return print_irq_inversion_bug(curr, &root, target_entry,
-					this, 1, irqclass);
+				       this, 1, irqclass);
 }
 
 /*
@@ -2684,9 +2693,12 @@ check_usage_forwards(struct task_struct *curr, struct held_lock *this,
  */
 static int
 check_usage_backwards(struct task_struct *curr, struct held_lock *this,
-		      u64 usage_mask, const char *irqclass)
+		      u64 usage_mask, bool read)
 {
 	int ret;
+	u64 intersec;
+	int intersec_bit;
+	const char *irqclass;
 	struct lock_list root;
 	struct lock_list *uninitialized_var(target_entry);
 
@@ -2698,6 +2710,12 @@ check_usage_backwards(struct task_struct *curr, struct held_lock *this,
 	if (ret == 1)
 		return ret;
 
+	intersec = usage_mask & target_entry->class->usage_mask;
+	intersec_bit = __ffs64(intersec);
+	if (read)
+		intersec_bit += LOCK_USAGE_READ_MASK;
+	irqclass = state_name(intersec_bit);
+
 	return print_irq_inversion_bug(curr, &root, target_entry,
 					this, 0, irqclass);
 }
@@ -2751,7 +2769,7 @@ static inline int state_verbose(enum lock_usage_bit bit,
 }
 
 typedef int (*check_usage_f)(struct task_struct *, struct held_lock *,
-			     u64 usage_mask, const char *name);
+			     u64 usage_mask, bool read);
 
 static int
 mark_lock_irq(struct task_struct *curr, struct held_lock *this,
@@ -2787,7 +2805,7 @@ mark_lock_irq(struct task_struct *curr, struct held_lock *this,
 	 * states.
 	 */
 	if ((!read || !dir || STRICT_READ_CHECKS) &&
-	    !usage(curr, this, lock_usage_mask(&excl_usage), state_name(new_usage->bit & ~LOCK_USAGE_READ_MASK)))
+	    !usage(curr, this, lock_usage_mask(&excl_usage), false))
 		return 0;
 
 	/*
@@ -2801,7 +2819,7 @@ mark_lock_irq(struct task_struct *curr, struct held_lock *this,
 
 		if (STRICT_READ_CHECKS &&
 		    !usage(curr, this, lock_usage_mask(&excl_usage),
-			   state_name(new_usage->bit + LOCK_USAGE_READ_MASK)))
+			       true))
 			return 0;
 	}
 
-- 
2.17.1

[PATCH 07/32] locking/lockdep: Prepare state_verbose() to handle all softirqs

[Frederic Weisbecker]

Loosely handle all the softirq vectors verbosity under the same
function.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/locking/lockdep.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index bb66327e5cc3..baba291f2cab 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2755,17 +2755,13 @@ static int SOFTIRQ_verbose(struct lock_class *class)
 
 #define STRICT_READ_CHECKS	1
 
-static int (*state_verbose_f[])(struct lock_class *class) = {
-#define LOCKDEP_STATE(__STATE) \
-	__STATE##_verbose,
-#include "lockdep_states.h"
-#undef LOCKDEP_STATE
-};
-
 static inline int state_verbose(enum lock_usage_bit bit,
 				struct lock_class *class)
 {
-	return state_verbose_f[bit >> 2](class);
+	if (bit < LOCK_USED_IN_SOFTIRQ)
+		return HARDIRQ_verbose(class);
+	else
+		return SOFTIRQ_verbose(class);
 }
 
 typedef int (*check_usage_f)(struct task_struct *, struct held_lock *,
-- 
2.17.1

[PATCH 08/32] locking/lockdep: Make mark_lock() fastpath to work with multiple usage at once

[Frederic Weisbecker]

Now that mark_lock() is going to handle multiple softirq vectors at
once for a given lock usage, the current fast path optimization that
simply check if the new bit usage is already present won't work anymore.

Indeed if the new usage is only partially present, such as for some
softirq vectors and not for others, we may spuriously ignore all the
verifications for the new vectors.

What we must check instead is a bit different: we have to make sure that
the new usage with all its vectors are entirely present in the current
usage mask before ignoring further checks.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/locking/lockdep.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index baba291f2cab..9194f11d3dfb 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -3157,7 +3157,7 @@ static int mark_lock(struct task_struct *curr, struct held_lock *this,
 	 * If already set then do not dirty the cacheline,
 	 * nor do any checks:
 	 */
-	if (likely(hlock_class(this)->usage_mask & new_mask))
+	if (likely(!(new_mask & ~hlock_class(this)->usage_mask)))
 		return 1;
 
 	if (!graph_lock())
@@ -3165,7 +3165,7 @@ static int mark_lock(struct task_struct *curr, struct held_lock *this,
 	/*
 	 * Make sure we didn't race:
 	 */
-	if (unlikely(hlock_class(this)->usage_mask & new_mask)) {
+	if (unlikely(!(new_mask & ~hlock_class(this)->usage_mask))) {
 		graph_unlock();
 		return 1;
 	}
-- 
2.17.1

[PATCH 09/32] locking/lockdep: Save stack trace for each softirq vector involved

[Frederic Weisbecker]

We are going to save as much traces as we have softirq vectors involved
in a given usage. Expand the stack trace record code accordingly.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 include/linux/lockdep.h  |  3 ++-
 kernel/locking/lockdep.c | 18 +++++++++++++++++-
 2 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 06669f20a30a..69d2dac3d821 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -31,8 +31,9 @@ extern int lock_stat;
 /*
  * We'd rather not expose kernel/lockdep_states.h this wide, but we do need
  * the total number of states... :-(
+ * 1 bit for LOCK_USED, 4 bits for hardirqs and 4 * NR_SOFTIRQS bits
  */
-#define XXX_LOCK_USAGE_STATES		(1+2*4)
+#define XXX_LOCK_USAGE_STATES		(1 + (1 + 10) * 4)
 
 /*
  * NR_LOCKDEP_CACHING_CLASSES ... Number of classes
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 9194f11d3dfb..4bac8c1a3929 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -3145,6 +3145,22 @@ static inline int separate_irq_context(struct task_struct *curr,
 
 #endif /* defined(CONFIG_TRACE_IRQFLAGS) && defined(CONFIG_PROVE_LOCKING) */
 
+static int save_trace_mask(struct lock_class *class, u64 mask)
+{
+	int bit = 0;
+
+	while (mask) {
+		long fs = __ffs64(mask) + 1;
+
+		mask >>= fs;
+		bit += fs;
+		if (!save_trace(class->usage_traces + bit - 1))
+			return -1;
+	}
+
+	return 0;
+}
+
 /*
  * Mark a lock with a usage bit, and validate the state transition:
  */
@@ -3172,7 +3188,7 @@ static int mark_lock(struct task_struct *curr, struct held_lock *this,
 
 	hlock_class(this)->usage_mask |= new_mask;
 
-	if (!save_trace(hlock_class(this)->usage_traces + new_usage->bit))
+	if (save_trace_mask(hlock_class(this), new_mask) < 0)
 		return 0;
 
 	switch (new_usage->bit) {
-- 
2.17.1

Re: [PATCH 09/32] locking/lockdep: Save stack trace for each softirq vector involved

[Linus Torvalds]

On Tue, Feb 12, 2019 at 9:15 AM Frederic Weisbecker <frederic@kernel.org> wrote:
>
>
> +static int save_trace_mask(struct lock_class *class, u64 mask)
> +{
> +       int bit = 0;
> +
> +       while (mask) {
> +               long fs = __ffs64(mask) + 1;
> +
> +               mask >>= fs;
> +               bit += fs;

Same buggy pattern of "ffs+1" and overflow of shift count.

                 Linus

Re: [PATCH 09/32] locking/lockdep: Save stack trace for each softirq vector involved

[Frederic Weisbecker]

On Tue, Feb 12, 2019 at 09:47:39AM -0800, Linus Torvalds wrote:
> On Tue, Feb 12, 2019 at 9:15 AM Frederic Weisbecker <frederic@kernel.org> wrote:
> >
> >
> > +static int save_trace_mask(struct lock_class *class, u64 mask)
> > +{
> > +       int bit = 0;
> > +
> > +       while (mask) {
> > +               long fs = __ffs64(mask) + 1;
> > +
> > +               mask >>= fs;
> > +               bit += fs;
> 
> Same buggy pattern of "ffs+1" and overflow of shift count.

You're right and there are more. I'll unify all that around
a safe iterator.

Thanks.

[PATCH 10/32] locking/lockdep: Make mark_lock() verbosity aware of vector

[Frederic Weisbecker]

Expand the effective usage bit on top of the lock usage bit/vector pair
while in verbose mode logging in mark_lock().

FIXME: This only handle the first bit in the mask. We may need to
iterate over all of them.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/locking/lockdep.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 4bac8c1a3929..6a74b433fe4c 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -3219,7 +3219,7 @@ static int mark_lock(struct task_struct *curr, struct held_lock *this,
 	 * We must printk outside of the graph_lock:
 	 */
 	if (ret == 2) {
-		printk("\nmarked lock as {%s}:\n", usage_str[new_usage->bit]);
+		printk("\nmarked lock as {%s}:\n", usage_str[__ffs64(lock_usage_mask(new_usage))]);
 		print_lock(this);
 		print_irqtrace_events(curr);
 		dump_stack();
-- 
2.17.1

[PATCH 11/32] softirq: Macrofy softirq vectors

[Frederic Weisbecker]

Define the softirq vectors through macros so that we can later include
them in the automated definition of lockdep usage states.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 include/linux/interrupt.h      | 17 ++++-------------
 include/linux/softirq_vector.h | 10 ++++++++++
 2 files changed, 14 insertions(+), 13 deletions(-)
 create mode 100644 include/linux/softirq_vector.h

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index c672f34235e7..e871f361f1f1 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -474,21 +474,12 @@ extern bool force_irqthreads;
    tasklets are more than enough. F.e. all serial device BHs et
    al. should be converted to tasklets, not to softirqs.
  */
-
 enum
 {
-	HI_SOFTIRQ=0,
-	TIMER_SOFTIRQ,
-	NET_TX_SOFTIRQ,
-	NET_RX_SOFTIRQ,
-	BLOCK_SOFTIRQ,
-	IRQ_POLL_SOFTIRQ,
-	TASKLET_SOFTIRQ,
-	SCHED_SOFTIRQ,
-	HRTIMER_SOFTIRQ, /* Unused, but kept as tools rely on the
-			    numbering. Sigh! */
-	RCU_SOFTIRQ,    /* Preferable RCU should always be the last softirq */
-
+#define SOFTIRQ_VECTOR(__SVEC) \
+	__SVEC##_SOFTIRQ,
+#include <linux/softirq_vector.h>
+#undef SOFTIRQ_VECTOR
 	NR_SOFTIRQS
 };
 
diff --git a/include/linux/softirq_vector.h b/include/linux/softirq_vector.h
new file mode 100644
index 000000000000..949720c866fd
--- /dev/null
+++ b/include/linux/softirq_vector.h
@@ -0,0 +1,10 @@
+SOFTIRQ_VECTOR(HI)
+SOFTIRQ_VECTOR(TIMER)
+SOFTIRQ_VECTOR(NET_TX)
+SOFTIRQ_VECTOR(NET_RX)
+SOFTIRQ_VECTOR(BLOCK)
+SOFTIRQ_VECTOR(IRQ_POLL)
+SOFTIRQ_VECTOR(TASKLET)
+SOFTIRQ_VECTOR(SCHED)
+SOFTIRQ_VECTOR(HRTIMER)
+SOFTIRQ_VECTOR(RCU)    /* Preferable RCU should always be the last softirq */
-- 
2.17.1

Re: [PATCH 11/32] softirq: Macrofy softirq vectors

[Sebastian Andrzej Siewior]

On 2019-02-12 18:14:02 [+0100], Frederic Weisbecker wrote:
> --- /dev/null
> +++ b/include/linux/softirq_vector.h
> @@ -0,0 +1,10 @@
could you please add a spdx header/identifier here?

> +SOFTIRQ_VECTOR(HI)
> +SOFTIRQ_VECTOR(TIMER)
> +SOFTIRQ_VECTOR(NET_TX)
> +SOFTIRQ_VECTOR(NET_RX)
> +SOFTIRQ_VECTOR(BLOCK)
> +SOFTIRQ_VECTOR(IRQ_POLL)
> +SOFTIRQ_VECTOR(TASKLET)
> +SOFTIRQ_VECTOR(SCHED)
> +SOFTIRQ_VECTOR(HRTIMER)
> +SOFTIRQ_VECTOR(RCU)    /* Preferable RCU should always be the last softirq */

Sebastian

Re: [PATCH 11/32] softirq: Macrofy softirq vectors

[Frederic Weisbecker]

On Wed, Feb 27, 2019 at 10:54:10AM +0100, Sebastian Andrzej Siewior wrote:
> On 2019-02-12 18:14:02 [+0100], Frederic Weisbecker wrote:
> > --- /dev/null
> > +++ b/include/linux/softirq_vector.h
> > @@ -0,0 +1,10 @@
> could you please add a spdx header/identifier here?

Ah right, will do.

Thanks.

[PATCH 12/32] locking/lockdep: Define per vector softirq lock usage states

[Frederic Weisbecker]

Now that the code is mostly ready to play at this finegrained level,
reuse the macrofied vector list to define the usual 4 lock usage states
for each softirq vectors. This way we can perform the validations
independently for each of them.

The new usage mask layout becomes:

                              TIMER_SOFTIRQ
            LOCK_USED             bits       HARDIRQ bits
                \                   |            |
                 \                  |            |
                  0   0000  [...]  0000  0000  0000
                        |                  |
                        |                  |
                   RCU_SOFTIRQ        HI_SOFTIRQ bits
                      bits

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/locking/lockdep_internals.h | 43 +++++++++++++++++++++++++++---
 kernel/locking/lockdep_states.h    |  4 ++-
 2 files changed, 42 insertions(+), 5 deletions(-)

diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h
index e714c823f594..4b0c03f0f7ce 100644
--- a/kernel/locking/lockdep_internals.h
+++ b/kernel/locking/lockdep_internals.h
@@ -22,9 +22,15 @@ enum lock_usage_bit {
 	LOCK_USAGE_STATES
 };
 
-#define LOCK_USAGE_READ_MASK 1
-#define LOCK_USAGE_DIR_MASK  2
-#define LOCK_USAGE_STATE_MASK (~(LOCK_USAGE_READ_MASK | LOCK_USAGE_DIR_MASK))
+#define LOCK_USAGE_READ_MASK		1
+#define LOCK_USAGE_DIR_MASK 		2
+#define LOCK_USAGE_STATE_MASK		(~(LOCK_USAGE_READ_MASK | LOCK_USAGE_DIR_MASK))
+
+/* Base softirq bits that get shifted by the index in stuct lock_usage::vector */
+#define LOCK_USED_IN_SOFTIRQ		LOCK_USED_IN_HI_SOFTIRQ
+#define LOCK_USED_IN_SOFTIRQ_READ	LOCK_USED_IN_HI_SOFTIRQ_READ
+#define LOCK_ENABLED_SOFTIRQ		LOCK_ENABLED_HI_SOFTIRQ
+#define LOCK_ENABLED_SOFTIRQ_READ	LOCK_ENABLED_HI_SOFTIRQ_READ
 
 struct lock_usage {
 	enum lock_usage_bit bit;
@@ -35,7 +41,7 @@ struct lock_usage {
 /*
  * Usage-state bitmasks:
  */
-#define __LOCKF(__STATE)	LOCKF_##__STATE = (1 << LOCK_##__STATE),
+#define __LOCKF(__STATE)	LOCKF_##__STATE = (1ULL << LOCK_##__STATE),
 
 enum {
 #define LOCKDEP_STATE(__STATE)						\
@@ -48,11 +54,40 @@ enum {
 	__LOCKF(USED)
 };
 
+#define LOCKF_ENABLED_SOFTIRQ \
+	(LOCKF_ENABLED_HI_SOFTIRQ | LOCKF_ENABLED_TIMER_SOFTIRQ | \
+	LOCKF_ENABLED_NET_TX_SOFTIRQ | LOCKF_ENABLED_NET_RX_SOFTIRQ | \
+	LOCKF_ENABLED_BLOCK_SOFTIRQ | LOCKF_ENABLED_IRQ_POLL_SOFTIRQ | \
+	LOCKF_ENABLED_TASKLET_SOFTIRQ | LOCKF_ENABLED_SCHED_SOFTIRQ | \
+	 LOCKF_ENABLED_HRTIMER_SOFTIRQ | LOCKF_ENABLED_RCU_SOFTIRQ)
 #define LOCKF_ENABLED_IRQ (LOCKF_ENABLED_HARDIRQ | LOCKF_ENABLED_SOFTIRQ)
+
+
+#define LOCKF_USED_IN_SOFTIRQ \
+	(LOCKF_USED_IN_HI_SOFTIRQ | LOCKF_USED_IN_TIMER_SOFTIRQ | \
+	LOCKF_USED_IN_NET_TX_SOFTIRQ | LOCKF_USED_IN_NET_RX_SOFTIRQ | \
+	LOCKF_USED_IN_BLOCK_SOFTIRQ | LOCKF_USED_IN_IRQ_POLL_SOFTIRQ | \
+	LOCKF_USED_IN_TASKLET_SOFTIRQ | LOCKF_USED_IN_SCHED_SOFTIRQ | \
+	 LOCKF_USED_IN_HRTIMER_SOFTIRQ | LOCKF_USED_IN_RCU_SOFTIRQ)
 #define LOCKF_USED_IN_IRQ (LOCKF_USED_IN_HARDIRQ | LOCKF_USED_IN_SOFTIRQ)
 
+
+#define LOCKF_ENABLED_SOFTIRQ_READ \
+	(LOCKF_ENABLED_HI_SOFTIRQ_READ | LOCKF_ENABLED_TIMER_SOFTIRQ_READ | \
+	LOCKF_ENABLED_NET_TX_SOFTIRQ_READ | LOCKF_ENABLED_NET_RX_SOFTIRQ_READ | \
+	LOCKF_ENABLED_BLOCK_SOFTIRQ_READ | LOCKF_ENABLED_IRQ_POLL_SOFTIRQ_READ | \
+	LOCKF_ENABLED_TASKLET_SOFTIRQ_READ | LOCKF_ENABLED_SCHED_SOFTIRQ_READ | \
+	 LOCKF_ENABLED_HRTIMER_SOFTIRQ_READ | LOCKF_ENABLED_RCU_SOFTIRQ_READ)
 #define LOCKF_ENABLED_IRQ_READ \
 		(LOCKF_ENABLED_HARDIRQ_READ | LOCKF_ENABLED_SOFTIRQ_READ)
+
+
+#define LOCKF_USED_IN_SOFTIRQ_READ \
+	(LOCKF_USED_IN_HI_SOFTIRQ_READ | LOCKF_USED_IN_TIMER_SOFTIRQ_READ | \
+	LOCKF_USED_IN_NET_TX_SOFTIRQ_READ | LOCKF_USED_IN_NET_RX_SOFTIRQ_READ | \
+	LOCKF_USED_IN_BLOCK_SOFTIRQ_READ | LOCKF_USED_IN_IRQ_POLL_SOFTIRQ_READ | \
+	LOCKF_USED_IN_TASKLET_SOFTIRQ_READ | LOCKF_USED_IN_SCHED_SOFTIRQ_READ | \
+	 LOCKF_USED_IN_HRTIMER_SOFTIRQ_READ | LOCKF_USED_IN_RCU_SOFTIRQ_READ)
 #define LOCKF_USED_IN_IRQ_READ \
 		(LOCKF_USED_IN_HARDIRQ_READ | LOCKF_USED_IN_SOFTIRQ_READ)
 
diff --git a/kernel/locking/lockdep_states.h b/kernel/locking/lockdep_states.h
index 35ca09f2ed0b..34ff569d2024 100644
--- a/kernel/locking/lockdep_states.h
+++ b/kernel/locking/lockdep_states.h
@@ -5,4 +5,6 @@
  * you add one, or come up with a nice dynamic solution.
  */
 LOCKDEP_STATE(HARDIRQ)
-LOCKDEP_STATE(SOFTIRQ)
+#define SOFTIRQ_VECTOR(__SVEC) LOCKDEP_STATE(__SVEC##_SOFTIRQ)
+#include <linux/softirq_vector.h>
+#undef SOFTIRQ_VECTOR
-- 
2.17.1

[PATCH 13/32] softirq: Pass softirq vector number to lockdep on vector execution

[Frederic Weisbecker]

Pass the softirq vector number to lockdep on callback execution so that
we know which one is involved while holding a lock. We will then be able
to pick up the proper LOCK_USED_IN_*_SOFTIRQ index to perform the
finegrained verifications.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 include/linux/irqflags.h | 12 ++++++------
 kernel/softirq.c         |  8 ++++----
 lib/locking-selftest.c   |  4 ++--
 3 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/include/linux/irqflags.h b/include/linux/irqflags.h
index 21619c92c377..623030a74866 100644
--- a/include/linux/irqflags.h
+++ b/include/linux/irqflags.h
@@ -43,13 +43,13 @@ do {						\
 do {						\
 	current->hardirq_context--;		\
 } while (0)
-# define lockdep_softirq_enter()		\
+# define lockdep_softirq_enter(__VEC)		\
 do {						\
-	current->softirq_context++;		\
+	current->softirq_context |= BIT(__VEC);	\
 } while (0)
-# define lockdep_softirq_exit()			\
+# define lockdep_softirq_exit(__VEC)		\
 do {						\
-	current->softirq_context--;		\
+	current->softirq_context &= ~BIT(__VEC);\
 } while (0)
 #else
 # define trace_hardirqs_on()		do { } while (0)
@@ -60,8 +60,8 @@ do {						\
 # define trace_softirqs_enabled(p)	0
 # define trace_hardirq_enter()		do { } while (0)
 # define trace_hardirq_exit()		do { } while (0)
-# define lockdep_softirq_enter()	do { } while (0)
-# define lockdep_softirq_exit()		do { } while (0)
+# define lockdep_softirq_enter(vec)	do { } while (0)
+# define lockdep_softirq_exit(vec)	do { } while (0)
 #endif
 
 #if defined(CONFIG_IRQSOFF_TRACER) || \
diff --git a/kernel/softirq.c b/kernel/softirq.c
index d28813306b2c..2dcaef813acb 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -229,18 +229,15 @@ static inline bool lockdep_softirq_start(void)
 		trace_hardirq_exit();
 	}
 
-	lockdep_softirq_enter();
-
 	return in_hardirq;
 }
 
 static inline void lockdep_softirq_end(bool in_hardirq)
 {
-	lockdep_softirq_exit();
-
 	if (in_hardirq)
 		trace_hardirq_enter();
 }
+
 #else
 static inline bool lockdep_softirq_start(void) { return false; }
 static inline void lockdep_softirq_end(bool in_hardirq) { }
@@ -288,9 +285,12 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)
 
 		kstat_incr_softirqs_this_cpu(vec_nr);
 
+		lockdep_softirq_enter(vec_nr);
 		trace_softirq_entry(vec_nr);
 		h->action(h);
 		trace_softirq_exit(vec_nr);
+		lockdep_softirq_exit(vec_nr);
+
 		if (unlikely(prev_count != preempt_count())) {
 			pr_err("huh, entered softirq %u %s %p with preempt_count %08x, exited with %08x?\n",
 			       vec_nr, softirq_to_name[vec_nr], h->action,
diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c
index 1e1bbf171eca..2bd782d7f2e5 100644
--- a/lib/locking-selftest.c
+++ b/lib/locking-selftest.c
@@ -197,11 +197,11 @@ static void init_shared_classes(void)
 #define SOFTIRQ_ENTER()				\
 		local_bh_disable();		\
 		local_irq_disable();		\
-		lockdep_softirq_enter();	\
+		lockdep_softirq_enter(0);	\
 		WARN_ON(!in_softirq());
 
 #define SOFTIRQ_EXIT()				\
-		lockdep_softirq_exit();		\
+		lockdep_softirq_exit(0);	\
 		local_irq_enable();		\
 		local_bh_enable();
 
-- 
2.17.1

[PATCH 14/32] x86: Revert "x86/irq: Demote irq_cpustat_t::__softirq_pending to u16"

[Frederic Weisbecker]

This reverts commit 9aee5f8a7e30330d0a8f4c626dc924ca5590aba5.

We are going to need the 16 high bits above in order to implement
a softirq enable mask. x86 is the only architecture that doesn't use
unsigned int to implement softirq_pending.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 arch/x86/include/asm/hardirq.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h
index d9069bb26c7f..a8e8e126b421 100644
--- a/arch/x86/include/asm/hardirq.h
+++ b/arch/x86/include/asm/hardirq.h
@@ -5,7 +5,7 @@
 #include <linux/threads.h>
 
 typedef struct {
-	u16	     __softirq_pending;
+	unsigned int __softirq_pending;
 #if IS_ENABLED(CONFIG_KVM_INTEL)
 	u8	     kvm_cpu_l1tf_flush_l1d;
 #endif
-- 
2.17.1

[PATCH 15/32] arch/softirq: Rename softirq_pending fields to softirq_data

[Frederic Weisbecker]

We are going to extend the softirq bits with an enabled vector mask.
Provide the field with a more generic name to later layout the pending
states on the lower bits and the enabled states on the higher bits.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 arch/arm/include/asm/hardirq.h      |  2 +-
 arch/arm64/include/asm/hardirq.h    |  2 +-
 arch/h8300/kernel/asm-offsets.c     |  2 +-
 arch/ia64/include/asm/hardirq.h     |  2 +-
 arch/ia64/include/asm/processor.h   |  2 +-
 arch/m68k/include/asm/hardirq.h     |  2 +-
 arch/m68k/kernel/asm-offsets.c      |  2 +-
 arch/parisc/include/asm/hardirq.h   |  2 +-
 arch/powerpc/include/asm/hardirq.h  |  2 +-
 arch/s390/include/asm/hardirq.h     |  6 +++---
 arch/sh/include/asm/hardirq.h       |  2 +-
 arch/sparc/include/asm/cpudata_64.h |  2 +-
 arch/sparc/include/asm/hardirq_64.h |  4 ++--
 arch/um/include/asm/hardirq.h       |  2 +-
 arch/x86/include/asm/hardirq.h      |  2 +-
 include/asm-generic/hardirq.h       |  2 +-
 include/linux/interrupt.h           | 10 +++++-----
 17 files changed, 24 insertions(+), 24 deletions(-)

diff --git a/arch/arm/include/asm/hardirq.h b/arch/arm/include/asm/hardirq.h
index cba23eaa6072..e5b06dd41b88 100644
--- a/arch/arm/include/asm/hardirq.h
+++ b/arch/arm/include/asm/hardirq.h
@@ -9,7 +9,7 @@
 #define NR_IPI	7
 
 typedef struct {
-	unsigned int __softirq_pending;
+	unsigned int __softirq_data;
 #ifdef CONFIG_SMP
 	unsigned int ipi_irqs[NR_IPI];
 #endif
diff --git a/arch/arm64/include/asm/hardirq.h b/arch/arm64/include/asm/hardirq.h
index 1473fc2f7ab7..e9add887e2f2 100644
--- a/arch/arm64/include/asm/hardirq.h
+++ b/arch/arm64/include/asm/hardirq.h
@@ -23,7 +23,7 @@
 #define NR_IPI	7
 
 typedef struct {
-	unsigned int __softirq_pending;
+	unsigned int __softirq_data;
 	unsigned int ipi_irqs[NR_IPI];
 } ____cacheline_aligned irq_cpustat_t;
 
diff --git a/arch/h8300/kernel/asm-offsets.c b/arch/h8300/kernel/asm-offsets.c
index 85e60509f0a8..719d4cff704e 100644
--- a/arch/h8300/kernel/asm-offsets.c
+++ b/arch/h8300/kernel/asm-offsets.c
@@ -32,7 +32,7 @@ int main(void)
 
 	/* offsets into the irq_cpustat_t struct */
 	DEFINE(CPUSTAT_SOFTIRQ_PENDING, offsetof(irq_cpustat_t,
-						 __softirq_pending));
+						 __softirq_data));
 
 	/* offsets into the thread struct */
 	OFFSET(THREAD_KSP, thread_struct, ksp);
diff --git a/arch/ia64/include/asm/hardirq.h b/arch/ia64/include/asm/hardirq.h
index ccde7c2ba00f..004f6093a11a 100644
--- a/arch/ia64/include/asm/hardirq.h
+++ b/arch/ia64/include/asm/hardirq.h
@@ -13,7 +13,7 @@
 
 #define __ARCH_IRQ_STAT	1
 
-#define local_softirq_pending_ref	ia64_cpu_info.softirq_pending
+#define local_softirq_data_ref	ia64_cpu_info.softirq_data
 
 #include <linux/threads.h>
 #include <linux/irq.h>
diff --git a/arch/ia64/include/asm/processor.h b/arch/ia64/include/asm/processor.h
index c91ef98ed6bf..72a70c42b701 100644
--- a/arch/ia64/include/asm/processor.h
+++ b/arch/ia64/include/asm/processor.h
@@ -188,7 +188,7 @@ union  ia64_rr {
  * state comes earlier:
  */
 struct cpuinfo_ia64 {
-	unsigned int softirq_pending;
+	unsigned int softirq_data;
 	unsigned long itm_delta;	/* # of clock cycles between clock ticks */
 	unsigned long itm_next;		/* interval timer mask value to use for next clock tick */
 	unsigned long nsec_per_cyc;	/* (1000000000<<IA64_NSEC_PER_CYC_SHIFT)/itc_freq */
diff --git a/arch/m68k/include/asm/hardirq.h b/arch/m68k/include/asm/hardirq.h
index 11793165445d..6ad364cc3799 100644
--- a/arch/m68k/include/asm/hardirq.h
+++ b/arch/m68k/include/asm/hardirq.h
@@ -15,7 +15,7 @@ static inline void ack_bad_irq(unsigned int irq)
 
 /* entry.S is sensitive to the offsets of these fields */
 typedef struct {
-	unsigned int __softirq_pending;
+	unsigned int __softirq_data;
 } ____cacheline_aligned irq_cpustat_t;
 
 #include <linux/irq_cpustat.h>	/* Standard mappings for irq_cpustat_t above */
diff --git a/arch/m68k/kernel/asm-offsets.c b/arch/m68k/kernel/asm-offsets.c
index ccea355052ef..93b6bea52c8b 100644
--- a/arch/m68k/kernel/asm-offsets.c
+++ b/arch/m68k/kernel/asm-offsets.c
@@ -64,7 +64,7 @@ int main(void)
 #endif
 
 	/* offsets into the irq_cpustat_t struct */
-	DEFINE(CPUSTAT_SOFTIRQ_PENDING, offsetof(irq_cpustat_t, __softirq_pending));
+	DEFINE(CPUSTAT_SOFTIRQ_PENDING, offsetof(irq_cpustat_t, __softirq_data));
 
 	/* signal defines */
 	DEFINE(LSIGSEGV, SIGSEGV);
diff --git a/arch/parisc/include/asm/hardirq.h b/arch/parisc/include/asm/hardirq.h
index 1a1235a9d533..28d8ceeab077 100644
--- a/arch/parisc/include/asm/hardirq.h
+++ b/arch/parisc/include/asm/hardirq.h
@@ -17,7 +17,7 @@
 #endif
 
 typedef struct {
-	unsigned int __softirq_pending;
+	unsigned int __softirq_data;
 	unsigned int kernel_stack_usage;
 	unsigned int irq_stack_usage;
 #ifdef CONFIG_SMP
diff --git a/arch/powerpc/include/asm/hardirq.h b/arch/powerpc/include/asm/hardirq.h
index f1e9067bd5ac..d3a896b402da 100644
--- a/arch/powerpc/include/asm/hardirq.h
+++ b/arch/powerpc/include/asm/hardirq.h
@@ -6,7 +6,7 @@
 #include <linux/irq.h>
 
 typedef struct {
-	unsigned int __softirq_pending;
+	unsigned int __softirq_data;
 	unsigned int timer_irqs_event;
 	unsigned int broadcast_irqs_event;
 	unsigned int timer_irqs_others;
diff --git a/arch/s390/include/asm/hardirq.h b/arch/s390/include/asm/hardirq.h
index dfbc3c6c0674..e26325fe287d 100644
--- a/arch/s390/include/asm/hardirq.h
+++ b/arch/s390/include/asm/hardirq.h
@@ -13,9 +13,9 @@
 
 #include <asm/lowcore.h>
 
-#define local_softirq_pending() (S390_lowcore.softirq_pending)
-#define set_softirq_pending(x) (S390_lowcore.softirq_pending = (x))
-#define or_softirq_pending(x)  (S390_lowcore.softirq_pending |= (x))
+#define local_softirq_pending() (S390_lowcore.softirq_data)
+#define set_softirq_pending(x) (S390_lowcore.softirq_data = (x))
+#define or_softirq_pending(x)  (S390_lowcore.softirq_data |= (x))
 
 #define __ARCH_IRQ_STAT
 #define __ARCH_HAS_DO_SOFTIRQ
diff --git a/arch/sh/include/asm/hardirq.h b/arch/sh/include/asm/hardirq.h
index edaea3559a23..e364a63babd3 100644
--- a/arch/sh/include/asm/hardirq.h
+++ b/arch/sh/include/asm/hardirq.h
@@ -6,7 +6,7 @@
 #include <linux/irq.h>
 
 typedef struct {
-	unsigned int __softirq_pending;
+	unsigned int __softirq_data;
 	unsigned int __nmi_count;		/* arch dependent */
 } ____cacheline_aligned irq_cpustat_t;
 
diff --git a/arch/sparc/include/asm/cpudata_64.h b/arch/sparc/include/asm/cpudata_64.h
index 9c3fc03abe9a..9e52c6bf69b9 100644
--- a/arch/sparc/include/asm/cpudata_64.h
+++ b/arch/sparc/include/asm/cpudata_64.h
@@ -11,7 +11,7 @@
 
 typedef struct {
 	/* Dcache line 1 */
-	unsigned int	__softirq_pending; /* must be 1st, see rtrap.S */
+	unsigned int	__softirq_data; /* must be 1st, see rtrap.S */
 	unsigned int	__nmi_count;
 	unsigned long	clock_tick;	/* %tick's per second */
 	unsigned long	__pad;
diff --git a/arch/sparc/include/asm/hardirq_64.h b/arch/sparc/include/asm/hardirq_64.h
index 75b92bfe04b5..8ff0458870e1 100644
--- a/arch/sparc/include/asm/hardirq_64.h
+++ b/arch/sparc/include/asm/hardirq_64.h
@@ -11,8 +11,8 @@
 
 #define __ARCH_IRQ_STAT
 
-#define local_softirq_pending_ref \
-	__cpu_data.__softirq_pending
+#define local_softirq_data_ref \
+	__cpu_data.__softirq_data
 
 void ack_bad_irq(unsigned int irq);
 
diff --git a/arch/um/include/asm/hardirq.h b/arch/um/include/asm/hardirq.h
index b426796d26fd..96844938b92e 100644
--- a/arch/um/include/asm/hardirq.h
+++ b/arch/um/include/asm/hardirq.h
@@ -6,7 +6,7 @@
 #include <linux/threads.h>
 
 typedef struct {
-	unsigned int __softirq_pending;
+	unsigned int __softirq_data;
 } ____cacheline_aligned irq_cpustat_t;
 
 #include <linux/irq_cpustat.h>	/* Standard mappings for irq_cpustat_t above */
diff --git a/arch/x86/include/asm/hardirq.h b/arch/x86/include/asm/hardirq.h
index a8e8e126b421..875f7de3b65b 100644
--- a/arch/x86/include/asm/hardirq.h
+++ b/arch/x86/include/asm/hardirq.h
@@ -5,7 +5,7 @@
 #include <linux/threads.h>
 
 typedef struct {
-	unsigned int __softirq_pending;
+	unsigned int __softirq_data;
 #if IS_ENABLED(CONFIG_KVM_INTEL)
 	u8	     kvm_cpu_l1tf_flush_l1d;
 #endif
diff --git a/include/asm-generic/hardirq.h b/include/asm-generic/hardirq.h
index d14214dfc10b..4ea87b50a257 100644
--- a/include/asm-generic/hardirq.h
+++ b/include/asm-generic/hardirq.h
@@ -6,7 +6,7 @@
 #include <linux/threads.h>
 
 typedef struct {
-	unsigned int __softirq_pending;
+	unsigned int __softirq_data;
 } ____cacheline_aligned irq_cpustat_t;
 
 #include <linux/irq_cpustat.h>	/* Standard mappings for irq_cpustat_t above */
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index e871f361f1f1..a32bdd9bb103 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -449,13 +449,13 @@ extern bool force_irqthreads;
 
 #ifndef local_softirq_pending
 
-#ifndef local_softirq_pending_ref
-#define local_softirq_pending_ref irq_stat.__softirq_pending
+#ifndef local_softirq_data_ref
+#define local_softirq_data_ref irq_stat.__softirq_data
 #endif
 
-#define local_softirq_pending()	(__this_cpu_read(local_softirq_pending_ref))
-#define set_softirq_pending(x)	(__this_cpu_write(local_softirq_pending_ref, (x)))
-#define or_softirq_pending(x)	(__this_cpu_or(local_softirq_pending_ref, (x)))
+#define local_softirq_pending()	(__this_cpu_read(local_softirq_data_ref))
+#define set_softirq_pending(x)	(__this_cpu_write(local_softirq_data_ref, (x)))
+#define or_softirq_pending(x)	(__this_cpu_or(local_softirq_data_ref, (x)))
 
 #endif /* local_softirq_pending */
 
-- 
2.17.1

[PATCH 16/32] softirq: Normalize softirq_pending naming scheme

[Frederic Weisbecker]

From: Frederic Weisbecker <fweisbec@gmail.com>

Use the subsystem as the prefix to name the __softirq_data accessors.
They are going to be extended and want a more greppable and standard
naming sheme.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 arch/s390/include/asm/hardirq.h |  4 ++--
 include/linux/interrupt.h       | 24 ++++++++++++------------
 kernel/softirq.c                |  4 ++--
 3 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/arch/s390/include/asm/hardirq.h b/arch/s390/include/asm/hardirq.h
index e26325fe287d..3103680487cf 100644
--- a/arch/s390/include/asm/hardirq.h
+++ b/arch/s390/include/asm/hardirq.h
@@ -14,8 +14,8 @@
 #include <asm/lowcore.h>
 
 #define local_softirq_pending() (S390_lowcore.softirq_data)
-#define set_softirq_pending(x) (S390_lowcore.softirq_data = (x))
-#define or_softirq_pending(x)  (S390_lowcore.softirq_data |= (x))
+#define softirq_pending_set(x) (S390_lowcore.softirq_data = (x))
+#define softirq_pending_or(x)  (S390_lowcore.softirq_data |= (x))
 
 #define __ARCH_IRQ_STAT
 #define __ARCH_HAS_DO_SOFTIRQ
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index a32bdd9bb103..a5dec926531b 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -447,18 +447,6 @@ extern bool force_irqthreads;
 #define force_irqthreads	(0)
 #endif
 
-#ifndef local_softirq_pending
-
-#ifndef local_softirq_data_ref
-#define local_softirq_data_ref irq_stat.__softirq_data
-#endif
-
-#define local_softirq_pending()	(__this_cpu_read(local_softirq_data_ref))
-#define set_softirq_pending(x)	(__this_cpu_write(local_softirq_data_ref, (x)))
-#define or_softirq_pending(x)	(__this_cpu_or(local_softirq_data_ref, (x)))
-
-#endif /* local_softirq_pending */
-
 /* Some architectures might implement lazy enabling/disabling of
  * interrupts. In some cases, such as stop_machine, we might want
  * to ensure that after a local_irq_disable(), interrupts have
@@ -485,6 +473,18 @@ enum
 
 #define SOFTIRQ_STOP_IDLE_MASK (~(1 << RCU_SOFTIRQ))
 
+#ifndef local_softirq_pending
+
+#ifndef local_softirq_data_ref
+#define local_softirq_data_ref irq_stat.__softirq_data
+#endif
+
+#define local_softirq_pending()	(__this_cpu_read(local_softirq_data_ref))
+#define softirq_pending_set(x)	(__this_cpu_write(local_softirq_data_ref, (x)))
+#define softirq_pending_or(x)	(__this_cpu_or(local_softirq_data_ref, (x)))
+
+#endif /* local_softirq_pending */
+
 /* map softirq index to softirq name. update 'softirq_to_name' in
  * kernel/softirq.c when adding a new softirq.
  */
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 2dcaef813acb..2137c54baf98 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -268,7 +268,7 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)
 
 restart:
 	/* Reset the pending bitmask before enabling irqs */
-	set_softirq_pending(0);
+	softirq_pending_set(0);
 
 	local_irq_enable();
 
@@ -449,7 +449,7 @@ void raise_softirq(unsigned int nr)
 void __raise_softirq_irqoff(unsigned int nr)
 {
 	trace_softirq_raise(nr);
-	or_softirq_pending(1UL << nr);
+	softirq_pending_or(1UL << nr);
 }
 
 void open_softirq(int nr, void (*action)(struct softirq_action *))
-- 
2.17.1

[PATCH 17/32] softirq: Convert softirq_pending_*() to set/clear mask scheme

[Frederic Weisbecker]

We are going to need a nand version of softirq_pending_or() in order
to clear specific bits from the pending mask.

But instead of naming the mutators after their implementation, rather
name them after their high level purpose.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 arch/s390/include/asm/hardirq.h | 4 ++--
 include/linux/interrupt.h       | 5 +++--
 kernel/softirq.c                | 4 ++--
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/s390/include/asm/hardirq.h b/arch/s390/include/asm/hardirq.h
index 3103680487cf..54e81f520175 100644
--- a/arch/s390/include/asm/hardirq.h
+++ b/arch/s390/include/asm/hardirq.h
@@ -14,8 +14,8 @@
 #include <asm/lowcore.h>
 
 #define local_softirq_pending() (S390_lowcore.softirq_data)
-#define softirq_pending_set(x) (S390_lowcore.softirq_data = (x))
-#define softirq_pending_or(x)  (S390_lowcore.softirq_data |= (x))
+#define softirq_pending_clear_mask(x) (S390_lowcore.softirq_data &= ~(x))
+#define softirq_pending_set_mask(x)  (S390_lowcore.softirq_data |= (x))
 
 #define __ARCH_IRQ_STAT
 #define __ARCH_HAS_DO_SOFTIRQ
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index a5dec926531b..c402770ae45b 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -472,6 +472,7 @@ enum
 };
 
 #define SOFTIRQ_STOP_IDLE_MASK (~(1 << RCU_SOFTIRQ))
+#define SOFTIRQ_ALL_MASK (BIT(NR_SOFTIRQS) - 1)
 
 #ifndef local_softirq_pending
 
@@ -480,8 +481,8 @@ enum
 #endif
 
 #define local_softirq_pending()	(__this_cpu_read(local_softirq_data_ref))
-#define softirq_pending_set(x)	(__this_cpu_write(local_softirq_data_ref, (x)))
-#define softirq_pending_or(x)	(__this_cpu_or(local_softirq_data_ref, (x)))
+#define softirq_pending_clear_mask(x)	(__this_cpu_and(local_softirq_data_ref, ~(x)))
+#define softirq_pending_set_mask(x)	(__this_cpu_or(local_softirq_data_ref, (x)))
 
 #endif /* local_softirq_pending */
 
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 2137c54baf98..5f167fc43ab9 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -268,7 +268,7 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)
 
 restart:
 	/* Reset the pending bitmask before enabling irqs */
-	softirq_pending_set(0);
+	softirq_pending_clear_mask(SOFTIRQ_ALL_MASK);
 
 	local_irq_enable();
 
@@ -449,7 +449,7 @@ void raise_softirq(unsigned int nr)
 void __raise_softirq_irqoff(unsigned int nr)
 {
 	trace_softirq_raise(nr);
-	softirq_pending_or(1UL << nr);
+	softirq_pending_set_mask(1UL << nr);
 }
 
 void open_softirq(int nr, void (*action)(struct softirq_action *))
-- 
2.17.1

[PATCH 18/32] softirq: Introduce disabled softirq vectors bits

[Frederic Weisbecker]

Disabling the softirqs is currently an all-or-nothing operation: either
all softirqs are enabled or none of them. However we plan to introduce a
per vector granularity of this ability to improve latency response and
make each softirq vector interruptible by the others.

The first step carried here is to provide the necessary APIs to control
the per-vector enable bits.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 arch/s390/include/asm/hardirq.h |  9 ++++--
 include/linux/interrupt.h       | 53 ++++++++++++++++++++++++++++++---
 2 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/arch/s390/include/asm/hardirq.h b/arch/s390/include/asm/hardirq.h
index 54e81f520175..377fd4184f86 100644
--- a/arch/s390/include/asm/hardirq.h
+++ b/arch/s390/include/asm/hardirq.h
@@ -13,9 +13,14 @@
 
 #include <asm/lowcore.h>
 
-#define local_softirq_pending() (S390_lowcore.softirq_data)
-#define softirq_pending_clear_mask(x) (S390_lowcore.softirq_data &= ~(x))
+#define local_softirq_data() (S390_lowcore.softirq_data)
+#define local_softirq_pending() (local_softirq_data() & SOFTIRQ_PENDING_MASK)
+#define local_softirq_disabled() (local_softirq_data() & ~SOFTIRQ_PENDING_MASK)
+#define softirq_enabled_set_mask(x) (S390_lowcore.softirq_data |= ((x) << SOFTIRQ_ENABLED_SHIFT))
+#define softirq_enabled_clear_mask(x) (S390_lowcore.softirq_data &= ~((x) << SOFTIRQ_ENABLED_SHIFT))
+#define softirq_enabled_set(x) (S390_lowcore.softirq_data = ((x) << SOFTIRQ_ENABLED_SHIFT))
 #define softirq_pending_set_mask(x)  (S390_lowcore.softirq_data |= (x))
+#define softirq_pending_clear_mask(x) (S390_lowcore.softirq_data &= ~(x))
 
 #define __ARCH_IRQ_STAT
 #define __ARCH_HAS_DO_SOFTIRQ
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index c402770ae45b..1a08fb0e4781 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -474,16 +474,61 @@ enum
 #define SOFTIRQ_STOP_IDLE_MASK (~(1 << RCU_SOFTIRQ))
 #define SOFTIRQ_ALL_MASK (BIT(NR_SOFTIRQS) - 1)
 
-#ifndef local_softirq_pending
+#define SOFTIRQ_ENABLED_SHIFT 16
+#define SOFTIRQ_PENDING_MASK (BIT(SOFTIRQ_ENABLED_SHIFT) - 1)
+
+
+#ifndef local_softirq_data
 
 #ifndef local_softirq_data_ref
 #define local_softirq_data_ref irq_stat.__softirq_data
 #endif
 
-#define local_softirq_pending()	(__this_cpu_read(local_softirq_data_ref))
-#define softirq_pending_clear_mask(x)	(__this_cpu_and(local_softirq_data_ref, ~(x)))
-#define softirq_pending_set_mask(x)	(__this_cpu_or(local_softirq_data_ref, (x)))
+static inline unsigned int local_softirq_data(void)
+{
+	return __this_cpu_read(local_softirq_data_ref);
+}
 
+static inline unsigned int local_softirq_enabled(void)
+{
+	return local_softirq_data() >> SOFTIRQ_ENABLED_SHIFT;
+}
+
+static inline unsigned int local_softirq_pending(void)
+{
+	return local_softirq_data() & SOFTIRQ_PENDING_MASK;
+}
+
+static inline void softirq_enabled_clear_mask(unsigned int enabled)
+{
+	enabled <<= SOFTIRQ_ENABLED_SHIFT;
+	__this_cpu_and(local_softirq_data_ref, ~enabled);
+}
+
+static inline void softirq_enabled_set_mask(unsigned int enabled)
+{
+	enabled <<= SOFTIRQ_ENABLED_SHIFT;
+	__this_cpu_or(local_softirq_data_ref, enabled);
+}
+
+static inline void softirq_enabled_set(unsigned int enabled)
+{
+	unsigned int data;
+
+	data = enabled << SOFTIRQ_ENABLED_SHIFT;
+	data |= local_softirq_pending();
+	__this_cpu_write(local_softirq_data_ref, data);
+}
+
+static inline void softirq_pending_clear_mask(unsigned int pending)
+{
+	__this_cpu_and(local_softirq_data_ref, ~pending);
+}
+
+static inline void softirq_pending_set_mask(unsigned int pending)
+{
+	__this_cpu_or(local_softirq_data_ref, pending);
+}
 #endif /* local_softirq_pending */
 
 /* map softirq index to softirq name. update 'softirq_to_name' in
-- 
2.17.1

[PATCH 19/32] softirq: Rename _local_bh_enable() to local_bh_enable_no_softirq()

[Frederic Weisbecker]

The bottom half masking APIs have become interestingly confusing with all
these flavours:

		local_bh_enable()
		_local_bh_enable()
		local_bh_enable_ip()
		__local_bh_enable_ip()

_local_bh_enable() is an exception here because it's the only version
that won't execute do_softirq() in the end.

Clarify this straight in the name. It may help reviewers who are already
familiar with functions such as preempt_enable_no_resched().

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 arch/s390/lib/delay.c       |  2 +-
 drivers/s390/char/sclp.c    |  2 +-
 drivers/s390/cio/cio.c      |  2 +-
 include/linux/bottom_half.h |  2 +-
 kernel/softirq.c            | 12 ++++++------
 5 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/s390/lib/delay.c b/arch/s390/lib/delay.c
index d4aa10795605..3f83ee9446b7 100644
--- a/arch/s390/lib/delay.c
+++ b/arch/s390/lib/delay.c
@@ -91,7 +91,7 @@ void __udelay(unsigned long long usecs)
 	if (raw_irqs_disabled_flags(flags)) {
 		local_bh_disable();
 		__udelay_disabled(usecs);
-		_local_bh_enable();
+		local_bh_enable_no_softirq();
 		goto out;
 	}
 	__udelay_enabled(usecs);
diff --git a/drivers/s390/char/sclp.c b/drivers/s390/char/sclp.c
index e9aa71cdfc44..6c6b7456b368 100644
--- a/drivers/s390/char/sclp.c
+++ b/drivers/s390/char/sclp.c
@@ -572,7 +572,7 @@ sclp_sync_wait(void)
 	local_irq_disable();
 	__ctl_load(cr0, 0, 0);
 	if (!irq_context)
-		_local_bh_enable();
+		local_bh_enable_no_softirq();
 	local_tick_enable(old_tick);
 	local_irq_restore(flags);
 }
diff --git a/drivers/s390/cio/cio.c b/drivers/s390/cio/cio.c
index de744ca158fd..e3fb83b3c6c1 100644
--- a/drivers/s390/cio/cio.c
+++ b/drivers/s390/cio/cio.c
@@ -607,7 +607,7 @@ void cio_tsch(struct subchannel *sch)
 		inc_irq_stat(IRQIO_CIO);
 	if (!irq_context) {
 		irq_exit();
-		_local_bh_enable();
+		local_bh_enable_no_softirq();
 	}
 }
 
diff --git a/include/linux/bottom_half.h b/include/linux/bottom_half.h
index a19519f4241d..a104f815efcf 100644
--- a/include/linux/bottom_half.h
+++ b/include/linux/bottom_half.h
@@ -19,7 +19,7 @@ static inline void local_bh_disable(void)
 	__local_bh_disable_ip(_THIS_IP_, SOFTIRQ_DISABLE_OFFSET);
 }
 
-extern void _local_bh_enable(void);
+extern void local_bh_enable_no_softirq(void);
 extern void __local_bh_enable_ip(unsigned long ip, unsigned int cnt);
 
 static inline void local_bh_enable_ip(unsigned long ip)
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 5f167fc43ab9..d305b4c8d1a7 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -139,7 +139,7 @@ void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
 EXPORT_SYMBOL(__local_bh_disable_ip);
 #endif /* CONFIG_TRACE_IRQFLAGS */
 
-static void __local_bh_enable(unsigned int cnt)
+static void __local_bh_enable_no_softirq(unsigned int cnt)
 {
 	lockdep_assert_irqs_disabled();
 
@@ -156,12 +156,12 @@ static void __local_bh_enable(unsigned int cnt)
  * Special-case - softirqs can safely be enabled by __do_softirq(),
  * without processing still-pending softirqs:
  */
-void _local_bh_enable(void)
+void local_bh_enable_no_softirq(void)
 {
 	WARN_ON_ONCE(in_irq());
-	__local_bh_enable(SOFTIRQ_DISABLE_OFFSET);
+	__local_bh_enable_no_softirq(SOFTIRQ_DISABLE_OFFSET);
 }
-EXPORT_SYMBOL(_local_bh_enable);
+EXPORT_SYMBOL(local_bh_enable_no_softirq);
 
 void __local_bh_enable_ip(unsigned long ip, unsigned int cnt)
 {
@@ -316,7 +316,7 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)
 
 	lockdep_softirq_end(in_hardirq);
 	account_irq_exit_time(current);
-	__local_bh_enable(SOFTIRQ_OFFSET);
+	__local_bh_enable_no_softirq(SOFTIRQ_OFFSET);
 	WARN_ON_ONCE(in_interrupt());
 	current_restore_flags(old_flags, PF_MEMALLOC);
 }
@@ -352,7 +352,7 @@ void irq_enter(void)
 		 */
 		local_bh_disable();
 		tick_irq_enter();
-		_local_bh_enable();
+		local_bh_enable_no_softirq();
 	}
 
 	__irq_enter();
-- 
2.17.1

[PATCH 20/32] softirq: Move vectors bits to bottom_half.h

[Frederic Weisbecker]

From: Frederic Weisbecker <fweisbec@gmail.com>

Using the bottom-half masking APIs defined in linux/bottom-half.h won't
be possible without passing the relevant softirq vectors that are
currently defined in linux/interrupt.h

Yet we can't include linux/interrupt.h from linux/bottom-half.h due to
circular dependencies.

Now the vector bits belong to bottom halves anyway, so moving them there
is more natural and avoid nasty header dances.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 include/linux/bottom_half.h | 24 ++++++++++++++++++++++++
 include/linux/interrupt.h   | 21 ---------------------
 2 files changed, 24 insertions(+), 21 deletions(-)

diff --git a/include/linux/bottom_half.h b/include/linux/bottom_half.h
index a104f815efcf..39aaf9189226 100644
--- a/include/linux/bottom_half.h
+++ b/include/linux/bottom_half.h
@@ -4,6 +4,30 @@
 
 #include <linux/preempt.h>
 
+
+/*
+ * PLEASE, avoid to allocate new softirqs, if you need not _really_ high
+ * frequency threaded job scheduling. For almost all the purposes
+ * tasklets are more than enough. F.e. all serial device BHs et
+ * al. should be converted to tasklets, not to softirqs.
+ */
+enum
+{
+#define SOFTIRQ_VECTOR(__SVEC) \
+	__SVEC##_SOFTIRQ,
+#include <linux/softirq_vector.h>
+#undef SOFTIRQ_VECTOR
+	NR_SOFTIRQS
+};
+
+#define SOFTIRQ_STOP_IDLE_MASK (~(1 << RCU_SOFTIRQ))
+#define SOFTIRQ_ALL_MASK (BIT(NR_SOFTIRQS) - 1)
+
+#define SOFTIRQ_ENABLED_SHIFT 16
+#define SOFTIRQ_PENDING_MASK (BIT(SOFTIRQ_ENABLED_SHIFT) - 1)
+
+
+
 #ifdef CONFIG_TRACE_IRQFLAGS
 extern void __local_bh_disable_ip(unsigned long ip, unsigned int cnt);
 #else
diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 1a08fb0e4781..346fb1e8e55b 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -457,27 +457,6 @@ extern bool force_irqthreads;
 #define hard_irq_disable()	do { } while(0)
 #endif
 
-/* PLEASE, avoid to allocate new softirqs, if you need not _really_ high
-   frequency threaded job scheduling. For almost all the purposes
-   tasklets are more than enough. F.e. all serial device BHs et
-   al. should be converted to tasklets, not to softirqs.
- */
-enum
-{
-#define SOFTIRQ_VECTOR(__SVEC) \
-	__SVEC##_SOFTIRQ,
-#include <linux/softirq_vector.h>
-#undef SOFTIRQ_VECTOR
-	NR_SOFTIRQS
-};
-
-#define SOFTIRQ_STOP_IDLE_MASK (~(1 << RCU_SOFTIRQ))
-#define SOFTIRQ_ALL_MASK (BIT(NR_SOFTIRQS) - 1)
-
-#define SOFTIRQ_ENABLED_SHIFT 16
-#define SOFTIRQ_PENDING_MASK (BIT(SOFTIRQ_ENABLED_SHIFT) - 1)
-
-
 #ifndef local_softirq_data
 
 #ifndef local_softirq_data_ref
-- 
2.17.1

[PATCH 21/32] x86: Init softirq enabled field

[Frederic Weisbecker]

From: Frederic Weisbecker <fweisbec@gmail.com>

All softirqs must be set enabled on boot.

Signed-off-by: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 arch/x86/kernel/irq.c       | 5 ++++-
 include/linux/bottom_half.h | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/irq.c b/arch/x86/kernel/irq.c
index 59b5f2ea7c2f..b859861733a4 100644
--- a/arch/x86/kernel/irq.c
+++ b/arch/x86/kernel/irq.c
@@ -11,6 +11,7 @@
 #include <linux/delay.h>
 #include <linux/export.h>
 #include <linux/irq.h>
+#include <linux/bottom_half.h>
 
 #include <asm/apic.h>
 #include <asm/io_apic.h>
@@ -22,7 +23,9 @@
 #define CREATE_TRACE_POINTS
 #include <asm/trace/irq_vectors.h>
 
-DEFINE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat);
+DEFINE_PER_CPU_SHARED_ALIGNED(irq_cpustat_t, irq_stat) = {
+	.__softirq_data = SOFTIRQ_DATA_INIT,
+};
 EXPORT_PER_CPU_SYMBOL(irq_stat);
 
 DEFINE_PER_CPU(struct pt_regs *, irq_regs);
diff --git a/include/linux/bottom_half.h b/include/linux/bottom_half.h
index 39aaf9189226..240419382978 100644
--- a/include/linux/bottom_half.h
+++ b/include/linux/bottom_half.h
@@ -26,6 +26,8 @@ enum
 #define SOFTIRQ_ENABLED_SHIFT 16
 #define SOFTIRQ_PENDING_MASK (BIT(SOFTIRQ_ENABLED_SHIFT) - 1)
 
+#define SOFTIRQ_DATA_INIT (SOFTIRQ_ALL_MASK << SOFTIRQ_ENABLED_SHIFT)
+
 
 
 #ifdef CONFIG_TRACE_IRQFLAGS
-- 
2.17.1

[PATCH 22/32] softirq: Check enabled vectors before processing

[Frederic Weisbecker]

There is no need to process softirqs if none of those pending are
enabled. Check about that early to avoid unnecessary overhead.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 include/linux/interrupt.h |  5 +++++
 kernel/softirq.c          | 21 +++++++++++----------
 2 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/include/linux/interrupt.h b/include/linux/interrupt.h
index 346fb1e8e55b..161babfc1a0d 100644
--- a/include/linux/interrupt.h
+++ b/include/linux/interrupt.h
@@ -508,6 +508,11 @@ static inline void softirq_pending_set_mask(unsigned int pending)
 {
 	__this_cpu_or(local_softirq_data_ref, pending);
 }
+
+static inline int softirq_pending_enabled(void)
+{
+	return local_softirq_pending() & local_softirq_enabled();
+}
 #endif /* local_softirq_pending */
 
 /* map softirq index to softirq name. update 'softirq_to_name' in
diff --git a/kernel/softirq.c b/kernel/softirq.c
index d305b4c8d1a7..a2257a5aaa0b 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -181,7 +181,7 @@ void __local_bh_enable_ip(unsigned long ip, unsigned int cnt)
 	 */
 	preempt_count_sub(cnt - 1);
 
-	if (unlikely(!in_interrupt() && local_softirq_pending())) {
+	if (unlikely(!in_interrupt() && softirq_pending_enabled())) {
 		/*
 		 * Run softirq if any pending. And do it in its own stack
 		 * as we may be calling this deep in a task call stack already.
@@ -250,7 +250,7 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)
 	int max_restart = MAX_SOFTIRQ_RESTART;
 	struct softirq_action *h;
 	bool in_hardirq;
-	__u32 pending;
+	__u32 pending, enabled;
 	int softirq_bit;
 
 	/*
@@ -260,7 +260,8 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)
 	 */
 	current->flags &= ~PF_MEMALLOC;
 
-	pending = local_softirq_pending();
+	enabled = local_softirq_enabled();
+	pending = local_softirq_pending() & enabled;
 	account_irq_enter_time(current);
 
 	__local_bh_disable_ip(_RET_IP_, SOFTIRQ_OFFSET);
@@ -268,7 +269,7 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)
 
 restart:
 	/* Reset the pending bitmask before enabling irqs */
-	softirq_pending_clear_mask(SOFTIRQ_ALL_MASK);
+	softirq_pending_clear_mask(pending);
 
 	local_irq_enable();
 
@@ -305,7 +306,7 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)
 		rcu_softirq_qs();
 	local_irq_disable();
 
-	pending = local_softirq_pending();
+	pending = local_softirq_pending() & enabled;
 	if (pending) {
 		if (time_before(jiffies, end) && !need_resched() &&
 		    --max_restart)
@@ -331,7 +332,7 @@ asmlinkage __visible void do_softirq(void)
 
 	local_irq_save(flags);
 
-	pending = local_softirq_pending();
+	pending = softirq_pending_enabled();
 
 	if (pending && !ksoftirqd_running(pending))
 		do_softirq_own_stack();
@@ -360,7 +361,7 @@ void irq_enter(void)
 
 static inline void invoke_softirq(void)
 {
-	if (ksoftirqd_running(local_softirq_pending()))
+	if (ksoftirqd_running(softirq_pending_enabled()))
 		return;
 
 	if (!force_irqthreads) {
@@ -409,7 +410,7 @@ void irq_exit(void)
 #endif
 	account_irq_exit_time(current);
 	preempt_count_sub(HARDIRQ_OFFSET);
-	if (!in_interrupt() && local_softirq_pending())
+	if (!in_interrupt() && softirq_pending_enabled())
 		invoke_softirq();
 
 	tick_irq_exit();
@@ -640,13 +641,13 @@ void __init softirq_init(void)
 
 static int ksoftirqd_should_run(unsigned int cpu)
 {
-	return local_softirq_pending();
+	return softirq_pending_enabled();
 }
 
 static void run_ksoftirqd(unsigned int cpu)
 {
 	local_irq_disable();
-	if (local_softirq_pending()) {
+	if (softirq_pending_enabled()) {
 		/*
 		 * We can safely run softirq on inline stack, as we are not deep
 		 * in the task stack here.
-- 
2.17.1

[PATCH 23/32] softirq: Remove stale comment

[Frederic Weisbecker]

__local_bh_disable_ip() is neither for strict internal use nor does it
require the caller to disable hardirqs. Probaby a celebration for ancient
behaviour.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/softirq.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/kernel/softirq.c b/kernel/softirq.c
index a2257a5aaa0b..5fea9e299caf 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -102,10 +102,6 @@ static bool ksoftirqd_running(unsigned long pending)
  * softirq and whether we just have bh disabled.
  */
 
-/*
- * This one is for softirq.c-internal use,
- * where hardirqs are disabled legitimately:
- */
 #ifdef CONFIG_TRACE_IRQFLAGS
 void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
 {
-- 
2.17.1

Re: [PATCH 23/32] softirq: Remove stale comment

[Sebastian Andrzej Siewior]

On 2019-02-12 18:14:14 [+0100], Frederic Weisbecker wrote:
> __local_bh_disable_ip() is neither for strict internal use nor does it
> require the caller to disable hardirqs. Probaby a celebration for ancient

Probaby

> behaviour.

I think the point was to override the IP for the tracer. So everyone
else used local_bh_disable() and was recorded as the caller except for
softirq.c internal usage where __do_softirq() did also
"local_bh_disable()" but recorded its caller (instead recording
__do_softirq()).

Sebastian

Re: [PATCH 23/32] softirq: Remove stale comment

[Frederic Weisbecker]

On Wed, Feb 27, 2019 at 12:04:04PM +0100, Sebastian Andrzej Siewior wrote:
> On 2019-02-12 18:14:14 [+0100], Frederic Weisbecker wrote:
> > __local_bh_disable_ip() is neither for strict internal use nor does it
> > require the caller to disable hardirqs. Probaby a celebration for ancient
> 
> Probaby
> 
> > behaviour.
> 
> I think the point was to override the IP for the tracer. So everyone
> else used local_bh_disable() and was recorded as the caller except for
> softirq.c internal usage where __do_softirq() did also
> "local_bh_disable()" but recorded its caller (instead recording
> __do_softirq()).

Looks so. Anyway now it's also used by locking functions that need to pass
their own callers. So the comment is stale.

[PATCH 24/32] softirq: Uninline !CONFIG_TRACE_IRQFLAGS __local_bh_disable_ip()

[Frederic Weisbecker]

The common code between both versions of __local_bh_disable_ip(), whether
CONFIG_TRACE_IRQFLAGS is on or off, is going to grow up in order to
support vector masking granularity.

Merge these versions together to prepare for that.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 include/linux/bottom_half.h | 10 ----------
 kernel/softirq.c            | 10 ++++++----
 2 files changed, 6 insertions(+), 14 deletions(-)

diff --git a/include/linux/bottom_half.h b/include/linux/bottom_half.h
index 240419382978..ef9e4c752f56 100644
--- a/include/linux/bottom_half.h
+++ b/include/linux/bottom_half.h
@@ -28,17 +28,7 @@ enum
 
 #define SOFTIRQ_DATA_INIT (SOFTIRQ_ALL_MASK << SOFTIRQ_ENABLED_SHIFT)
 
-
-
-#ifdef CONFIG_TRACE_IRQFLAGS
 extern void __local_bh_disable_ip(unsigned long ip, unsigned int cnt);
-#else
-static __always_inline void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
-{
-	preempt_count_add(cnt);
-	barrier();
-}
-#endif
 
 static inline void local_bh_disable(void)
 {
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 5fea9e299caf..91dee716e139 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -102,14 +102,14 @@ static bool ksoftirqd_running(unsigned long pending)
  * softirq and whether we just have bh disabled.
  */
 
-#ifdef CONFIG_TRACE_IRQFLAGS
 void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
 {
+#ifdef CONFIG_TRACE_IRQFLAGS
 	unsigned long flags;
 
-	WARN_ON_ONCE(in_irq());
-
 	raw_local_irq_save(flags);
+#endif
+	WARN_ON_ONCE(in_irq());
 	/*
 	 * The preempt tracer hooks into preempt_count_add and will break
 	 * lockdep because it calls back into lockdep after SOFTIRQ_OFFSET
@@ -123,7 +123,10 @@ void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
 	 */
 	if (softirq_count() == (cnt & SOFTIRQ_MASK))
 		trace_softirqs_off(ip);
+
+#ifdef CONFIG_TRACE_IRQFLAGS
 	raw_local_irq_restore(flags);
+#endif
 
 	if (preempt_count() == cnt) {
 #ifdef CONFIG_DEBUG_PREEMPT
@@ -133,7 +136,6 @@ void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
 	}
 }
 EXPORT_SYMBOL(__local_bh_disable_ip);
-#endif /* CONFIG_TRACE_IRQFLAGS */
 
 static void __local_bh_enable_no_softirq(unsigned int cnt)
 {
-- 
2.17.1

Re: [PATCH 24/32] softirq: Uninline !CONFIG_TRACE_IRQFLAGS __local_bh_disable_ip()

[Sebastian Andrzej Siewior]

On 2019-02-12 18:14:15 [+0100], Frederic Weisbecker wrote:
> diff --git a/include/linux/bottom_half.h b/include/linux/bottom_half.h
> index 240419382978..ef9e4c752f56 100644
> --- a/include/linux/bottom_half.h
> +++ b/include/linux/bottom_half.h
> @@ -28,17 +28,7 @@ enum
>  
>  #define SOFTIRQ_DATA_INIT (SOFTIRQ_ALL_MASK << SOFTIRQ_ENABLED_SHIFT)
>  
> -
> -
> -#ifdef CONFIG_TRACE_IRQFLAGS
>  extern void __local_bh_disable_ip(unsigned long ip, unsigned int cnt);
> -#else
> -static __always_inline void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
> -{
> -	preempt_count_add(cnt);
> -	barrier();
> -}
> -#endif

you are aware that a simple local_bh_disable() / spin_lock_bh() gains a
function call. Let me look further how much code you are going to add
here…

Sebastian

Re: [PATCH 24/32] softirq: Uninline !CONFIG_TRACE_IRQFLAGS __local_bh_disable_ip()

[Frederic Weisbecker]

On Wed, Feb 27, 2019 at 12:14:29PM +0100, Sebastian Andrzej Siewior wrote:
> On 2019-02-12 18:14:15 [+0100], Frederic Weisbecker wrote:
> > diff --git a/include/linux/bottom_half.h b/include/linux/bottom_half.h
> > index 240419382978..ef9e4c752f56 100644
> > --- a/include/linux/bottom_half.h
> > +++ b/include/linux/bottom_half.h
> > @@ -28,17 +28,7 @@ enum
> >  
> >  #define SOFTIRQ_DATA_INIT (SOFTIRQ_ALL_MASK << SOFTIRQ_ENABLED_SHIFT)
> >  
> > -
> > -
> > -#ifdef CONFIG_TRACE_IRQFLAGS
> >  extern void __local_bh_disable_ip(unsigned long ip, unsigned int cnt);
> > -#else
> > -static __always_inline void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
> > -{
> > -	preempt_count_add(cnt);
> > -	barrier();
> > -}
> > -#endif
> 
> you are aware that a simple local_bh_disable() / spin_lock_bh() gains a
> function call. Let me look further how much code you are going to add
> here…

Indeed but as you'll find out, the function is growing beyond reasonable inlining.

[PATCH 25/32] softirq: Prepare for mixing all/per-vector masking

[Frederic Weisbecker]

In order to be able to mix and nest full and per vector softirq masking,
we need to be able to track the nesting state using a "full masking"
counter and a mask of "individual disabled vectors".

Start with introducing the full masking counter. For now it's a simple
mirror of softirq_count() because there is no per vector masking API
yet.

When this full masking counter is non 0, all softirq vectors are
explicitly disabled.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/softirq.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/kernel/softirq.c b/kernel/softirq.c
index 91dee716e139..4477a03afd94 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -57,6 +57,12 @@ static struct softirq_action softirq_vec[NR_SOFTIRQS] __cacheline_aligned_in_smp
 
 DEFINE_PER_CPU(struct task_struct *, ksoftirqd);
 
+struct softirq_nesting {
+	unsigned int disabled_all;
+};
+
+static DEFINE_PER_CPU(struct softirq_nesting, softirq_nesting);
+
 const char * const softirq_to_name[NR_SOFTIRQS] = {
 	"HI", "TIMER", "NET_TX", "NET_RX", "BLOCK", "IRQ_POLL",
 	"TASKLET", "SCHED", "HRTIMER", "RCU"
@@ -118,11 +124,11 @@ void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
 	 * call the trace_preempt_off later.
 	 */
 	__preempt_count_add(cnt);
-	/*
-	 * Were softirqs turned off above:
-	 */
-	if (softirq_count() == (cnt & SOFTIRQ_MASK))
+
+	if (__this_cpu_inc_return(softirq_nesting.disabled_all) == 1) {
+		softirq_enabled_clear_mask(SOFTIRQ_ALL_MASK);
 		trace_softirqs_off(ip);
+	}
 
 #ifdef CONFIG_TRACE_IRQFLAGS
 	raw_local_irq_restore(flags);
@@ -137,6 +143,15 @@ void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
 }
 EXPORT_SYMBOL(__local_bh_disable_ip);
 
+static void local_bh_enable_common(unsigned long ip, unsigned int cnt)
+{
+	if (__this_cpu_dec_return(softirq_nesting.disabled_all))
+		return;
+
+	softirq_enabled_set(SOFTIRQ_ALL_MASK);
+	trace_softirqs_on(ip);
+}
+
 static void __local_bh_enable_no_softirq(unsigned int cnt)
 {
 	lockdep_assert_irqs_disabled();
@@ -144,8 +159,7 @@ static void __local_bh_enable_no_softirq(unsigned int cnt)
 	if (preempt_count() == cnt)
 		trace_preempt_on(CALLER_ADDR0, get_lock_parent_ip());
 
-	if (softirq_count() == (cnt & SOFTIRQ_MASK))
-		trace_softirqs_on(_RET_IP_);
+	local_bh_enable_common(_RET_IP_, cnt);
 
 	__preempt_count_sub(cnt);
 }
@@ -168,11 +182,8 @@ void __local_bh_enable_ip(unsigned long ip, unsigned int cnt)
 #ifdef CONFIG_TRACE_IRQFLAGS
 	local_irq_disable();
 #endif
-	/*
-	 * Are softirqs going to be turned on now:
-	 */
-	if (softirq_count() == SOFTIRQ_DISABLE_OFFSET)
-		trace_softirqs_on(ip);
+	local_bh_enable_common(ip, cnt);
+
 	/*
 	 * Keep preemption disabled until we are done with
 	 * softirq processing:
-- 
2.17.1

[PATCH 26/32] softirq: Support per vector masking

[Frederic Weisbecker]

Provide the low level APIs to support per-vector masking. In order
to allow these to properly nest with itself and with full softirq
masking APIs, we provide two mechanisms:

1) Self nesting: use a caller stack saved/restored state model similar to
  that of local_irq_save() and local_irq_restore():

      bh = local_bh_disable_mask(BIT(NET_RX_SOFTIRQ));
      [...]
          bh2 = local_bh_disable_mask(BIT(TIMER_SOFTIRQ));
          [...]
          local_bh_enable_mask(bh2);
      local_bh_enable_mask(bh);

2) Nest against full masking: save the per-vector disabled state prior
   to the first full disable operation and restore it on the last full
   enable operation:

      bh = local_bh_disable_mask(BIT(NET_RX_SOFTIRQ));
      [...]
          local_bh_disable() <---- save state with NET_RX_SOFTIRQ disabled
          [...]
          local_bh_enable() <---- restore state with NET_RX_SOFTIRQ disabled
      local_bh_enable_mask(bh);

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 include/linux/bottom_half.h |  7 +++
 kernel/softirq.c            | 85 +++++++++++++++++++++++++++++++------
 2 files changed, 80 insertions(+), 12 deletions(-)

diff --git a/include/linux/bottom_half.h b/include/linux/bottom_half.h
index ef9e4c752f56..a6996e3f4526 100644
--- a/include/linux/bottom_half.h
+++ b/include/linux/bottom_half.h
@@ -35,6 +35,10 @@ static inline void local_bh_disable(void)
 	__local_bh_disable_ip(_THIS_IP_, SOFTIRQ_DISABLE_OFFSET);
 }
 
+extern unsigned int local_bh_disable_mask(unsigned long ip,
+					  unsigned int cnt, unsigned int mask);
+
+
 extern void local_bh_enable_no_softirq(void);
 extern void __local_bh_enable_ip(unsigned long ip, unsigned int cnt);
 
@@ -48,4 +52,7 @@ static inline void local_bh_enable(void)
 	__local_bh_enable_ip(_THIS_IP_, SOFTIRQ_DISABLE_OFFSET);
 }
 
+extern void local_bh_enable_mask(unsigned long ip, unsigned int cnt,
+				 unsigned int mask);
+
 #endif /* _LINUX_BH_H */
diff --git a/kernel/softirq.c b/kernel/softirq.c
index 4477a03afd94..4a32effbb1fc 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -59,6 +59,7 @@ DEFINE_PER_CPU(struct task_struct *, ksoftirqd);
 
 struct softirq_nesting {
 	unsigned int disabled_all;
+	unsigned int enabled_vector;
 };
 
 static DEFINE_PER_CPU(struct softirq_nesting, softirq_nesting);
@@ -108,8 +109,10 @@ static bool ksoftirqd_running(unsigned long pending)
  * softirq and whether we just have bh disabled.
  */
 
-void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
+static unsigned int local_bh_disable_common(unsigned long ip, unsigned int cnt,
+					    bool per_vec, unsigned int vec_mask)
 {
+	unsigned int enabled;
 #ifdef CONFIG_TRACE_IRQFLAGS
 	unsigned long flags;
 
@@ -125,10 +128,31 @@ void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
 	 */
 	__preempt_count_add(cnt);
 
-	if (__this_cpu_inc_return(softirq_nesting.disabled_all) == 1) {
-		softirq_enabled_clear_mask(SOFTIRQ_ALL_MASK);
-		trace_softirqs_off(ip);
-	}
+	enabled = local_softirq_enabled();
+
+	/*
+	 * Handle nesting of full/per-vector masking. Per vector masking
+	 * takes effect only if full masking hasn't taken place yet.
+	 */
+	if (!__this_cpu_read(softirq_nesting.disabled_all)) {
+		if (enabled & vec_mask) {
+			softirq_enabled_clear_mask(vec_mask);
+			if (!local_softirq_enabled())
+				trace_softirqs_off(ip);
+		}
+
+		/*
+		 * Save the state prior to full masking. We'll restore it
+		 * on next non-nesting full unmasking in case some vectors
+		 * have been individually disabled before (case of full masking
+		 * nesting inside per-vector masked code).
+		 */
+		if (!per_vec)
+			__this_cpu_write(softirq_nesting.enabled_vector, enabled);
+        }
+
+	if (!per_vec)
+		__this_cpu_inc(softirq_nesting.disabled_all);
 
 #ifdef CONFIG_TRACE_IRQFLAGS
 	raw_local_irq_restore(flags);
@@ -140,15 +164,38 @@ void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
 #endif
 		trace_preempt_off(CALLER_ADDR0, get_lock_parent_ip());
 	}
+
+	return enabled;
+}
+
+void __local_bh_disable_ip(unsigned long ip, unsigned int cnt)
+{
+	local_bh_disable_common(ip, cnt, false, SOFTIRQ_ALL_MASK);
 }
 EXPORT_SYMBOL(__local_bh_disable_ip);
 
-static void local_bh_enable_common(unsigned long ip, unsigned int cnt)
+unsigned int local_bh_disable_mask(unsigned long ip, unsigned int cnt,
+				   unsigned int vec_mask)
 {
-	if (__this_cpu_dec_return(softirq_nesting.disabled_all))
-		return;
+	return local_bh_disable_common(ip, cnt, true, vec_mask);
+}
+EXPORT_SYMBOL(local_bh_disable_mask);
 
-	softirq_enabled_set(SOFTIRQ_ALL_MASK);
+static void local_bh_enable_common(unsigned long ip, unsigned int cnt,
+				   bool per_vec, unsigned int mask)
+{
+	/*
+	 * Restore the previous softirq mask state. If this was the last
+	 * full unmasking, restore what was saved.
+	 */
+	if (!per_vec) {
+		if (__this_cpu_dec_return(softirq_nesting.disabled_all))
+			return;
+		else
+			mask = __this_cpu_read(softirq_nesting.enabled_vector);
+	}
+
+	softirq_enabled_set(mask);
 	trace_softirqs_on(ip);
 }
 
@@ -159,7 +206,7 @@ static void __local_bh_enable_no_softirq(unsigned int cnt)
 	if (preempt_count() == cnt)
 		trace_preempt_on(CALLER_ADDR0, get_lock_parent_ip());
 
-	local_bh_enable_common(_RET_IP_, cnt);
+	local_bh_enable_common(_RET_IP_, cnt, false, SOFTIRQ_ALL_MASK);
 
 	__preempt_count_sub(cnt);
 }
@@ -175,14 +222,15 @@ void local_bh_enable_no_softirq(void)
 }
 EXPORT_SYMBOL(local_bh_enable_no_softirq);
 
-void __local_bh_enable_ip(unsigned long ip, unsigned int cnt)
+static void local_bh_enable_ip_mask(unsigned long ip, unsigned int cnt,
+				    bool per_vec, unsigned int mask)
 {
 	WARN_ON_ONCE(in_irq());
 	lockdep_assert_irqs_enabled();
 #ifdef CONFIG_TRACE_IRQFLAGS
 	local_irq_disable();
 #endif
-	local_bh_enable_common(ip, cnt);
+	local_bh_enable_common(ip, cnt, per_vec, mask);
 
 	/*
 	 * Keep preemption disabled until we are done with
@@ -204,8 +252,21 @@ void __local_bh_enable_ip(unsigned long ip, unsigned int cnt)
 #endif
 	preempt_check_resched();
 }
+
+void __local_bh_enable_ip(unsigned long ip, unsigned int cnt)
+{
+	local_bh_enable_ip_mask(ip, cnt, false, SOFTIRQ_ALL_MASK);
+}
 EXPORT_SYMBOL(__local_bh_enable_ip);
 
+void local_bh_enable_mask(unsigned long ip, unsigned int cnt,
+			  unsigned int mask)
+{
+	local_bh_enable_ip_mask(ip, cnt, true, mask);
+}
+EXPORT_SYMBOL(local_bh_enable_mask);
+
+
 /*
  * We restart softirq processing for at most MAX_SOFTIRQ_RESTART times,
  * but break the loop if need_resched() is set or after 2 ms.
-- 
2.17.1

[PATCH 27/32] locking/lockdep: Remove redundant softirqs on check

[Frederic Weisbecker]

It makes no more sense to check for redundant softirqs on because
trace_softirqs_on() is no more symmetrical to trace_softirqs_off().

Indeed trace_softirqs_off() is called whenever we know that all softirq
vectors have been disabled. And trace_softirqs_on() is called everytime
we enable at least one vector. So curr->softirqs_enabled may well remain
true throughout subsequent calls.

FIXME: Perhaps we should rename those functions. Another solution would
be to make curr->softirqs_enabled to record the last value of
local_softirqs_enabled() so we could track again redundant calls.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/locking/lockdep.c           | 5 -----
 kernel/locking/lockdep_internals.h | 1 -
 kernel/locking/lockdep_proc.c      | 2 +-
 3 files changed, 1 insertion(+), 7 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 6a74b433fe4c..9bb39677fd97 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2976,11 +2976,6 @@ void trace_softirqs_on(unsigned long ip)
 	if (DEBUG_LOCKS_WARN_ON(!irqs_disabled()))
 		return;
 
-	if (curr->softirqs_enabled) {
-		debug_atomic_inc(redundant_softirqs_on);
-		return;
-	}
-
 	current->lockdep_recursion = 1;
 	/*
 	 * We'll do an OFF -> ON transition:
diff --git a/kernel/locking/lockdep_internals.h b/kernel/locking/lockdep_internals.h
index 4b0c03f0f7ce..3401e4a91afb 100644
--- a/kernel/locking/lockdep_internals.h
+++ b/kernel/locking/lockdep_internals.h
@@ -186,7 +186,6 @@ struct lockdep_stats {
 	int	redundant_hardirqs_off;
 	int	softirqs_on_events;
 	int	softirqs_off_events;
-	int	redundant_softirqs_on;
 	int	redundant_softirqs_off;
 	int	nr_unused_locks;
 	int	nr_redundant_checks;
diff --git a/kernel/locking/lockdep_proc.c b/kernel/locking/lockdep_proc.c
index 3d31f9b0059e..4157560b36d2 100644
--- a/kernel/locking/lockdep_proc.c
+++ b/kernel/locking/lockdep_proc.c
@@ -169,7 +169,7 @@ static void lockdep_stats_debug_show(struct seq_file *m)
 			   hr2 = debug_atomic_read(redundant_hardirqs_off),
 			   si1 = debug_atomic_read(softirqs_on_events),
 			   si2 = debug_atomic_read(softirqs_off_events),
-			   sr1 = debug_atomic_read(redundant_softirqs_on),
+			   sr1 = 0,
 			   sr2 = debug_atomic_read(redundant_softirqs_off);
 
 	seq_printf(m, " chain lookup misses:           %11llu\n",
-- 
2.17.1

[PATCH 28/32] locking/lockdep: Update check_flags() according to new layout

[Frederic Weisbecker]

current->softirqs_enabled used to mean that either all or no softirqs
are enabled. Now things are getting a bit different with the new per
vector masking extension after which current->softirqs_enabled will
stay on as long as there is a single vector still enabled.

Let's adapt the check to the updated semantics. We can't deduce much
from softirq_count() alone anymore except when it's 0.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/locking/lockdep.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 9bb39677fd97..acd82145f6a6 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -3909,13 +3909,8 @@ static void check_flags(unsigned long flags)
 	 * check if not in hardirq contexts:
 	 */
 	if (!hardirq_count()) {
-		if (softirq_count()) {
-			/* like the above, but with softirqs */
-			DEBUG_LOCKS_WARN_ON(current->softirqs_enabled);
-		} else {
-			/* lick the above, does it taste good? */
+		if (!softirq_count())
 			DEBUG_LOCKS_WARN_ON(!current->softirqs_enabled);
-		}
 	}
 
 	if (!debug_locks)
-- 
2.17.1

[PATCH 29/32] locking/lockdep: Branch the new vec-finegrained softirq masking to lockdep

[Frederic Weisbecker]

Now that we have full support from softirqs to perform per vector
masking, let's feed lockdep with the proper inputs and push the vector
numbers involved in a base softirq lock usage:

LOCK_ENABLED_SOFTIRQ: push local_softirq_enabled()
LOCK_USED_IN_SOFTIRQ: push curr->softirq_context, modified by
                      lockdep_softirq_enter/exit()

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/locking/lockdep.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index acd82145f6a6..570eea5376ec 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2877,6 +2877,7 @@ static void __trace_hardirqs_on_caller(unsigned long ip)
 	 */
 	if (curr->softirqs_enabled) {
 		usage.bit = LOCK_ENABLED_SOFTIRQ;
+		usage.vector = local_softirq_enabled();
 		if (!mark_held_locks(curr, &usage))
 			return;
 	}
@@ -2964,6 +2965,7 @@ void trace_softirqs_on(unsigned long ip)
 	struct task_struct *curr = current;
 	struct lock_usage usage = {
 		.bit = LOCK_ENABLED_SOFTIRQ,
+		.vector = local_softirq_enabled()
 	};
 
 	if (unlikely(!debug_locks || current->lockdep_recursion))
@@ -3028,7 +3030,7 @@ void trace_softirqs_off(unsigned long ip)
 
 static int mark_irqflags(struct task_struct *curr, struct held_lock *hlock)
 {
-	struct lock_usage usage = { .vector = 0 };
+	struct lock_usage usage;
 	/*
 	 * If non-trylock use in a hardirq or softirq context, then
 	 * mark the lock as used in these contexts:
@@ -3037,22 +3039,26 @@ static int mark_irqflags(struct task_struct *curr, struct held_lock *hlock)
 		if (hlock->read) {
 			if (curr->hardirq_context) {
 				usage.bit = LOCK_USED_IN_HARDIRQ_READ;
+				usage.vector = 0;
 				if (!mark_lock(curr, hlock, &usage))
 					return 0;
 			}
 			if (curr->softirq_context) {
 				usage.bit = LOCK_USED_IN_SOFTIRQ_READ;
+				usage.vector = curr->softirq_context;
 				if (!mark_lock(curr, hlock, &usage))
 					return 0;
 			}
 		} else {
 			if (curr->hardirq_context) {
 				usage.bit = LOCK_USED_IN_HARDIRQ;
+				usage.vector = 0;
 				if (!mark_lock(curr, hlock, &usage))
 					return 0;
 			}
 			if (curr->softirq_context) {
 				usage.bit = LOCK_USED_IN_SOFTIRQ;
+				usage.vector = curr->softirq_context;
 				if (!mark_lock(curr, hlock, &usage))
 					return 0;
 			}
@@ -3061,19 +3067,23 @@ static int mark_irqflags(struct task_struct *curr, struct held_lock *hlock)
 	if (!hlock->hardirqs_off) {
 		if (hlock->read) {
 			usage.bit = LOCK_ENABLED_HARDIRQ_READ;
+			usage.vector = 0;
 			if (!mark_lock(curr, hlock, &usage))
 				return 0;
 			if (curr->softirqs_enabled) {
 				usage.bit = LOCK_ENABLED_SOFTIRQ_READ;
+				usage.vector = local_softirq_enabled();
 				if (!mark_lock(curr, hlock, &usage))
 					return 0;
 			}
 		} else {
 			usage.bit = LOCK_ENABLED_HARDIRQ;
+			usage.vector = 0;
 			if (!mark_lock(curr, hlock, &usage))
 				return 0;
 			if (curr->softirqs_enabled) {
 				usage.bit = LOCK_ENABLED_SOFTIRQ;
+				usage.vector = local_softirq_enabled();
 				if (!mark_lock(curr, hlock, &usage))
 					return 0;
 			}
-- 
2.17.1

[PATCH 30/32] softirq: Allow to soft interrupt vector-specific masked contexts

[Frederic Weisbecker]

Remove the old protections that prevented softirqs from interrupting any
softirq-disabled context. Now that we can disable specific vectors on
a given piece of code, we want to be able to soft-interrupt those places
with other vectors.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 kernel/softirq.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/kernel/softirq.c b/kernel/softirq.c
index 4a32effbb1fc..42dfcdfa423b 100644
--- a/kernel/softirq.c
+++ b/kernel/softirq.c
@@ -238,7 +238,7 @@ static void local_bh_enable_ip_mask(unsigned long ip, unsigned int cnt,
 	 */
 	preempt_count_sub(cnt - 1);
 
-	if (unlikely(!in_interrupt() && softirq_pending_enabled())) {
+	if (unlikely(softirq_pending_enabled())) {
 		/*
 		 * Run softirq if any pending. And do it in its own stack
 		 * as we may be calling this deep in a task call stack already.
@@ -388,7 +388,7 @@ asmlinkage __visible void __softirq_entry __do_softirq(void)
 	lockdep_softirq_end(in_hardirq);
 	account_irq_exit_time(current);
 	__local_bh_enable_no_softirq(SOFTIRQ_OFFSET);
-	WARN_ON_ONCE(in_interrupt());
+	WARN_ON_ONCE(in_irq());
 	current_restore_flags(old_flags, PF_MEMALLOC);
 }
 
@@ -397,7 +397,7 @@ asmlinkage __visible void do_softirq(void)
 	__u32 pending;
 	unsigned long flags;
 
-	if (in_interrupt())
+	if (in_irq())
 		return;
 
 	local_irq_save(flags);
@@ -480,7 +480,7 @@ void irq_exit(void)
 #endif
 	account_irq_exit_time(current);
 	preempt_count_sub(HARDIRQ_OFFSET);
-	if (!in_interrupt() && softirq_pending_enabled())
+	if (!in_irq() && softirq_pending_enabled())
 		invoke_softirq();
 
 	tick_irq_exit();
@@ -504,7 +504,7 @@ inline void raise_softirq_irqoff(unsigned int nr)
 	 * Otherwise we wake up ksoftirqd to make sure we
 	 * schedule the softirq soon.
 	 */
-	if (!in_interrupt())
+	if (!in_irq())
 		wakeup_softirqd();
 }
 
-- 
2.17.1

[PATCH 31/32] locking: Introduce spin_[un]lock_bh_mask()

[Frederic Weisbecker]

It allows us to extend the coverage of vector finegrained masking
throughout softirq safe locking. This is especially interesting with
networking that makes extensive use of it.

It works the same way as local_bh_disable_mask():

    bh = spin_lock_bh_mask(lock, BIT(NET_RX_SOFTIRQ));
    [...]
    spin_unlock_bh_mask(lock, bh);

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 include/linux/spinlock.h         | 14 ++++++++++++++
 include/linux/spinlock_api_smp.h | 26 ++++++++++++++++++++++++++
 include/linux/spinlock_api_up.h  | 13 +++++++++++++
 kernel/locking/spinlock.c        | 19 +++++++++++++++++++
 4 files changed, 72 insertions(+)

diff --git a/include/linux/spinlock.h b/include/linux/spinlock.h
index e089157dcf97..57dd73ed202d 100644
--- a/include/linux/spinlock.h
+++ b/include/linux/spinlock.h
@@ -270,6 +270,7 @@ static inline void do_raw_spin_unlock(raw_spinlock_t *lock) __releases(lock)
 
 #define raw_spin_lock_irq(lock)		_raw_spin_lock_irq(lock)
 #define raw_spin_lock_bh(lock)		_raw_spin_lock_bh(lock)
+#define raw_spin_lock_bh_mask(lock, mask)	_raw_spin_lock_bh_mask(lock, mask)
 #define raw_spin_unlock(lock)		_raw_spin_unlock(lock)
 #define raw_spin_unlock_irq(lock)	_raw_spin_unlock_irq(lock)
 
@@ -279,6 +280,7 @@ static inline void do_raw_spin_unlock(raw_spinlock_t *lock) __releases(lock)
 		_raw_spin_unlock_irqrestore(lock, flags);	\
 	} while (0)
 #define raw_spin_unlock_bh(lock)	_raw_spin_unlock_bh(lock)
+#define raw_spin_unlock_bh_mask(lock, mask)	_raw_spin_unlock_bh_mask(lock, mask)
 
 #define raw_spin_trylock_bh(lock) \
 	__cond_lock(lock, _raw_spin_trylock_bh(lock))
@@ -334,6 +336,12 @@ static __always_inline void spin_lock_bh(spinlock_t *lock)
 	raw_spin_lock_bh(&lock->rlock);
 }
 
+static __always_inline unsigned int spin_lock_bh_mask(spinlock_t *lock,
+						      unsigned int mask)
+{
+	return raw_spin_lock_bh_mask(&lock->rlock, mask);
+}
+
 static __always_inline int spin_trylock(spinlock_t *lock)
 {
 	return raw_spin_trylock(&lock->rlock);
@@ -374,6 +382,12 @@ static __always_inline void spin_unlock_bh(spinlock_t *lock)
 	raw_spin_unlock_bh(&lock->rlock);
 }
 
+static __always_inline void spin_unlock_bh_mask(spinlock_t *lock,
+						unsigned int mask)
+{
+	raw_spin_unlock_bh_mask(&lock->rlock, mask);
+}
+
 static __always_inline void spin_unlock_irq(spinlock_t *lock)
 {
 	raw_spin_unlock_irq(&lock->rlock);
diff --git a/include/linux/spinlock_api_smp.h b/include/linux/spinlock_api_smp.h
index 42dfab89e740..987ecc1e3bc3 100644
--- a/include/linux/spinlock_api_smp.h
+++ b/include/linux/spinlock_api_smp.h
@@ -26,6 +26,8 @@ void __lockfunc
 _raw_spin_lock_nest_lock(raw_spinlock_t *lock, struct lockdep_map *map)
 								__acquires(lock);
 void __lockfunc _raw_spin_lock_bh(raw_spinlock_t *lock)		__acquires(lock);
+unsigned int __lockfunc
+_raw_spin_lock_bh_mask(raw_spinlock_t *lock, unsigned int mask)	__acquires(lock);
 void __lockfunc _raw_spin_lock_irq(raw_spinlock_t *lock)
 								__acquires(lock);
 
@@ -38,6 +40,9 @@ int __lockfunc _raw_spin_trylock(raw_spinlock_t *lock);
 int __lockfunc _raw_spin_trylock_bh(raw_spinlock_t *lock);
 void __lockfunc _raw_spin_unlock(raw_spinlock_t *lock)		__releases(lock);
 void __lockfunc _raw_spin_unlock_bh(raw_spinlock_t *lock)	__releases(lock);
+void __lockfunc
+_raw_spin_unlock_bh_mask(raw_spinlock_t *lock, unsigned int mask)
+								__releases(lock);
 void __lockfunc _raw_spin_unlock_irq(raw_spinlock_t *lock)	__releases(lock);
 void __lockfunc
 _raw_spin_unlock_irqrestore(raw_spinlock_t *lock, unsigned long flags)
@@ -136,6 +141,19 @@ static inline void __raw_spin_lock_bh(raw_spinlock_t *lock)
 	LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
 }
 
+static inline unsigned int __raw_spin_lock_bh_mask(raw_spinlock_t *lock,
+						   unsigned int mask)
+{
+	unsigned int old_mask;
+
+	old_mask = local_bh_disable_mask(_RET_IP_, SOFTIRQ_LOCK_OFFSET, mask);
+	spin_acquire(&lock->dep_map, 0, 0, _RET_IP_);
+	LOCK_CONTENDED(lock, do_raw_spin_trylock, do_raw_spin_lock);
+
+	return old_mask;
+}
+
+
 static inline void __raw_spin_lock(raw_spinlock_t *lock)
 {
 	preempt_disable();
@@ -176,6 +194,14 @@ static inline void __raw_spin_unlock_bh(raw_spinlock_t *lock)
 	__local_bh_enable_ip(_RET_IP_, SOFTIRQ_LOCK_OFFSET);
 }
 
+static inline void __raw_spin_unlock_bh_mask(raw_spinlock_t *lock,
+					     unsigned int mask)
+{
+	spin_release(&lock->dep_map, 1, _RET_IP_);
+	do_raw_spin_unlock(lock);
+	local_bh_enable_mask(_RET_IP_, SOFTIRQ_LOCK_OFFSET, mask);
+}
+
 static inline int __raw_spin_trylock_bh(raw_spinlock_t *lock)
 {
 	__local_bh_disable_ip(_RET_IP_, SOFTIRQ_LOCK_OFFSET);
diff --git a/include/linux/spinlock_api_up.h b/include/linux/spinlock_api_up.h
index d0d188861ad6..3bfb7cbbee4e 100644
--- a/include/linux/spinlock_api_up.h
+++ b/include/linux/spinlock_api_up.h
@@ -33,6 +33,13 @@
 #define __LOCK_BH(lock) \
   do { __local_bh_disable_ip(_THIS_IP_, SOFTIRQ_LOCK_OFFSET); ___LOCK(lock); } while (0)
 
+#define __LOCK_BH_MASK(lock, mask) ({							\
+	unsigned int ____old_mask;							\
+	____old_mask = local_bh_disable_mask(_THIS_IP_, SOFTIRQ_LOCK_OFFSET, mask);	\
+	___LOCK(lock);									\
+	____old_mask;
+})
+
 #define __LOCK_IRQ(lock) \
   do { local_irq_disable(); __LOCK(lock); } while (0)
 
@@ -49,6 +56,10 @@
   do { __local_bh_enable_ip(_THIS_IP_, SOFTIRQ_LOCK_OFFSET); \
        ___UNLOCK(lock); } while (0)
 
+#define __UNLOCK_BH_MASK(lock, mask)				   \
+  do { local_bh_enable_mask(_THIS_IP_, SOFTIRQ_LOCK_OFFSET, mask); \
+       ___UNLOCK(lock); } while (0)
+
 #define __UNLOCK_IRQ(lock) \
   do { local_irq_enable(); __UNLOCK(lock); } while (0)
 
@@ -60,6 +71,7 @@
 #define _raw_read_lock(lock)			__LOCK(lock)
 #define _raw_write_lock(lock)			__LOCK(lock)
 #define _raw_spin_lock_bh(lock)			__LOCK_BH(lock)
+#define _raw_spin_lock_bh_mask(lock, mask)	__LOCK_BH_MASK(lock, mask)
 #define _raw_read_lock_bh(lock)			__LOCK_BH(lock)
 #define _raw_write_lock_bh(lock)		__LOCK_BH(lock)
 #define _raw_spin_lock_irq(lock)		__LOCK_IRQ(lock)
@@ -76,6 +88,7 @@
 #define _raw_read_unlock(lock)			__UNLOCK(lock)
 #define _raw_write_unlock(lock)			__UNLOCK(lock)
 #define _raw_spin_unlock_bh(lock)		__UNLOCK_BH(lock)
+#define _raw_spin_unlock_bh_mask(lock, mask)	__UNLOCK_BH_MASK(lock, mask)
 #define _raw_write_unlock_bh(lock)		__UNLOCK_BH(lock)
 #define _raw_read_unlock_bh(lock)		__UNLOCK_BH(lock)
 #define _raw_spin_unlock_irq(lock)		__UNLOCK_IRQ(lock)
diff --git a/kernel/locking/spinlock.c b/kernel/locking/spinlock.c
index 936f3d14dd6b..4245cb3cda5a 100644
--- a/kernel/locking/spinlock.c
+++ b/kernel/locking/spinlock.c
@@ -170,6 +170,16 @@ void __lockfunc _raw_spin_lock_bh(raw_spinlock_t *lock)
 EXPORT_SYMBOL(_raw_spin_lock_bh);
 #endif
 
+#ifndef CONFIG_INLINE_SPIN_LOCK_BH
+unsigned int __lockfunc _raw_spin_lock_bh_mask(raw_spinlock_t *lock,
+					       unsigned int mask)
+{
+	return __raw_spin_lock_bh_mask(lock, mask);
+}
+EXPORT_SYMBOL(_raw_spin_lock_bh_mask);
+#endif
+
+
 #ifdef CONFIG_UNINLINE_SPIN_UNLOCK
 void __lockfunc _raw_spin_unlock(raw_spinlock_t *lock)
 {
@@ -202,6 +212,15 @@ void __lockfunc _raw_spin_unlock_bh(raw_spinlock_t *lock)
 EXPORT_SYMBOL(_raw_spin_unlock_bh);
 #endif
 
+#ifndef CONFIG_INLINE_SPIN_UNLOCK_BH
+void __lockfunc _raw_spin_unlock_bh_mask(raw_spinlock_t *lock,
+					 unsigned int mask)
+{
+	__raw_spin_unlock_bh_mask(lock, mask);
+}
+EXPORT_SYMBOL(_raw_spin_unlock_bh_mask);
+#endif
+
 #ifndef CONFIG_INLINE_READ_TRYLOCK
 int __lockfunc _raw_read_trylock(rwlock_t *lock)
 {
-- 
2.17.1

[PATCH 32/32] net: Make softirq vector masking finegrained on release_sock()

[Frederic Weisbecker]

This is a usage example of spin_lock_bh_mask(). NET_RX, TIMER and
TASKLET are the three softirqs that have been reported by lockdep to
be used for this socket lock, at least on my own usecases.

Signed-off-by: Frederic Weisbecker <frederic@kernel.org>
Cc: Mauro Carvalho Chehab <mchehab@s-opensource.com>
Cc: Joel Fernandes <joel@joelfernandes.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Pavan Kondeti <pkondeti@codeaurora.org>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: David S . Miller <davem@davemloft.net>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
---
 net/core/sock.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/core/sock.c b/net/core/sock.c
index 6aa2e7e0b4fb..24ca1cdacfe3 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2795,7 +2795,9 @@ EXPORT_SYMBOL(lock_sock_nested);
 
 void release_sock(struct sock *sk)
 {
-	spin_lock_bh(&sk->sk_lock.slock);
+	unsigned int bh;
+
+	bh = spin_lock_bh_mask(&sk->sk_lock.slock, BIT(NET_RX_SOFTIRQ) | BIT(TIMER_SOFTIRQ) | BIT(TASKLET_SOFTIRQ));
 	if (sk->sk_backlog.tail)
 		__release_sock(sk);
 
@@ -2808,7 +2810,7 @@ void release_sock(struct sock *sk)
 	sock_release_ownership(sk);
 	if (waitqueue_active(&sk->sk_lock.wq))
 		wake_up(&sk->sk_lock.wq);
-	spin_unlock_bh(&sk->sk_lock.slock);
+	spin_unlock_bh_mask(&sk->sk_lock.slock, bh);
 }
 EXPORT_SYMBOL(release_sock);
 
-- 
2.17.1

Re: [PATCH 00/32] softirq: Per vector masking v2

[David Miller]

From: Frederic Weisbecker <frederic@kernel.org>
Date: Tue, 12 Feb 2019 18:13:51 +0100

> For those who missed the infinitely invasive and carpal tunnel
> unfriendly v1: https://lwn.net/Articles/768157/
> 
> Softirqs masking is an all or nothing operation. It's currently not
> possible to disable a single vector. Yet some workloads are interested
> in deterministic latencies for vectors execution. Reducing their
> interdependencies is a first step toward that.
> 
> Unlike the previous take, the current APIs aren't changed, as advised
> by reviewers. But new APIs have been introduced. An individual vector
> can be disabled and that behaviour can self-nest and nest with existing
> APIs:
...

I really like this stuff, nice work:

Reviewed-by: David S. Miller <davem@davemloft.net>