Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
MIME-Version: 1.0
References: <20230922091138.18014-1-linyunsheng@huawei.com>
 <20230922091138.18014-2-linyunsheng@huawei.com> <b70b44bec789b60a99c18e43f6270f9c48e3d704.camel@redhat.com>
In-Reply-To: <b70b44bec789b60a99c18e43f6270f9c48e3d704.camel@redhat.com>
From:   Ilias Apalodimas <ilias.apalodimas@linaro.org>
Date:   Tue, 3 Oct 2023 12:40:13 +0300
Message-ID: <CAC_iWjL8jjZWSMdbZ=LqWKVKLR_3ZYzrBv8RwG+AgMqZWQEyaA@mail.gmail.com>
Subject: Re: [PATCH net-next v10 1/6] page_pool: fragment API support for
 32-bit arch with 64-bit DMA
To:     Paolo Abeni <pabeni@redhat.com>
Cc:     Yunsheng Lin <linyunsheng@huawei.com>, davem@davemloft.net,
        kuba@kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Lorenzo Bianconi <lorenzo@kernel.org>,
        Alexander Duyck <alexander.duyck@gmail.com>,
        Liang Chen <liangchen.linux@gmail.com>,
        Alexander Lobakin <aleksander.lobakin@intel.com>,
        Guillaume Tucker <guillaume.tucker@collabora.com>,
        Matthew Wilcox <willy@infradead.org>,
        Linux-MM <linux-mm@kvack.org>,
        Jesper Dangaard Brouer <hawk@kernel.org>,
        Eric Dumazet <edumazet@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Paolo

On Tue, 3 Oct 2023 at 10:46, Paolo Abeni <pabeni@redhat.com> wrote:
>
> On Fri, 2023-09-22 at 17:11 +0800, Yunsheng Lin wrote:
> > Currently page_pool_alloc_frag() is not supported in 32-bit
> > arch with 64-bit DMA because of the overlap issue between
> > pp_frag_count and dma_addr_upper in 'struct page' for those
> > arches, which seems to be quite common, see [1], which means
> > driver may need to handle it when using fragment API.
> >
> > It is assumed that the combination of the above arch with an
> > address space >16TB does not exist, as all those arches have
> > 64b equivalent, it seems logical to use the 64b version for a
> > system with a large address space. It is also assumed that dma
> > address is page aligned when we are dma mapping a page aligned
> > buffer, see [2].
> >
> > That means we're storing 12 bits of 0 at the lower end for a
> > dma address, we can reuse those bits for the above arches to
> > support 32b+12b, which is 16TB of memory.
> >
> > If we make a wrong assumption, a warning is emitted so that
> > user can report to us.
> >
> > 1. https://lore.kernel.org/all/20211117075652.58299-1-linyunsheng@huawei.com/
> > 2. https://lore.kernel.org/all/20230818145145.4b357c89@kernel.org/
> >
> > Signed-off-by: Jakub Kicinski <kuba@kernel.org>
> > Signed-off-by: Yunsheng Lin <linyunsheng@huawei.com>
> > CC: Lorenzo Bianconi <lorenzo@kernel.org>
> > CC: Alexander Duyck <alexander.duyck@gmail.com>
> > CC: Liang Chen <liangchen.linux@gmail.com>
> > CC: Alexander Lobakin <aleksander.lobakin@intel.com>
> > CC: Guillaume Tucker <guillaume.tucker@collabora.com>
> > CC: Matthew Wilcox <willy@infradead.org>
> > CC: Linux-MM <linux-mm@kvack.org>
> > ---
> >  include/linux/mm_types.h        | 13 +------------
> >  include/net/page_pool/helpers.h | 20 ++++++++++++++------
> >  net/core/page_pool.c            | 14 +++++++++-----
> >  3 files changed, 24 insertions(+), 23 deletions(-)
> >
> > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > index 36c5b43999e6..74b49c4c7a52 100644
> > --- a/include/linux/mm_types.h
> > +++ b/include/linux/mm_types.h
> > @@ -125,18 +125,7 @@ struct page {
> >                       struct page_pool *pp;
> >                       unsigned long _pp_mapping_pad;
> >                       unsigned long dma_addr;
> > -                     union {
> > -                             /**
> > -                              * dma_addr_upper: might require a 64-bit
> > -                              * value on 32-bit architectures.
> > -                              */
> > -                             unsigned long dma_addr_upper;
> > -                             /**
> > -                              * For frag page support, not supported in
> > -                              * 32-bit architectures with 64-bit DMA.
> > -                              */
> > -                             atomic_long_t pp_frag_count;
> > -                     };
> > +                     atomic_long_t pp_frag_count;
> >               };
> >               struct {        /* Tail pages of compound page */
> >                       unsigned long compound_head;    /* Bit zero is set */
>
> As noted by Jesper, since this is touching the super-critcal struct
> page, an explicit ack from the mm people is required.
>
> @Matthew: could you please have a look?
>
> I think it would be nice also an explicit ack from Jesper and/or Ilias.

I am trying! Unfortunately, it's this time of the year when I have to
travel a lot.  I'll be back in 15 days from now and I will be able to
have a look

Thanks and sorry for the delay
/Ilias

>
> Cheers,
>
> Paolo
>