Skip to contentSkip to conversation

Using lockdep instead of spin_is_locked()

PATCH Applied Lance Roy 16 participants 37 messages Oct 3, 2018 – Nov 2, 2018

Subsystems (20)
lore
Share & tools
lore

Lance Roy

author
#0 lore raw

One of the main uses of spin_is_locked() is to require that a lock is held when a function is called, for debugging, but lockdep_assert_held() is better for this purpose since it won't make a mistake when someone else is holding the lock. This patch series replaces all of this kind of use of spin_is_locked() with calls to lockdep_assert_held(). An ulterior motive is to reduce the number of uses of spin_is_locked() from the kernel, to work towards possibly eliminating it.

Thanks,
Lance

 arch/x86/pci/i386.c                          |  2 +-
 drivers/hv/hv_balloon.c                      |  2 +-
 drivers/misc/sgi-xp/xpc_channel.c            |  6 +++---
 drivers/misc/sgi-xp/xpc_sn2.c                |  2 +-
 drivers/misc/sgi-xp/xpc_uv.c                 |  2 +-
 drivers/net/ethernet/intel/i40e/i40e_main.c  |  3 +--
 drivers/net/ethernet/intel/igbvf/mbx.c       |  4 ++--
 drivers/net/ethernet/sfc/efx.c               |  2 +-
 drivers/net/ethernet/smsc/smsc911x.h         |  2 +-
 drivers/net/wireless/zydas/zd1211rw/zd_mac.c |  2 +-
 drivers/scsi/snic/snic_scsi.c                |  4 ++--
 fs/userfaultfd.c                             |  2 +-
 kernel/futex.c                               |  4 ++--
 kernel/locking/mutex-debug.c                 |  4 ++--
 mm/khugepaged.c                              |  4 ++--
 mm/swap.c                                    |  3 +--
 net/netfilter/ipset/ip_set_hash_gen.h        |  2 +-
 security/apparmor/file.c                     |  2 +-
 virt/kvm/arm/vgic/vgic.c                     | 12 ++++++------
 19 files changed, 31 insertions(+), 33 deletions(-)
author
#1 lore raw

PATCH 1/16 x86/PCI: Replace spin_is_locked() with lockdep

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Bjorn Helgaas <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
---
 arch/x86/pci/i386.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/pci/i386.c +1 -1
index ed4ac215305d..24bb58a007de 100644
@@ -59,7 +59,7 @@ static struct pcibios_fwaddrmap *pcibios_fwaddrmap_lookup(struct pci_dev *dev)
 {
 	struct pcibios_fwaddrmap *map;
 
-	WARN_ON_SMP(!spin_is_locked(&pcibios_fwaddrmap_lock));
+	lockdep_assert_held(&pcibios_fwaddrmap_lock);
 
 	list_for_each_entry(map, &pcibios_fwaddrmappings, list)
 		if (map->dev == dev)
Signature · 1 line
2.19.0
#2 lore raw

Re: [PATCH 01/16] x86/PCI: Replace spin_is_locked() with lockdep

Quoted 4 lines · Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Quoted 8 lines
Cc Bjorn Helgaas <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc "H. Peter Anvin" <[email protected]>

I assume you plan to merge the whole series together. I don't object to that, but I don't know enough to be able to formally ack this.

It would be useful to include a tiny bit more detail in the changelog. The spin_is_locked() documentation doesn't mention anything about differences with respect to the lock being held by self vs by someone else, so I can't tell where the confusion arises.

Bjorn

Quoted 20 lines
---
 arch/x86/pci/i386.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/pci/i386.c +1 -1
index ed4ac215305d..24bb58a007de 100644
@@ -59,7 +59,7 @@ static struct pcibios_fwaddrmap *pcibios_fwaddrmap_lookup(struct pci_dev *dev)
 {
 	struct pcibios_fwaddrmap *map;
 
-	WARN_ON_SMP(!spin_is_locked(&pcibios_fwaddrmap_lock));
+	lockdep_assert_held(&pcibios_fwaddrmap_lock);
 
 	list_for_each_entry(map, &pcibios_fwaddrmappings, list)
 		if (map->dev == dev)
Signature · 1 line
2.19.0
author
#3 lore raw

Re: [PATCH 01/16] x86/PCI: Replace spin_is_locked() with lockdep

Quoted 14 lines · Bjorn Helgaas
Quoted 4 lines · Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Quoted 8 lines
Cc Bjorn Helgaas <[email protected]>
Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Borislav Petkov <[email protected]>
Cc "H. Peter Anvin" <[email protected]>
Quoted 7 lines

I assume you plan to merge the whole series together. I don't object to that, but I don't know enough to be able to formally ack this.

It would be useful to include a tiny bit more detail in the changelog. The spin_is_locked() documentation doesn't mention anything about differences with respect to the lock being held by self vs by someone else, so I can't tell where the confusion arises.

The difference is that spin_is_locked() will return true when someone else holds the lock, while lockdep_assert_held() asserts that the current thread holds the lock. How about the following for an new changelog?

lockdep_assert_held() is better suited to checking locking requirements, since it only checks if the current thread holds the lock regardless of whether someone else does. This is also a step towards possibly removing spin_is_locked().

Thanks,
Lance

Quoted 22 lines

Bjorn

Quoted 20 lines
---
 arch/x86/pci/i386.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/pci/i386.c +1 -2
index ed4ac215305d..24bb58a007de 100644
@@ -59,7 +59,7 @@ static struct pcibios_fwaddrmap *pcibios_fwaddrmap_lookup(struct pci_dev *dev)
 {
 	struct pcibios_fwaddrmap *map;
 
-	WARN_ON_SMP(!spin_is_locked(&pcibios_fwaddrmap_lock));
+	lockdep_assert_held(&pcibios_fwaddrmap_lock);
 
 	list_for_each_entry(map, &pcibios_fwaddrmappings, list)
 		if (map->dev == dev)
--

2.19.0

author
#4 lore raw

PATCH 2/16 hv_balloon: Replace spin_is_locked() with lockdep

In reply to Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc "K. Y. Srinivasan" <[email protected]>
Cc Haiyang Zhang <[email protected]>
Cc Stephen Hemminger <[email protected]>
---
 drivers/hv/hv_balloon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/hv/hv_balloon.c +1 -1
index b1b788082793..41631512ae97 100644
@@ -689,7 +689,7 @@ static void hv_page_online_one(struct hv_hotadd_state *has, struct page *pg)
 	__online_page_increment_counters(pg);
 	__online_page_free(pg);
 
-	WARN_ON_ONCE(!spin_is_locked(&dm_device.ha_lock));
+	lockdep_assert_held(&dm_device.ha_lock);
 	dm_device.num_pages_onlined++;
 }
 
Signature · 1 line
2.19.0
Reviewed-by on v1 2/16
#5 lore raw

Re: [PATCH 02/16] hv_balloon: Replace spin_is_locked() with lockdep

Quoted 18 lines · Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc "K. Y. Srinivasan" <[email protected]>
Cc Haiyang Zhang <[email protected]>
Cc Stephen Hemminger <[email protected]>
---
 drivers/hv/hv_balloon.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/hv/hv_balloon.c +0 -0
index b1b788082793..41631512ae97 100644

drivers/hv/hv_balloon.c static void hv_page_online_one(struct hv_hotadd_state *has, struct page *pg) · line 694

Quoted 7 lines
drivers/hv/hv_balloon.c +1 -1
@@ -689,7 +689,7 @@ static void hv_page_online_one(struct hv_hotadd_state *has, struct page *pg)
 	__online_page_increment_counters(pg);
 	__online_page_free(pg);
 
-	WARN_ON_ONCE(!spin_is_locked(&dm_device.ha_lock));
+	lockdep_assert_held(&dm_device.ha_lock);
 	dm_device.num_pages_onlined++;
 }

However,

lockdep_assert_held() is a no-op when !CONFIG_LOCKDEP but this doesn't really matter: hv_page_online_one() is static and it has only two call sites, both taking the dm_device.ha_lock lock - so the warning may just go away.

Signature · 1 line
Vitaly
author
#6 lore raw

PATCH 3/16 sgi-xp: Replace spin_is_locked() with lockdep

In reply to Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Cliff Whickman <[email protected]>
Cc Robin Holt <[email protected]>
Cc Arnd Bergmann <[email protected]>
Cc Greg Kroah-Hartman <[email protected]>
---
 drivers/misc/sgi-xp/xpc_channel.c | 6 +++---
 drivers/misc/sgi-xp/xpc_sn2.c     | 2 +-
 drivers/misc/sgi-xp/xpc_uv.c      | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)
drivers/misc/sgi-xp/xpc_channel.c +3 -3
index 05a890ce2ab8..8e6607fc8a67 100644
@@ -28,7 +28,7 @@ xpc_process_connect(struct xpc_channel *ch, unsigned long *irq_flags)
 {
 	enum xp_retval ret;
 
-	DBUG_ON(!spin_is_locked(&ch->lock));
+	lockdep_assert_held(&ch->lock);
 
 	if (!(ch->flags & XPC_C_OPENREQUEST) ||
 	    !(ch->flags & XPC_C_ROPENREQUEST)) {
@@ -82,7 +82,7 @@ xpc_process_disconnect(struct xpc_channel *ch, unsigned long *irq_flags)
 	struct xpc_partition *part = &xpc_partitions[ch->partid];
 	u32 channel_was_connected = (ch->flags & XPC_C_WASCONNECTED);
 
-	DBUG_ON(!spin_is_locked(&ch->lock));
+	lockdep_assert_held(&ch->lock);
 
 	if (!(ch->flags & XPC_C_DISCONNECTING))
 		return;
@@ -755,7 +755,7 @@ xpc_disconnect_channel(const int line, struct xpc_channel *ch,
 {
 	u32 channel_was_connected = (ch->flags & XPC_C_CONNECTED);
 
-	DBUG_ON(!spin_is_locked(&ch->lock));
+	lockdep_assert_held(&ch->lock);
 
 	if (ch->flags & (XPC_C_DISCONNECTING | XPC_C_DISCONNECTED))
 		return;
drivers/misc/sgi-xp/xpc_sn2.c +1 -1
index 5a12d2a54049..0ae69b9390ce 100644
@@ -1671,7 +1671,7 @@ xpc_teardown_msg_structures_sn2(struct xpc_channel *ch)
 {
 	struct xpc_channel_sn2 *ch_sn2 = &ch->sn.sn2;
 
-	DBUG_ON(!spin_is_locked(&ch->lock));
+	lockdep_assert_held(&ch->lock);
 
 	ch_sn2->remote_msgqueue_pa = 0;
 
drivers/misc/sgi-xp/xpc_uv.c +1 -1
index 340b44d9e8cf..0441abe87880 100644
@@ -1183,7 +1183,7 @@ xpc_teardown_msg_structures_uv(struct xpc_channel *ch)
 {
 	struct xpc_channel_uv *ch_uv = &ch->sn.uv;
 
-	DBUG_ON(!spin_is_locked(&ch->lock));
+	lockdep_assert_held(&ch->lock);
 
 	kfree(ch_uv->cached_notify_gru_mq_desc);
 	ch_uv->cached_notify_gru_mq_desc = NULL;
Signature · 1 line
2.19.0
author
#7 lore raw

PATCH 4/16 i40e: Replace spin_is_locked() with lockdep

In reply to Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Jeff Kirsher <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc intel-wired-lan at lists.osuosl.org
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
drivers/net/ethernet/intel/i40e/i40e_main.c +1 -2
index ac685ad4d877..8ce3471723d3 100644
@@ -1489,8 +1489,7 @@ int i40e_del_mac_filter(struct i40e_vsi *vsi, const u8 *macaddr)
 	bool found = false;
 	int bkt;
 
-	WARN(!spin_is_locked(&vsi->mac_filter_hash_lock),
-	     "Missing mac_filter_hash_lock\n");
+	lockdep_assert_held(&vsi->mac_filter_hash_lock);
 	hash_for_each_safe(vsi->mac_filter_hash, bkt, h, f, hlist) {
 		if (ether_addr_equal(macaddr, f->macaddr)) {
 			__i40e_del_filter(vsi, f);
Signature · 1 line
2.19.0
#8 lore raw

PATCH 4/16 i40e: Replace spin_is_locked() with lockdep

Quoted 15 lines

-----Original Message-----
From: Intel-wired-lan [mailto:intel-wired-lan-bounces at osuosl.org] On Behalf Of Lance Roy
Sent: Tuesday, October 2, 2018 10:39 PM
To: linux-kernel at vger.kernel.org
Cc: netdev at vger.kernel.org; Lance Roy <[email protected]>; intel-wired- lan at lists.osuosl.org; Paul E. McKenney <[email protected]>; David S. Miller <[email protected]>
Subject: [Intel-wired-lan] [PATCH 04/16] i40e: Replace spin_is_locked() with lockdep

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Quoted 8 lines
Cc Jeff Kirsher <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc intel-wired-lan at lists.osuosl.org
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
Tested-by on v1 4/16
#9 lore raw

RE: [Intel-wired-lan] [PATCH 04/16] i40e: Replace spin_is_locked() with lockdep

In reply to Lance Roy
Quoted 15 lines

-----Original Message-----
From: Intel-wired-lan [mailto:[email protected]] On Behalf Of Lance Roy
Sent: Tuesday, October 2, 2018 10:39 PM
To: [email protected]
Cc: [email protected]; Lance Roy <[email protected]>; intel-wired- [email protected]; Paul E. McKenney <[email protected]>; David S. Miller <[email protected]>
Subject: [Intel-wired-lan] [PATCH 04/16] i40e: Replace spin_is_locked() with lockdep

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Quoted 8 lines
---
 drivers/net/ethernet/intel/i40e/i40e_main.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
author
#10 lore raw

PATCH 5/16 igbvf: Replace spin_is_locked() with lockdep

In reply to Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Jeff Kirsher <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc intel-wired-lan at lists.osuosl.org
---
 drivers/net/ethernet/intel/igbvf/mbx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/net/ethernet/intel/igbvf/mbx.c +2 -2
index 163e5838f7c2..a3cd7ac48d4b 100644
@@ -241,7 +241,7 @@ static s32 e1000_write_mbx_vf(struct e1000_hw *hw, u32 *msg, u16 size)
 	s32 err;
 	u16 i;
 
-	WARN_ON_ONCE(!spin_is_locked(&hw->mbx_lock));
+	lockdep_assert_held(&hw->mbx_lock);
 
 	/* lock the mailbox to prevent pf/vf race condition */
 	err = e1000_obtain_mbx_lock_vf(hw);
@@ -279,7 +279,7 @@ static s32 e1000_read_mbx_vf(struct e1000_hw *hw, u32 *msg, u16 size)
 	s32 err;
 	u16 i;
 
-	WARN_ON_ONCE(!spin_is_locked(&hw->mbx_lock));
+	lockdep_assert_held(&hw->mbx_lock);
 
 	/* lock the mailbox to prevent pf/vf race condition */
 	err = e1000_obtain_mbx_lock_vf(hw);
Signature · 1 line
2.19.0
Tested-by on v1 5/16
#11 lore raw

RE: [PATCH 05/16] igbvf: Replace spin_is_locked() with lockdep

Quoted 14 lines

From: netdev-owner at vger.kernel.org [mailto:netdev-
owner at vger.kernel.org] On Behalf Of Lance Roy
Sent: Tuesday, October 2, 2018 10:39 PM
To: linux-kernel at vger.kernel.org
Cc: Paul E. McKenney <[email protected]>; Lance Roy <[email protected]>; Kirsher, Jeffrey T <[email protected]>; David S. Miller <[email protected]>; intel-wired-lan at lists.osuosl.org; netdev at vger.kernel.org
Subject: [PATCH 05/16] igbvf: Replace spin_is_locked() with lockdep

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Quoted 9 lines
Cc Jeff Kirsher <[email protected]>
Cc "David S. Miller" <[email protected]>
Cc intel-wired-lan at lists.osuosl.org
---
 drivers/net/ethernet/intel/igbvf/mbx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
author
#12 lore raw

PATCH 6/16 sfc: Replace spin_is_locked() with lockdep

In reply to Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Solarflare linux maintainers <[email protected]>
Cc Edward Cree <[email protected]>
Cc Bert Kenward <[email protected]>
Cc "David S. Miller" <[email protected]>
---
 drivers/net/ethernet/sfc/efx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/sfc/efx.c +1 -1
index 330233286e78..f40c3d5ca413 100644
@@ -3193,7 +3193,7 @@ struct hlist_head *efx_rps_hash_bucket(struct efx_nic *efx,
 {
 	u32 hash = efx_filter_spec_hash(spec);
 
-	WARN_ON(!spin_is_locked(&efx->rps_hash_lock));
+	lockdep_assert_held(&efx->rps_hash_lock);
 	if (!efx->rps_hash_table)
 		return NULL;
 	return &efx->rps_hash_table[hash % EFX_ARFS_HASH_TABLE_SIZE];
Signature · 1 line
2.19.0
author
#13 lore raw

PATCH 7/16 smsc: Replace spin_is_locked() with lockdep

In reply to Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Steve Glendinning <[email protected]>
Cc "David S. Miller" <[email protected]>
---
 drivers/net/ethernet/smsc/smsc911x.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/ethernet/smsc/smsc911x.h +1 -1
index 8d75508acd2b..51b2fc1a395f 100644
@@ -67,7 +67,7 @@
 
 #ifdef CONFIG_DEBUG_SPINLOCK
 #define SMSC_ASSERT_MAC_LOCK(pdata) \
-		WARN_ON_SMP(!spin_is_locked(&pdata->mac_lock))
+		lockdep_assert_held(&pdata->mac_lock)
 #else
 #define SMSC_ASSERT_MAC_LOCK(pdata) do {} while (0)
 #endif				/* CONFIG_DEBUG_SPINLOCK */
Signature · 1 line
2.19.0
author
#14 lore raw

PATCH 8/16 wireless: Replace spin_is_locked() with lockdep

In reply to Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Daniel Drake <[email protected]>
Cc Ulrich Kunitz <[email protected]>
Cc Kalle Valo <[email protected]>
Cc "David S. Miller" <[email protected]>
---
 drivers/net/wireless/zydas/zd1211rw/zd_mac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/wireless/zydas/zd1211rw/zd_mac.c +1 -1
index 1f6d9f357e57..9ccd780695f0 100644
@@ -235,7 +235,7 @@ void zd_mac_clear(struct zd_mac *mac)
 {
 	flush_workqueue(zd_workqueue);
 	zd_chip_clear(&mac->chip);
-	ZD_ASSERT(!spin_is_locked(&mac->lock));
+	lockdep_assert_held(&mac->lock);
 	ZD_MEMCLEAR(mac, sizeof(struct zd_mac));
 }
 
Signature · 1 line
2.19.0
Acked-by on v1 8/16
#15 lore raw

Re: [PATCH 08/16] wireless: Replace spin_is_locked() with lockdep

Quoted 11 lines · Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Daniel Drake <[email protected]>
Cc Ulrich Kunitz <[email protected]>
Cc Kalle Valo <[email protected]>
Cc "David S. Miller" <[email protected]>
Quoted 3 lines
---
 drivers/net/wireless/zydas/zd1211rw/zd_mac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Should I take this or is it going through some other tree?

If it goes to via some other tree:

Signature · 1 line
Kalle Valo
author
#16 lore raw

Re: [PATCH 08/16] wireless: Replace spin_is_locked() with lockdep

Quoted 17 lines · Kalle Valo
Quoted 11 lines · Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Daniel Drake <[email protected]>
Cc Ulrich Kunitz <[email protected]>
Cc Kalle Valo <[email protected]>
Cc "David S. Miller" <[email protected]>
Quoted 3 lines
---
 drivers/net/wireless/zydas/zd1211rw/zd_mac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 1 line

Should I take this or is it going through some other tree?

Sure you can take this if you like. If not, Paul McKenney will take it in his tree.

Thanks,
Lance

Quoted 6 lines

If it goes to via some other tree:

--
Kalle Valo

#17 lore raw

Re: [PATCH 08/16] wireless: Replace spin_is_locked() with lockdep

Quoted 18 lines · Lance Roy
Quoted 17 lines · Kalle Valo
Quoted 14 lines · Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Daniel Drake <[email protected]>
Cc Ulrich Kunitz <[email protected]>
Cc Kalle Valo <[email protected]>
Cc "David S. Miller" <[email protected]>
---
 drivers/net/wireless/zydas/zd1211rw/zd_mac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Quoted 4 lines
Quoted 1 line

Should I take this or is it going through some other tree?

Sure you can take this if you like. If not, Paul McKenney will take it in his tree.

Ok, then I'll take this to wireless-drivers-next. And I'll change the title prefix to "zd1211rw: ".

Signature · 1 line
Kalle Valo
#18 lore raw

Re: [PATCH 08/16] zd1211rw: Replace spin_is_locked() with lockdep

In reply to Lance Roy
Quoted 11 lines · Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Daniel Drake <[email protected]>
Cc Ulrich Kunitz <[email protected]>
Cc Kalle Valo <[email protected]>
Cc "David S. Miller" <[email protected]>

Patch applied to wireless-drivers-next.git, thanks.

209e957b467b zd1211rw: Replace spin_is_locked() with lockdep

Signature · 3 lines
https://patchwork.kernel.org/patch/10624325/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
author
#19 lore raw

PATCH 9/16 scsi: Replace spin_is_locked() with lockdep

In reply to Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Karan Tilak Kumar <[email protected]>
Cc Sesidhar Baddela <[email protected]>
Cc "James E.J. Bottomley" <[email protected]>
Cc "Martin K. Petersen" <[email protected]>
---
 drivers/scsi/snic/snic_scsi.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/scsi/snic/snic_scsi.c +2 -2
index d9b2e46424aa..42e485139fc9 100644
@@ -2001,7 +2001,7 @@ snic_dr_finish(struct snic *snic, struct scsi_cmnd *sc)
 	}
 
 dr_failed:
-	SNIC_BUG_ON(!spin_is_locked(io_lock));
+	lockdep_assert_held(io_lock);
 	if (rqi)
 		CMD_SP(sc) = NULL;
 	spin_unlock_irqrestore(io_lock, flags);
@@ -2604,7 +2604,7 @@ snic_internal_abort_io(struct snic *snic, struct scsi_cmnd *sc, int tmf)
 	ret = SUCCESS;
 
 skip_internal_abts:
-	SNIC_BUG_ON(!spin_is_locked(io_lock));
+	lockdep_assert_held(io_lock);
 	spin_unlock_irqrestore(io_lock, flags);
 
 	return ret;
Signature · 1 line
2.19.0
Reviewed-by on v1 9/16
#20 lore raw

Re: [PATCH 09/16] scsi: Replace spin_is_locked() with lockdep

Quoted 3 lines · Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

#21 lore raw

Re: [PATCH 09/16] scsi: Replace spin_is_locked() with lockdep

In reply to Lance Roy

Lance,

Quoted 3 lines

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Applied to 4.20/scsi-queue, thank you!

Signature · 1 line
Martin K. Petersen	Oracle Linux Engineering
author
#22 lore raw

PATCH 10/16 userfaultfd: Replace spin_is_locked() with lockdep

In reply to Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

---
 fs/userfaultfd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/userfaultfd.c +1 -1
index bfa0ec69f924..a20244ff0027 100644
@@ -926,7 +926,7 @@ static inline struct userfaultfd_wait_queue *find_userfault_in(
 	wait_queue_entry_t *wq;
 	struct userfaultfd_wait_queue *uwq;
 
-	VM_BUG_ON(!spin_is_locked(&wqh->lock));
+	lockdep_assert_held(&wqh->lock);
 
 	uwq = NULL;
 	if (!waitqueue_active(wqh))
Signature · 1 line
2.19.0
author
#23 lore raw

PATCH 11/16 futex: Replace spin_is_locked() with lockdep

In reply to Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Thomas Gleixner <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc Darren Hart <[email protected]>
---
 kernel/futex.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/futex.c +2 -2
index 11fc3bb456d6..3e2de8fc1891 100644
@@ -1365,9 +1365,9 @@ static void __unqueue_futex(struct futex_q *q)
 {
 	struct futex_hash_bucket *hb;
 
-	if (WARN_ON_SMP(!q->lock_ptr || !spin_is_locked(q->lock_ptr))
-	    || WARN_ON(plist_node_empty(&q->list)))
+	if (WARN_ON_SMP(!q->lock_ptr) || WARN_ON(plist_node_empty(&q->list)))
 		return;
+	lockdep_assert_held(q->lock_ptr);
 
 	hb = container_of(q->lock_ptr, struct futex_hash_bucket, lock);
 	plist_del(&q->list, &hb->chain);
Signature · 1 line
2.19.0
#24 lore raw

[tip:locking/core] futex: Replace spin_is_locked() with lockdep

futex: Replace spin_is_locked() with lockdep
Commit-ID:  4de1a293a08bdf8ec1530e02163930ac86f80ea2
Gitweb:     https://git.kernel.org/tip/4de1a293a08bdf8ec1530e02163930ac86f80ea2
Author:     Lance Roy <[email protected]>
AuthorDate: Tue, 2 Oct 2018 22:38:57 -0700
Committer:  Thomas Gleixner <[email protected]>
CommitDate: Tue, 9 Oct 2018 13:19:28 +0200

futex: Replace spin_is_locked() with lockdep

lockdep_assert_held() is better suited for checking locking requirements, since it won't get confused when the lock is held by some other task. This is also a step towards possibly removing spin_is_locked().

---
 kernel/futex.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/futex.c +2 -2
index 11fc3bb456d6..3e2de8fc1891 100644
@@ -1365,9 +1365,9 @@ static void __unqueue_futex(struct futex_q *q)
 {
 	struct futex_hash_bucket *hb;
 
-	if (WARN_ON_SMP(!q->lock_ptr || !spin_is_locked(q->lock_ptr))
-	    || WARN_ON(plist_node_empty(&q->list)))
+	if (WARN_ON_SMP(!q->lock_ptr) || WARN_ON(plist_node_empty(&q->list)))
 		return;
+	lockdep_assert_held(q->lock_ptr);
 
 	hb = container_of(q->lock_ptr, struct futex_hash_bucket, lock);
 	plist_del(&q->list, &hb->chain);
author
#25 lore raw

PATCH 12/16 locking/mutex: Replace spin_is_locked() with lockdep

In reply to Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Peter Zijlstra <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Will Deacon <[email protected]>
---
 kernel/locking/mutex-debug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/locking/mutex-debug.c +2 -2
index 9aa713629387..771d4ca96dda 100644
@@ -36,7 +36,7 @@ void debug_mutex_lock_common(struct mutex *lock, struct mutex_waiter *waiter)
 
 void debug_mutex_wake_waiter(struct mutex *lock, struct mutex_waiter *waiter)
 {
-	SMP_DEBUG_LOCKS_WARN_ON(!spin_is_locked(&lock->wait_lock));
+	lockdep_assert_held(&lock->wait_lock);
 	DEBUG_LOCKS_WARN_ON(list_empty(&lock->wait_list));
 	DEBUG_LOCKS_WARN_ON(waiter->magic != waiter);
 	DEBUG_LOCKS_WARN_ON(list_empty(&waiter->list));
@@ -51,7 +51,7 @@ void debug_mutex_free_waiter(struct mutex_waiter *waiter)
 void debug_mutex_add_waiter(struct mutex *lock, struct mutex_waiter *waiter,
 			    struct task_struct *task)
 {
-	SMP_DEBUG_LOCKS_WARN_ON(!spin_is_locked(&lock->wait_lock));
+	lockdep_assert_held(&lock->wait_lock);
 
 	/* Mark the current thread as blocked on the lock: */
 	task->blocked_on = waiter;
Signature · 1 line
2.19.0
#26 lore raw

Re: [PATCH 12/16] locking/mutex: Replace spin_is_locked() with lockdep

Quoted 26 lines · Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Peter Zijlstra <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Will Deacon <[email protected]>
---
 kernel/locking/mutex-debug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/locking/mutex-debug.c +1 -1
index 9aa713629387..771d4ca96dda 100644
@@ -36,7 +36,7 @@ void debug_mutex_lock_common(struct mutex *lock, struct mutex_waiter *waiter)
 
 void debug_mutex_wake_waiter(struct mutex *lock, struct mutex_waiter *waiter)
 {
-	SMP_DEBUG_LOCKS_WARN_ON(!spin_is_locked(&lock->wait_lock));
+	lockdep_assert_held(&lock->wait_lock);
 	DEBUG_LOCKS_WARN_ON(list_empty(&lock->wait_list));
 	DEBUG_LOCKS_WARN_ON(waiter->magic != waiter);
 	DEBUG_LOCKS_WARN_ON(list_empty(&waiter->list));

kernel/locking/mutex-debug.c void debug_mutex_free_waiter(struct mutex_waiter *waiter) · line 54

Quoted 5 lines
kernel/locking/mutex-debug.c +1 -1
@@ -51,7 +51,7 @@ void debug_mutex_free_waiter(struct mutex_waiter *waiter)
 void debug_mutex_add_waiter(struct mutex *lock, struct mutex_waiter *waiter,
 			    struct task_struct *task)
 {
-	SMP_DEBUG_LOCKS_WARN_ON(!spin_is_locked(&lock->wait_lock));
+	lockdep_assert_held(&lock->wait_lock);

I think it's a good idea to replace debug usage of spin_is_locked() with calls to lockdep, but I wonder whether that means that DEBUG_MUTEXES should select LOCKDEP so that we don't lose coverage?

What do you think?

Will

#27 lore raw

Re: [PATCH 12/16] locking/mutex: Replace spin_is_locked() with lockdep

Quoted 37 lines · Will Deacon
Quoted 26 lines · Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Peter Zijlstra <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Will Deacon <[email protected]>
---
 kernel/locking/mutex-debug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
kernel/locking/mutex-debug.c +1 -1
index 9aa713629387..771d4ca96dda 100644
@@ -36,7 +36,7 @@ void debug_mutex_lock_common(struct mutex *lock, struct mutex_waiter *waiter)
 
 void debug_mutex_wake_waiter(struct mutex *lock, struct mutex_waiter *waiter)
 {
-	SMP_DEBUG_LOCKS_WARN_ON(!spin_is_locked(&lock->wait_lock));
+	lockdep_assert_held(&lock->wait_lock);
 	DEBUG_LOCKS_WARN_ON(list_empty(&lock->wait_list));
 	DEBUG_LOCKS_WARN_ON(waiter->magic != waiter);
 	DEBUG_LOCKS_WARN_ON(list_empty(&waiter->list));

kernel/locking/mutex-debug.c void debug_mutex_free_waiter(struct mutex_waiter *waiter) · line 54

Quoted 5 lines
kernel/locking/mutex-debug.c +1 -1
@@ -51,7 +51,7 @@ void debug_mutex_free_waiter(struct mutex_waiter *waiter)
 void debug_mutex_add_waiter(struct mutex *lock, struct mutex_waiter *waiter,
 			    struct task_struct *task)
 {
-	SMP_DEBUG_LOCKS_WARN_ON(!spin_is_locked(&lock->wait_lock));
+	lockdep_assert_held(&lock->wait_lock);

I think it's a good idea to replace debug usage of spin_is_locked() with calls to lockdep, but I wonder whether that means that DEBUG_MUTEXES should select LOCKDEP so that we don't lose coverage?

Quoted 1 line

What do you think?

Makes sense to me!

But given that this was accepted into -tip, I have dropped it from my tree.

Thanx, Paul

author
#28 lore raw

PATCH 13/16 mm: Replace spin_is_locked() with lockdep

In reply to Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Andrew Morton <[email protected]>
Cc "Kirill A. Shutemov" <[email protected]>
Cc Yang Shi <[email protected]>
Cc Matthew Wilcox <[email protected]>
Cc Mel Gorman <[email protected]>
Cc Vlastimil Babka <[email protected]>
Cc Jan Kara <[email protected]>
Cc Shakeel Butt <[email protected]>
---
 mm/khugepaged.c | 4 ++--
 mm/swap.c       | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)
mm/khugepaged.c +2 -2
index a31d740e6cd1..80f12467ccb3 100644
@@ -1225,7 +1225,7 @@ static void collect_mm_slot(struct mm_slot *mm_slot)
 {
 	struct mm_struct *mm = mm_slot->mm;
 
-	VM_BUG_ON(NR_CPUS != 1 && !spin_is_locked(&khugepaged_mm_lock));
+	lockdep_assert_held(&khugepaged_mm_lock);
 
 	if (khugepaged_test_exit(mm)) {
 		/* free mm_slot */
@@ -1665,7 +1665,7 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages,
 	int progress = 0;
 
 	VM_BUG_ON(!pages);
-	VM_BUG_ON(NR_CPUS != 1 && !spin_is_locked(&khugepaged_mm_lock));
+	lockdep_assert_held(&khugepaged_mm_lock);
 
 	if (khugepaged_scan.mm_slot)
 		mm_slot = khugepaged_scan.mm_slot;
mm/swap.c +1 -2
index 26fc9b5f1b6c..c89eb442c0bf 100644
@@ -824,8 +824,7 @@ void lru_add_page_tail(struct page *page, struct page *page_tail,
 	VM_BUG_ON_PAGE(!PageHead(page), page);
 	VM_BUG_ON_PAGE(PageCompound(page_tail), page);
 	VM_BUG_ON_PAGE(PageLRU(page_tail), page);
-	VM_BUG_ON(NR_CPUS != 1 &&
-		  !spin_is_locked(&lruvec_pgdat(lruvec)->lru_lock));
+	lockdep_assert_held(&lruvec_pgdat(lruvec)->lru_lock);
 
 	if (!list)
 		SetPageLRU(page_tail);
Signature · 1 line
2.19.0
Acked-by on v1 13/16
#29 lore raw

Re: [PATCH 13/16] mm: Replace spin_is_locked() with lockdep

Quoted 3 lines · Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Agreed

Quoted 10 lines
Cc Andrew Morton <[email protected]>
Cc "Kirill A. Shutemov" <[email protected]>
Cc Yang Shi <[email protected]>
Cc Matthew Wilcox <[email protected]>
Cc Mel Gorman <[email protected]>
Cc Vlastimil Babka <[email protected]>
Cc Jan Kara <[email protected]>
Cc Shakeel Butt <[email protected]>
Quoted 42 lines
---
 mm/khugepaged.c | 4 ++--
 mm/swap.c       | 3 +--
 2 files changed, 3 insertions(+), 4 deletions(-)
mm/khugepaged.c +2 -2
index a31d740e6cd1..80f12467ccb3 100644
@@ -1225,7 +1225,7 @@ static void collect_mm_slot(struct mm_slot *mm_slot)
 {
 	struct mm_struct *mm = mm_slot->mm;
 
-	VM_BUG_ON(NR_CPUS != 1 && !spin_is_locked(&khugepaged_mm_lock));
+	lockdep_assert_held(&khugepaged_mm_lock);
 
 	if (khugepaged_test_exit(mm)) {
 		/* free mm_slot */
@@ -1665,7 +1665,7 @@ static unsigned int khugepaged_scan_mm_slot(unsigned int pages,
 	int progress = 0;
 
 	VM_BUG_ON(!pages);
-	VM_BUG_ON(NR_CPUS != 1 && !spin_is_locked(&khugepaged_mm_lock));
+	lockdep_assert_held(&khugepaged_mm_lock);
 
 	if (khugepaged_scan.mm_slot)
 		mm_slot = khugepaged_scan.mm_slot;
mm/swap.c +1 -2
index 26fc9b5f1b6c..c89eb442c0bf 100644
@@ -824,8 +824,7 @@ void lru_add_page_tail(struct page *page, struct page *page_tail,
 	VM_BUG_ON_PAGE(!PageHead(page), page);
 	VM_BUG_ON_PAGE(PageCompound(page_tail), page);
 	VM_BUG_ON_PAGE(PageLRU(page_tail), page);
-	VM_BUG_ON(NR_CPUS != 1 &&
-		  !spin_is_locked(&lruvec_pgdat(lruvec)->lru_lock));
+	lockdep_assert_held(&lruvec_pgdat(lruvec)->lru_lock);
 
 	if (!list)
 		SetPageLRU(page_tail);
author
#30 lore raw

PATCH 14/16 netfilter: Replace spin_is_locked() with lockdep

In reply to Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Pablo Neira Ayuso <[email protected]>
Cc Jozsef Kadlecsik <[email protected]>
Cc Florian Westphal <[email protected]>
Cc "David S. Miller" <[email protected]>
---
 net/netfilter/ipset/ip_set_hash_gen.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
net/netfilter/ipset/ip_set_hash_gen.h +1 -1
index 8a33dac4e805..e287da68d5fa 100644
@@ -15,7 +15,7 @@
 
 #define __ipset_dereference_protected(p, c)	rcu_dereference_protected(p, c)
 #define ipset_dereference_protected(p, set) \
-	__ipset_dereference_protected(p, spin_is_locked(&(set)->lock))
+	__ipset_dereference_protected(p, lockdep_is_held(&(set)->lock))
 
 #define rcu_dereference_bh_nfnl(p)	rcu_dereference_bh_check(p, 1)
 
Signature · 1 line
2.19.0
Acked-by on v1 14/16
#31 lore raw

Re: [PATCH 14/16] netfilter: Replace spin_is_locked() with lockdep

Quoted 12 lines · Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc Pablo Neira Ayuso <[email protected]>
Cc Jozsef Kadlecsik <[email protected]>
Cc Florian Westphal <[email protected]>
Cc "David S. Miller" <[email protected]>
Quoted 3 lines
---
 net/netfilter/ipset/ip_set_hash_gen.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Best regards,
Jozsef

Quoted 17 lines
net/netfilter/ipset/ip_set_hash_gen.h +1 -1
index 8a33dac4e805..e287da68d5fa 100644
@@ -15,7 +15,7 @@
 
 #define __ipset_dereference_protected(p, c)	rcu_dereference_protected(p, c)
 #define ipset_dereference_protected(p, set) \
-	__ipset_dereference_protected(p, spin_is_locked(&(set)->lock))
+	__ipset_dereference_protected(p, lockdep_is_held(&(set)->lock))
 
 #define rcu_dereference_bh_nfnl(p)	rcu_dereference_bh_check(p, 1)
 
Signature · 1 line
2.19.0

-
E-mail : [email protected], [email protected] PGP key : http://www.kfki.hu/~kadlec/pgp_public_key.txt
Address : Wigner Research Centre for Physics, Hungarian Academy of Sciences

          H-1525 Budapest 114, POB. 49, Hungary
author
#32 lore raw

PATCH 15/16 apparmor: Replace spin_is_locked() with lockdep

In reply to Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc John Johansen <[email protected]>
Cc James Morris <[email protected]>
Cc "Serge E. Hallyn" <[email protected]>
---
 security/apparmor/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
security/apparmor/file.c +1 -1
index 4285943f7260..d0afed9ebd0e 100644
@@ -496,7 +496,7 @@ static void update_file_ctx(struct aa_file_ctx *fctx, struct aa_label *label,
 	/* update caching of label on file_ctx */
 	spin_lock(&fctx->lock);
 	old = rcu_dereference_protected(fctx->label,
-					spin_is_locked(&fctx->lock));
+					lockdep_is_held(&fctx->lock));
 	l = aa_label_merge(old, label, GFP_ATOMIC);
 	if (l) {
 		if (l != old) {
Signature · 1 line
2.19.0
maintainer Acked-by on v1 15/16
#33 lore raw

Re: [PATCH 15/16] apparmor: Replace spin_is_locked() with lockdep

Quoted 9 lines · Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Cc John Johansen <[email protected]>
Cc James Morris <[email protected]>
Cc "Serge E. Hallyn" <[email protected]>
Quoted 18 lines
---
 security/apparmor/file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
security/apparmor/file.c +1 -1
index 4285943f7260..d0afed9ebd0e 100644
@@ -496,7 +496,7 @@ static void update_file_ctx(struct aa_file_ctx *fctx, struct aa_label *label,
 	/* update caching of label on file_ctx */
 	spin_lock(&fctx->lock);
 	old = rcu_dereference_protected(fctx->label,
-					spin_is_locked(&fctx->lock));
+					lockdep_is_held(&fctx->lock));
 	l = aa_label_merge(old, label, GFP_ATOMIC);
 	if (l) {
 		if (l != old) {
author
#34 lore raw

PATCH 16/16 KVM: arm/arm64: vgic: Replace spin_is_locked() with lockdep

In reply to Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

---
 virt/kvm/arm/vgic/vgic.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
virt/kvm/arm/vgic/vgic.c +6 -6
index 7cfdfbc910e0..50e25438fb3c 100644
@@ -196,7 +196,7 @@ void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active)
  */
 static struct kvm_vcpu *vgic_target_oracle(struct vgic_irq *irq)
 {
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&irq->irq_lock));
+	lockdep_assert_held(&irq->irq_lock);
 
 	/* If the interrupt is active, it must stay on the current vcpu */
 	if (irq->active)
@@ -273,7 +273,7 @@ static void vgic_sort_ap_list(struct kvm_vcpu *vcpu)
 {
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&vgic_cpu->ap_list_lock));
+	lockdep_assert_held(&vgic_cpu->ap_list_lock);
 
 	list_sort(NULL, &vgic_cpu->ap_list_head, vgic_irq_cmp);
 }
@@ -311,7 +311,7 @@ bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq,
 {
 	struct kvm_vcpu *vcpu;
 
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&irq->irq_lock));
+	lockdep_assert_held(&irq->irq_lock);
 
 retry:
 	vcpu = vgic_target_oracle(irq);
@@ -702,7 +702,7 @@ static inline void vgic_fold_lr_state(struct kvm_vcpu *vcpu)
 static inline void vgic_populate_lr(struct kvm_vcpu *vcpu,
 				    struct vgic_irq *irq, int lr)
 {
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&irq->irq_lock));
+	lockdep_assert_held(&irq->irq_lock);
 
 	if (kvm_vgic_global_state.type == VGIC_V2)
 		vgic_v2_populate_lr(vcpu, irq, lr);
@@ -736,7 +736,7 @@ static int compute_ap_list_depth(struct kvm_vcpu *vcpu,
 
 	*multi_sgi = false;
 
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&vgic_cpu->ap_list_lock));
+	lockdep_assert_held(&vgic_cpu->ap_list_lock);
 
 	list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) {
 		int w;
@@ -761,7 +761,7 @@ static void vgic_flush_lr_state(struct kvm_vcpu *vcpu)
 	bool multi_sgi;
 	u8 prio = 0xff;
 
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&vgic_cpu->ap_list_lock));
+	lockdep_assert_held(&vgic_cpu->ap_list_lock);
 
 	count = compute_ap_list_depth(vcpu, &multi_sgi);
 	if (count > kvm_vgic_global_state.nr_lr || multi_sgi)
Signature · 1 line
2.19.0
Acked-by on v1 16/16
#35 lore raw

Re: [PATCH 16/16] KVM: arm/arm64: vgic: Replace spin_is_locked() with lockdep

Quoted 10 lines · Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Quoted 70 lines
---
 virt/kvm/arm/vgic/vgic.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
virt/kvm/arm/vgic/vgic.c +6 -6
index 7cfdfbc910e0..50e25438fb3c 100644
@@ -196,7 +196,7 @@ void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active)
  */
 static struct kvm_vcpu *vgic_target_oracle(struct vgic_irq *irq)
 {
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&irq->irq_lock));
+	lockdep_assert_held(&irq->irq_lock);
 
 	/* If the interrupt is active, it must stay on the current vcpu */
 	if (irq->active)
@@ -273,7 +273,7 @@ static void vgic_sort_ap_list(struct kvm_vcpu *vcpu)
 {
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&vgic_cpu->ap_list_lock));
+	lockdep_assert_held(&vgic_cpu->ap_list_lock);
 
 	list_sort(NULL, &vgic_cpu->ap_list_head, vgic_irq_cmp);
 }
@@ -311,7 +311,7 @@ bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq,
 {
 	struct kvm_vcpu *vcpu;
 
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&irq->irq_lock));
+	lockdep_assert_held(&irq->irq_lock);
 
 retry:
 	vcpu = vgic_target_oracle(irq);
@@ -702,7 +702,7 @@ static inline void vgic_fold_lr_state(struct kvm_vcpu *vcpu)
 static inline void vgic_populate_lr(struct kvm_vcpu *vcpu,
 				    struct vgic_irq *irq, int lr)
 {
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&irq->irq_lock));
+	lockdep_assert_held(&irq->irq_lock);
 
 	if (kvm_vgic_global_state.type == VGIC_V2)
 		vgic_v2_populate_lr(vcpu, irq, lr);
@@ -736,7 +736,7 @@ static int compute_ap_list_depth(struct kvm_vcpu *vcpu,
 
 	*multi_sgi = false;
 
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&vgic_cpu->ap_list_lock));
+	lockdep_assert_held(&vgic_cpu->ap_list_lock);
 
 	list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) {
 		int w;
@@ -761,7 +761,7 @@ static void vgic_flush_lr_state(struct kvm_vcpu *vcpu)
 	bool multi_sgi;
 	u8 prio = 0xff;
 
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&vgic_cpu->ap_list_lock));
+	lockdep_assert_held(&vgic_cpu->ap_list_lock);
 
 	count = compute_ap_list_depth(vcpu, &multi_sgi);
 	if (count > kvm_vgic_global_state.nr_lr || multi_sgi)
Signature · 7 lines
2.19.0


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#36 lore raw

Re: [PATCH 16/16] KVM: arm/arm64: vgic: Replace spin_is_locked() with lockdep

Quoted 5 lines · Christoffer Dall
Quoted 4 lines · Lance Roy

lockdep_assert_held() is better suited to checking locking requirements, since it won't get confused when someone else holds the lock. This is also a step towards possibly removing spin_is_locked().

Quoted 8 lines
Quoted 6 lines

Applied, thank you!

Thanx, Paul

Quoted 71 lines
Quoted 70 lines
---
 virt/kvm/arm/vgic/vgic.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
virt/kvm/arm/vgic/vgic.c +6 -6
index 7cfdfbc910e0..50e25438fb3c 100644
@@ -196,7 +196,7 @@ void vgic_irq_set_phys_active(struct vgic_irq *irq, bool active)
  */
 static struct kvm_vcpu *vgic_target_oracle(struct vgic_irq *irq)
 {
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&irq->irq_lock));
+	lockdep_assert_held(&irq->irq_lock);
 
 	/* If the interrupt is active, it must stay on the current vcpu */
 	if (irq->active)
@@ -273,7 +273,7 @@ static void vgic_sort_ap_list(struct kvm_vcpu *vcpu)
 {
 	struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
 
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&vgic_cpu->ap_list_lock));
+	lockdep_assert_held(&vgic_cpu->ap_list_lock);
 
 	list_sort(NULL, &vgic_cpu->ap_list_head, vgic_irq_cmp);
 }
@@ -311,7 +311,7 @@ bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq,
 {
 	struct kvm_vcpu *vcpu;
 
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&irq->irq_lock));
+	lockdep_assert_held(&irq->irq_lock);
 
 retry:
 	vcpu = vgic_target_oracle(irq);
@@ -702,7 +702,7 @@ static inline void vgic_fold_lr_state(struct kvm_vcpu *vcpu)
 static inline void vgic_populate_lr(struct kvm_vcpu *vcpu,
 				    struct vgic_irq *irq, int lr)
 {
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&irq->irq_lock));
+	lockdep_assert_held(&irq->irq_lock);
 
 	if (kvm_vgic_global_state.type == VGIC_V2)
 		vgic_v2_populate_lr(vcpu, irq, lr);
@@ -736,7 +736,7 @@ static int compute_ap_list_depth(struct kvm_vcpu *vcpu,
 
 	*multi_sgi = false;
 
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&vgic_cpu->ap_list_lock));
+	lockdep_assert_held(&vgic_cpu->ap_list_lock);
 
 	list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) {
 		int w;
@@ -761,7 +761,7 @@ static void vgic_flush_lr_state(struct kvm_vcpu *vcpu)
 	bool multi_sgi;
 	u8 prio = 0xff;
 
-	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&vgic_cpu->ap_list_lock));
+	lockdep_assert_held(&vgic_cpu->ap_list_lock);
 
 	count = compute_ap_list_depth(vcpu, &multi_sgi);
 	if (count > kvm_vgic_global_state.nr_lr || multi_sgi)
Signature · 7 lines
2.19.0


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel