* [PATCH] net: xen-netbank: hash.c: Use built-in RCU list checking
@ 2020-01-15 12:41 madhuparnabhowmik04
2020-01-15 13:56 ` Wei Liu
0 siblings, 1 reply; 3+ messages in thread
From: madhuparnabhowmik04 @ 2020-01-15 12:41 UTC (permalink / raw)
To: wei.liu, paul, davem
Cc: xen-devel, netdev, linux-kernel, paulmck, joel, frextrite,
linux-kernel-mentees, Madhuparna Bhowmik
From: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
list_for_each_entry_rcu has built-in RCU and lock checking.
Pass cond argument to list_for_each_entry_rcu.
Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
---
drivers/net/xen-netback/hash.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/net/xen-netback/hash.c b/drivers/net/xen-netback/hash.c
index 10d580c3dea3..30709bc9d170 100644
--- a/drivers/net/xen-netback/hash.c
+++ b/drivers/net/xen-netback/hash.c
@@ -51,7 +51,8 @@ static void xenvif_add_hash(struct xenvif *vif, const u8 *tag,
found = false;
oldest = NULL;
- list_for_each_entry_rcu(entry, &vif->hash.cache.list, link) {
+ list_for_each_entry_rcu(entry, &vif->hash.cache.list, link,
+ lockdep_is_held(&vif->hash.cache.lock)) {
/* Make sure we don't add duplicate entries */
if (entry->len == len &&
memcmp(entry->tag, tag, len) == 0)
@@ -102,7 +103,8 @@ static void xenvif_flush_hash(struct xenvif *vif)
spin_lock_irqsave(&vif->hash.cache.lock, flags);
- list_for_each_entry_rcu(entry, &vif->hash.cache.list, link) {
+ list_for_each_entry_rcu(entry, &vif->hash.cache.list, link,
+ lockdep_is_held(&vif->hash.cache.lock)) {
list_del_rcu(&entry->link);
vif->hash.cache.count--;
kfree_rcu(entry, rcu);
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] net: xen-netbank: hash.c: Use built-in RCU list checking
2020-01-15 12:41 [PATCH] net: xen-netbank: hash.c: Use built-in RCU list checking madhuparnabhowmik04
@ 2020-01-15 13:56 ` Wei Liu
[not found] ` <CAF65HP0q_KcrUP_50JxZL1xNc47=detHvdOzjBmuiqUtB3AwfA@mail.gmail.com>
0 siblings, 1 reply; 3+ messages in thread
From: Wei Liu @ 2020-01-15 13:56 UTC (permalink / raw)
To: madhuparnabhowmik04
Cc: wei.liu, paul, davem, xen-devel, netdev, linux-kernel, paulmck,
joel, frextrite, linux-kernel-mentees
Thanks for the patch.
There is a typo in the subject line. It should say xen-netback, not
xen-netbank.
On Wed, Jan 15, 2020 at 06:11:28PM +0530, madhuparnabhowmik04@gmail.com wrote:
> From: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
>
> list_for_each_entry_rcu has built-in RCU and lock checking.
> Pass cond argument to list_for_each_entry_rcu.
>
> Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik04@gmail.com>
> ---
> drivers/net/xen-netback/hash.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/xen-netback/hash.c b/drivers/net/xen-netback/hash.c
> index 10d580c3dea3..30709bc9d170 100644
> --- a/drivers/net/xen-netback/hash.c
> +++ b/drivers/net/xen-netback/hash.c
> @@ -51,7 +51,8 @@ static void xenvif_add_hash(struct xenvif *vif, const u8 *tag,
>
> found = false;
> oldest = NULL;
> - list_for_each_entry_rcu(entry, &vif->hash.cache.list, link) {
> + list_for_each_entry_rcu(entry, &vif->hash.cache.list, link,
> + lockdep_is_held(&vif->hash.cache.lock)) {
There are probably too many tabs here. Indentation looks wrong.
The surrounding code makes it pretty clear that the lock is already held
by the time list_for_each_entry_rcu is called, yet the checking involved
in lockdep_is_held is not trivial, so I'm afraid I don't consider this a
strict improvement over the existing code.
If there is something I misunderstood, let me know.
Wei.
> /* Make sure we don't add duplicate entries */
> if (entry->len == len &&
> memcmp(entry->tag, tag, len) == 0)
> @@ -102,7 +103,8 @@ static void xenvif_flush_hash(struct xenvif *vif)
>
> spin_lock_irqsave(&vif->hash.cache.lock, flags);
>
> - list_for_each_entry_rcu(entry, &vif->hash.cache.list, link) {
> + list_for_each_entry_rcu(entry, &vif->hash.cache.list, link,
> + lockdep_is_held(&vif->hash.cache.lock)) {
> list_del_rcu(&entry->link);
> vif->hash.cache.count--;
> kfree_rcu(entry, rcu);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] net: xen-netbank: hash.c: Use built-in RCU list checking
[not found] ` <CAF65HP0q_KcrUP_50JxZL1xNc47=detHvdOzjBmuiqUtB3AwfA@mail.gmail.com>
@ 2020-01-15 15:04 ` Wei Liu
0 siblings, 0 replies; 3+ messages in thread
From: Wei Liu @ 2020-01-15 15:04 UTC (permalink / raw)
To: Madhuparna Bhowmik
Cc: Wei Liu, paul, davem, xen-devel, netdev, linux-kernel,
Paul E. McKenney, Joel Fernandes, Amol Grover,
linux-kernel-mentees
On Wed, Jan 15, 2020 at 07:36:38PM +0530, Madhuparna Bhowmik wrote:
[...]
>
> > The surrounding code makes it pretty clear that the lock is already held
> > by the time list_for_each_entry_rcu is called, yet the checking involved
> > in lockdep_is_held is not trivial, so I'm afraid I don't consider this a
> > strict improvement over the existing code.
> >
> > Actually, we want to make CONFIG_PROVE_LIST_RCU enabled by default.
I think you meant CONFIG_PROVE_RCU_LIST.
> And if the cond argument is not passed when the usage of
> list_for_each_entry_rcu()
> is outside of rcu_read_lock(), it will lead to a false positive.
> Therefore, I think this patch is required.
Fair enough.
Wei.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-01-15 15:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-15 12:41 [PATCH] net: xen-netbank: hash.c: Use built-in RCU list checking madhuparnabhowmik04
2020-01-15 13:56 ` Wei Liu
[not found] ` <CAF65HP0q_KcrUP_50JxZL1xNc47=detHvdOzjBmuiqUtB3AwfA@mail.gmail.com>
2020-01-15 15:04 ` Wei Liu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).