2017-09-27 12:25:50

by Artem Savkov

Subject: [PATCH] xfrm: don't call xfrm_policy_cache_flush under xfrm_state_lock

I might be wrong but it doesn't look like xfrm_state_lock is required
for xfrm_policy_cache_flush and calling it under this lock triggers both
"sleeping function called from invalid context" and "possible circular
locking dependency detected" warnings on flush.

Fixes: ec30d78c14a8 xfrm: add xdst pcpu cache
Signed-off-by: Artem Savkov <[email protected]>
---
net/xfrm/xfrm_state.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_state.c b/net/xfrm/xfrm_state.c
index 0dab1cd79ce4..12213477cd3a 100644
--- a/net/xfrm/xfrm_state.c
+++ b/net/xfrm/xfrm_state.c
@@ -732,12 +732,12 @@ int xfrm_state_flush(struct net *net, u8 proto, bool task_valid)
}
}
}
+out:
+ spin_unlock_bh(&net->xfrm.xfrm_state_lock);
if (cnt) {
err = 0;
xfrm_policy_cache_flush();
}
-out:
- spin_unlock_bh(&net->xfrm.xfrm_state_lock);
return err;
}
EXPORT_SYMBOL(xfrm_state_flush);
--
2.13.5

2017-09-27 12:34:53

by Florian Westphal

Subject: Re: [PATCH] xfrm: don't call xfrm_policy_cache_flush under xfrm_state_lock

Artem Savkov <[email protected]> wrote:
> I might be wrong but it doesn't look like xfrm_state_lock is required
> for xfrm_policy_cache_flush and calling it under this lock triggers both
> "sleeping function called from invalid context" and "possible circular
> locking dependency detected" warnings on flush.
>
> Fixes: ec30d78c14a8 xfrm: add xdst pcpu cache
> Signed-off-by: Artem Savkov <[email protected]>

You're right, its not needed (and wrong).

Acked-by: Florian Westphal <[email protected]>

2017-09-28 11:12:41

by Steffen Klassert

Subject: Re: [PATCH] xfrm: don't call xfrm_policy_cache_flush under xfrm_state_lock

On Wed, Sep 27, 2017 at 02:31:03PM +0200, Florian Westphal wrote:
> Artem Savkov <[email protected]> wrote:
> > I might be wrong but it doesn't look like xfrm_state_lock is required
> > for xfrm_policy_cache_flush and calling it under this lock triggers both
> > "sleeping function called from invalid context" and "possible circular
> > locking dependency detected" warnings on flush.
> >
> > Fixes: ec30d78c14a8 xfrm: add xdst pcpu cache
> > Signed-off-by: Artem Savkov <[email protected]>
>
> You're right, its not needed (and wrong).
>
> Acked-by: Florian Westphal <[email protected]>

Applied to the ipsec tree, thanks everyone!