Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20220629085836.18042-1-fmdefrancesco@gmail.com>
 <2254584.ElGaqSPkdT@opensuse> <CAKgT0UfThk3MLcE38wQu5+2Qy7Ld2px-2WJgnD+2xbDsA8iEEw@mail.gmail.com>
 <2834855.e9J7NaK4W3@opensuse> <d4e33ca3-92e5-ba30-f103-09d028526ea2@intel.com>
In-Reply-To: <d4e33ca3-92e5-ba30-f103-09d028526ea2@intel.com>
From:   Alexander Duyck <alexander.duyck@gmail.com>
Date:   Thu, 22 Sep 2022 13:58:50 -0700
Message-ID: <CAKgT0Uf1o+i0qKf7J_xqC3SACRFhiYqyhBeQydgUafB5uFkAvg@mail.gmail.com>
Subject: Re: [Intel-wired-lan] [PATCH] ixgbe: Use kmap_local_page in ixgbe_check_lbtest_frame()
To:     Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
Cc:     "Fabio M. De Francesco" <fmdefrancesco@gmail.com>,
        Jakub Kicinski <kuba@kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Netdev <netdev@vger.kernel.org>,
        Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
        Jesper Dangaard Brouer <hawk@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        intel-wired-lan <intel-wired-lan@lists.osuosl.org>,
        Alexander Duyck <alexanderduyck@fb.com>,
        John Fastabend <john.fastabend@gmail.com>,
        Jesse Brandeburg <jesse.brandeburg@intel.com>,
        Alexei Starovoitov <ast@kernel.org>, bpf <bpf@vger.kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Ira Weiny <ira.weiny@intel.com>,
        LKML <linux-kernel@vger.kernel.org>,
        Tony Nguyen <anthony.l.nguyen@intel.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Sep 22, 2022 at 1:07 PM Anirudh Venkataramanan
<anirudh.venkataramanan@intel.com> wrote:
>
>
> 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/
>
> Ani

For the two patches you referenced the driver is the one allocating
the pages. So in such a case the page_address should be acceptable.
Specifically we are falling into alloc_page(GFP_ATOMIC) which should
fall into the first case that Dave Hansen called out.

If it was the Tx path that would be another matter, however these are
Rx only pages so they are allocated by the driver directly and won't
be allocated from HIGHMEM.