| Message ID | 2bc5f05b0c50082cf0f5c817ae7c9f660848f146.1618394396.git.sd@queasysnail.net (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [ipsec-next] xfrm: ipcomp: remove unnecessary get_cpu() | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Guessed tree name to be net-next |
| netdev/subject_prefix | warning | Target tree name not specified in the subject |
| netdev/cc_maintainers | warning | 3 maintainers not CCed: davem@davemloft.net herbert@gondor.apana.org.au kuba@kernel.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 54 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
On Fri, Apr 16, 2021 at 05:11:46PM +0200, Sabrina Dubroca wrote: > While testing ipcomp on a realtime kernel, Xiumei reported a "sleeping > in atomic" bug, caused by a memory allocation while preemption is > disabled (ipcomp_decompress -> alloc_page -> ... get_page_from_freelist). > > As Sebastian noted [1], this get_cpu() isn't actually needed, since > ipcomp_decompress() is called in napi context anyway, so BH is already > disabled. > > This patch replaces get_cpu + per_cpu_ptr with this_cpu_ptr, then > simplifies the error returns, since there isn't any common operation > left. > > [1] https://lore.kernel.org/lkml/20190820082810.ixkmi56fp7u7eyn2@linutronix.de/ > > Cc: Juri Lelli <jlelli@redhat.com> > Reported-by: Xiumei Mu <xmu@redhat.com> > Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> > Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> Applied, thanks a lot!
diff --git a/net/xfrm/xfrm_ipcomp.c b/net/xfrm/xfrm_ipcomp.c index 4d422447aadc..2e8afe078d61 100644 --- a/net/xfrm/xfrm_ipcomp.c +++ b/net/xfrm/xfrm_ipcomp.c @@ -41,19 +41,16 @@ static int ipcomp_decompress(struct xfrm_state *x, struct sk_buff *skb) const int plen = skb->len; int dlen = IPCOMP_SCRATCH_SIZE; const u8 *start = skb->data; - const int cpu = get_cpu(); - u8 *scratch = *per_cpu_ptr(ipcomp_scratches, cpu); - struct crypto_comp *tfm = *per_cpu_ptr(ipcd->tfms, cpu); + u8 *scratch = *this_cpu_ptr(ipcomp_scratches); + struct crypto_comp *tfm = *this_cpu_ptr(ipcd->tfms); int err = crypto_comp_decompress(tfm, start, plen, scratch, &dlen); int len; if (err) - goto out; + return err; - if (dlen < (plen + sizeof(struct ip_comp_hdr))) { - err = -EINVAL; - goto out; - } + if (dlen < (plen + sizeof(struct ip_comp_hdr))) + return -EINVAL; len = dlen - plen; if (len > skb_tailroom(skb)) @@ -68,16 +65,14 @@ static int ipcomp_decompress(struct xfrm_state *x, struct sk_buff *skb) skb_frag_t *frag; struct page *page; - err = -EMSGSIZE; if (WARN_ON(skb_shinfo(skb)->nr_frags >= MAX_SKB_FRAGS)) - goto out; + return -EMSGSIZE; frag = skb_shinfo(skb)->frags + skb_shinfo(skb)->nr_frags; page = alloc_page(GFP_ATOMIC); - err = -ENOMEM; if (!page) - goto out; + return -ENOMEM; __skb_frag_set_page(frag, page); @@ -96,11 +91,7 @@ static int ipcomp_decompress(struct xfrm_state *x, struct sk_buff *skb) skb_shinfo(skb)->nr_frags++; } - err = 0; - -out: - put_cpu(); - return err; + return 0; } int ipcomp_input(struct xfrm_state *x, struct sk_buff *skb)
While testing ipcomp on a realtime kernel, Xiumei reported a "sleeping in atomic" bug, caused by a memory allocation while preemption is disabled (ipcomp_decompress -> alloc_page -> ... get_page_from_freelist). As Sebastian noted [1], this get_cpu() isn't actually needed, since ipcomp_decompress() is called in napi context anyway, so BH is already disabled. This patch replaces get_cpu + per_cpu_ptr with this_cpu_ptr, then simplifies the error returns, since there isn't any common operation left. [1] https://lore.kernel.org/lkml/20190820082810.ixkmi56fp7u7eyn2@linutronix.de/ Cc: Juri Lelli <jlelli@redhat.com> Reported-by: Xiumei Mu <xmu@redhat.com> Suggested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de> Signed-off-by: Sabrina Dubroca <sd@queasysnail.net> --- net/xfrm/xfrm_ipcomp.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-)