On Fri, 2019-05-31 at 20:03 +0300, Ivan Khoronzhuk wrote:

On Fri, May 31, 2019 at 06:32:41PM +0200, Jesper Dangaard Brouer
wrote:

quoted

On Fri, 31 May 2019 19:25:24 +0300 Ivan Khoronzhuk <
ivan.khoronzhuk@linaro.org> wrote:

quoted

On Fri, May 31, 2019 at 05:46:43PM +0200, Jesper Dangaard Brouer
wrote:

quoted

From below code snippets, it looks like you only allocated 1
page_pool
and sharing it with several RX-queues, as I don't have the full
context
and don't know this driver, I might be wrong?

To be clear, a page_pool object is needed per RX-queue, as it
is
accessing a small RX page cache (which protected by
NAPI/softirq).

There is one RX interrupt and one RX NAPI for all rx channels.

So, what are you saying?

You _are_ sharing the page_pool between several RX-channels, but it
is
safe because this hardware only have one RX interrupt + NAPI
instance??

I can miss smth but in case of cpsw technically it means:
1) RX interrupts are disabled while NAPI is scheduled,
   not for particular CPU or channel, but at all, for whole cpsw
module.
2) RX channels are handled one by one by priority.

Hi Ivan, I got a silly question.. 

What is the reason behind having multiple RX rings and one CPU/NAPI
handling all of them ? priority ? how do you priorities ?

3) After all of them handled and no more in budget - interrupts are
enabled.
4) If page is returned to the pool, and it's within NAPI, no races as
it's
   returned protected by softirq. If it's returned not in softirq
it's protected 
   by producer lock of the ring.

Probably it's not good example for others how it should be used, not
a big
problem to move it to separate pools.., even don't remember why I
decided to
use shared pool, there was some more reasons... need search in
history.

quoted

-- 
Best regards,
 Jesper Dangaard Brouer
 MSc.CS, Principal Kernel Engineer at Red Hat
 LinkedIn: http://www.linkedin.com/in/brouer