Thread (20 messages) 20 messages, 6 authors, 2022-09-30

Re: [Intel-wired-lan] [PATCH] ixgbe: Use kmap_local_page in ixgbe_check_lbtest_frame()

From: Fabio M. De Francesco <hidden>
Date: 2022-07-01 15:37:03
Also in: bpf, intel-wired-lan, lkml

On giovedì 30 giugno 2022 23:59:23 CEST Alexander Duyck wrote:
On Thu, Jun 30, 2022 at 11:18 AM Fabio M. De Francesco
[off-list ref] wrote:
quoted
On giovedì 30 giugno 2022 18:09:18 CEST Alexander Duyck wrote:
quoted
On Thu, Jun 30, 2022 at 8:25 AM Eric Dumazet [off-list ref] 
wrote:
quoted
quoted
quoted
On Thu, Jun 30, 2022 at 5:17 PM Alexander Duyck
[off-list ref] wrote:
quoted
On Thu, Jun 30, 2022 at 3:10 AM Maciej Fijalkowski
[off-list ref] wrote:
quoted
On Wed, Jun 29, 2022 at 10:58:36AM +0200, Fabio M. De Francesco
wrote:
quoted
quoted
quoted
quoted
quoted
The use of kmap() is being deprecated in favor of
kmap_local_page().
quoted
quoted
quoted
quoted
quoted
With kmap_local_page(), the mapping is per thread, CPU local 
and
quoted
not
quoted
quoted
quoted
quoted
quoted
globally visible. Furthermore, the mapping can be acquired 
from
quoted
any context
quoted
quoted
quoted
quoted
quoted
(including interrupts).

Therefore, use kmap_local_page() in 
ixgbe_check_lbtest_frame()
quoted
because
quoted
quoted
quoted
quoted
quoted
this mapping is per thread, CPU local, and not globally 
visible.
quoted
quoted
quoted
quoted
quoted
Hi,

I'd like to ask why kmap was there in the first place and not 
plain
quoted
quoted
quoted
quoted
quoted
page_address() ?

Alex?
The page_address function only works on architectures that have
access
quoted
quoted
quoted
to all of physical memory via virtual memory addresses. The kmap
function is meant to take care of highmem which will need to be
mapped
quoted
quoted
quoted
before it can be accessed.

For non-highmem pages kmap just calls the page_address function.
https://elixir.bootlin.com/linux/latest/source/include/linux/
highmem-internal.h#L40
quoted
quoted
quoted

Sure, but drivers/net/ethernet/intel/ixgbe/ixgbe_main.c is 
allocating
quoted
quoted
quoted
pages that are not highmem ?

This kmap() does not seem needed.
Good point. So odds are page_address is fine to use. Actually there 
is
quoted
quoted
a note to that effect in ixgbe_pull_tail.

As such we could probably go through and update igb, and several of
the other Intel drivers as well.

- Alex
I don't know this code, however I know kmap*().

I assumed that, if author used kmap(), there was possibility that the 
page
quoted
came from highmem.

In that case kmap_local_page() looks correct here.

However, now I read that that page _cannot_ come from highmem. 
Therefore,
quoted
page_address() would suffice.

If you all want I can replace kmap() / kunmap() with a "plain"
page_address(). Please let me know.

Thanks,

Fabio
Replacing it with just page_address() should be fine. Back when I
wrote the code I didn't realize that GFP_ATOMIC pages weren't
allocated from highmem so I suspect I just used kmap since it was the
way to cover all the bases.

Thanks,

- Alex
OK, I'm about to prepare another patch with page_address() (obviously, this 
should be discarded).

Last thing... Is that page allocated with dma_pool_alloc() at
ixgbe/ixgbe_fcoe.c:196? Somewhere else?

Thanks,

Fabio

P.S.: Can you say something about how pages are allocated in intel/e1000 
and in intel/e1000e? I see that those drivers use kmap_atomic().


Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help