On 7/1/2022 8:36 AM, Fabio M. De Francesco wrote:
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
<fmdefrancesco@xxxxxxxxx> wrote:
On giovedì 30 giugno 2022 18:09:18 CEST Alexander Duyck wrote:
On Thu, Jun 30, 2022 at 8:25 AM Eric Dumazet <edumazet@xxxxxxxxxx>
wrote:
On Thu, Jun 30, 2022 at 5:17 PM Alexander Duyck
<alexander.duyck@xxxxxxxxx> wrote:
On Thu, Jun 30, 2022 at 3:10 AM Maciej Fijalkowski
<maciej.fijalkowski@xxxxxxxxx> wrote:
On Wed, Jun 29, 2022 at 10:58:36AM +0200, Fabio M. De Francesco
wrote:
The use of kmap() is being deprecated in favor of
kmap_local_page().
With kmap_local_page(), the mapping is per thread, CPU local
and
not
globally visible. Furthermore, the mapping can be acquired
from
any context
(including interrupts).
Therefore, use kmap_local_page() in
ixgbe_check_lbtest_frame()
because
this mapping is per thread, CPU local, and not globally
visible.
Hi,
I'd like to ask why kmap was there in the first place and not
plain
page_address() ?
Alex?
The page_address function only works on architectures that have
access
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
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
Sure, but drivers/net/ethernet/intel/ixgbe/ixgbe_main.c is
allocating
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
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
came from highmem.
In that case kmap_local_page() looks correct here.
However, now I read that that page _cannot_ come from highmem.
Therefore,
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@xxxxxxxxx/
https://patchwork.ozlabs.org/project/intel-wired-lan/patch/20220919180949.388785-2-anirudh.venkataramanan@xxxxxxxxx/
[2]
https://lore.kernel.org/lkml/5d667258-b58b-3d28-3609-e7914c99b31b@xxxxxxxxx/
Ani
