On 7/1/2022 8:36 AM, Fabio M. De Francesco wrote:

On giovedì 30 giugno 2022 23:59:23 CEST Alexander Duyck wrote:

quoted

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

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

quoted

not

quoted

quoted

quoted

quoted

quoted

globally visible. Furthermore, the mapping can be acquired

from

quoted

quoted

any context

quoted

quoted

quoted

quoted

quoted

(including interrupts).

Therefore, use kmap_local_page() in

ixgbe_check_lbtest_frame()

quoted

quoted

because

quoted

quoted

quoted

quoted

quoted

this mapping is per thread, CPU local, and not globally

visible.

quoted

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

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

quoted

Sure, but drivers/net/ethernet/intel/ixgbe/ixgbe_main.c is

allocating

quoted

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

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

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

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().

Following Fabio's patches, I made similar changes for e1000/e1000e and 
submitted them to IWL [1].

Yesterday, Ira Weiny pointed me to some feedback from Dave Hansen on the 
use of page_address() [2]. My understanding of this feedback is that 
it's safer to use kmap_local_page() instead of page_address(), because 
you don't always know how the underlying page was allocated.

This approach (of using kmap_local_page() instead of page_address()) 
makes sense to me. Any reason not to go this way?

[1]

https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20220919180949.388785-1-anirudh.venkataramanan@intel.com/

https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20220919180949.388785-2-anirudh.venkataramanan@intel.com/

[2] 
https://lore.kernel.org/lkml/5d667258-b58b-3d28-3609-e7914c99b31b@intel.com/ (local)

Ani