| Message ID | 20220514091443.4150162-1-william.xuanziyang@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: wwan: t7xx: fix GFP_KERNEL usage in spin_lock context | expand |
Hello Ziyang, On Sat, May 14, 2022 at 11:57 AM Ziyang Xuan <william.xuanziyang@huawei.com> wrote: > t7xx_cldma_clear_rxq() call t7xx_cldma_alloc_and_map_skb() in spin_lock > context, But __dev_alloc_skb() in t7xx_cldma_alloc_and_map_skb() uses > GFP_KERNEL, that will introduce scheduling factor in spin_lock context. > > Replace GFP_KERNEL with GFP_ATOMIC to fix it. Would not it will be more reliable to just rework t7xx_cldma_clear_rxq() to avoid calling t7xx_cldma_alloc_and_map_skb() under the spin lock instead of doing each allocation with GFP_ATOMIC? E.g. t7xx_cldma_gpd_rx_from_q() calls t7xx_cldma_alloc_and_map_skb() avoiding any lock holding.
On 5/16/2022 1:36 PM, Sergey Ryazanov wrote: > Hello Ziyang, > > On Sat, May 14, 2022 at 11:57 AM Ziyang Xuan > <william.xuanziyang@huawei.com> wrote: >> t7xx_cldma_clear_rxq() call t7xx_cldma_alloc_and_map_skb() in spin_lock >> context, But __dev_alloc_skb() in t7xx_cldma_alloc_and_map_skb() uses >> GFP_KERNEL, that will introduce scheduling factor in spin_lock context. >> >> Replace GFP_KERNEL with GFP_ATOMIC to fix it. > Would not it will be more reliable to just rework > t7xx_cldma_clear_rxq() to avoid calling t7xx_cldma_alloc_and_map_skb() > under the spin lock instead of doing each allocation with GFP_ATOMIC? > E.g. t7xx_cldma_gpd_rx_from_q() calls t7xx_cldma_alloc_and_map_skb() > avoiding any lock holding. t7xx_cldma_clear_rxq() is a helper for t7xx_cldma_clear_all_qs() which is only called by t7xx_cldma_exception() after stopping CLDMA, so it should be OK to remove the spin lock from t7xx_cldma_clear_rxq().
> > On 5/16/2022 1:36 PM, Sergey Ryazanov wrote: >> Hello Ziyang, >> >> On Sat, May 14, 2022 at 11:57 AM Ziyang Xuan >> <william.xuanziyang@huawei.com> wrote: >>> t7xx_cldma_clear_rxq() call t7xx_cldma_alloc_and_map_skb() in spin_lock >>> context, But __dev_alloc_skb() in t7xx_cldma_alloc_and_map_skb() uses >>> GFP_KERNEL, that will introduce scheduling factor in spin_lock context. >>> >>> Replace GFP_KERNEL with GFP_ATOMIC to fix it. >> Would not it will be more reliable to just rework >> t7xx_cldma_clear_rxq() to avoid calling t7xx_cldma_alloc_and_map_skb() >> under the spin lock instead of doing each allocation with GFP_ATOMIC? >> E.g. t7xx_cldma_gpd_rx_from_q() calls t7xx_cldma_alloc_and_map_skb() >> avoiding any lock holding. > > t7xx_cldma_clear_rxq() is a helper for t7xx_cldma_clear_all_qs() which is only called by t7xx_cldma_exception() after stopping CLDMA, so it should be OK to remove the spin lock from t7xx_cldma_clear_rxq(). > OK, I see. Thus we can remove spink_lock and annotate it. > > .
diff --git a/drivers/net/wwan/t7xx/t7xx_hif_cldma.c b/drivers/net/wwan/t7xx/t7xx_hif_cldma.c index 46066dcd2607..54c34639f1a5 100644 --- a/drivers/net/wwan/t7xx/t7xx_hif_cldma.c +++ b/drivers/net/wwan/t7xx/t7xx_hif_cldma.c @@ -93,7 +93,7 @@ static void t7xx_cldma_gpd_set_next_ptr(struct cldma_gpd *gpd, dma_addr_t next_p static int t7xx_cldma_alloc_and_map_skb(struct cldma_ctrl *md_ctrl, struct cldma_request *req, size_t size) { - req->skb = __dev_alloc_skb(size, GFP_KERNEL); + req->skb = __dev_alloc_skb(size, GFP_ATOMIC); if (!req->skb) return -ENOMEM;
t7xx_cldma_clear_rxq() call t7xx_cldma_alloc_and_map_skb() in spin_lock context, But __dev_alloc_skb() in t7xx_cldma_alloc_and_map_skb() uses GFP_KERNEL, that will introduce scheduling factor in spin_lock context. Replace GFP_KERNEL with GFP_ATOMIC to fix it. Fixes: 39d439047f1d ("net: wwan: t7xx: Add control DMA interface") Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com> --- drivers/net/wwan/t7xx/t7xx_hif_cldma.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)