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