netdev - 答复: [PATCH] page_pool: fill page only when refill condition is true

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <130d2489bca54b36bda0a8178b8535b4@baidu.com>
Date:   Thu, 6 Feb 2020 02:23:22 +0000
From:   "Li,Rongqing" <lirongqing@...du.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>
Subject: 答复: [PATCH] page_pool: fill page only when refill condition is true



> -----邮件原件-----
> 发件人: Jesper Dangaard Brouer [mailto:brouer@...hat.com]
> 发送时间: 2020年2月5日 19:51
> 收件人: Li,Rongqing <lirongqing@...du.com>
> 抄送: netdev@...r.kernel.org; brouer@...hat.com; Ilias Apalodimas
> <ilias.apalodimas@...aro.org>
> 主题: Re: [PATCH] page_pool: fill page only when refill condition is true
> 
> On Wed,  5 Feb 2020 16:22:34 +0800
> Li RongQing <lirongqing@...du.com> wrote:
> 
> > "do {} while" in page_pool_refill_alloc_cache will always refill page
> > once whether refill is true or false, and whether alloc.count of pool
> > is less than PP_ALLOC_CACHE_REFILL.
> >
> > so fix it by calling page_pool_refill_alloc_cache() only when refill
> > is true
> >
> > Fixes: 44768decb7c0 ("page_pool: handle page recycle for NUMA_NO_NODE
> > condition")
> > Signed-off-by: Li RongQing <lirongqing@...du.com>
> > Cc: Jesper Dangaard Brouer <brouer@...hat.com>
> 
> Hmmm... I'm not 100% convinced this is the right approach.
> 
> I do realize that in commit 44768decb7c0, I also added touching
> pool->alloc.cache[] which was protected by "called under" in_serving_softirq().
> (before I used a locked ptr_ring_consume(r)).
> 
> BUT maybe it will be better to remove, the test in_serving_softirq(), because
> the caller should provide guarantee that pool->alloc.cache[] is safe to access.
> 
> I added this in_serving_softirq() check, because I noticed NIC drivers will call
> this from normal process context, during (1) initial fill of their RX-rings, and (2)
> during driver RX-ring shutdown.  BUT in both cases the NIC drivers will first
> have made sure that their RX-ring have been disconnected and no concurrent
> accesses will happen.  Thus, access to pool->alloc.cache[] is safe, so
> page_pool API should trust the caller knows this.
> 
> 

I think it is better and semantic clarity to keep in_serving_softirq, and we can see opinions of others

Thanks

-Li