Received-SPF: pass (google.com: domain of linux-kernel+bounces-21056-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240103095650.25769-1-linyunsheng@huawei.com>
 <20240103095650.25769-4-linyunsheng@huawei.com> <f4abe71b3439b39d17a6fb2d410180f367cadf5c.camel@gmail.com>
 <74c9a3a1-5204-f79a-95ff-5c108ec6cf2a@huawei.com> <CAKgT0Uf=hFrXLzDFaOxs_j9yYP7aQCmi=wjUyuop3FBv2vzgCA@mail.gmail.com>
 <f138193c-30e0-b1ba-1735-5f569230724b@huawei.com>
In-Reply-To: <f138193c-30e0-b1ba-1735-5f569230724b@huawei.com>
From: Alexander Duyck <alexander.duyck@gmail.com>
Date: Tue, 9 Jan 2024 07:37:46 -0800
Message-ID: <CAKgT0UcujEktOnHx7mxWd+Jah1J9mHFWnTx35vc3x25uUadxaA@mail.gmail.com>
Subject: Re: [PATCH net-next 3/6] mm/page_alloc: use initial zero offset for page_frag_alloc_align()
To: Yunsheng Lin <linyunsheng@huawei.com>
Cc: davem@davemloft.net, kuba@kernel.org, pabeni@redhat.com, 
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org, 
	Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Jan 9, 2024 at 3:22=E2=80=AFAM Yunsheng Lin <linyunsheng@huawei.com=
> wrote:
>
> On 2024/1/9 0:25, Alexander Duyck wrote:
> > On Mon, Jan 8, 2024 at 12:59=E2=80=AFAM Yunsheng Lin <linyunsheng@huawe=
i.com> wrote:
>
> ...
>
> >
> >>>
> >>> 2. By starting at the end and working toward zero we can use built in
> >>> functionality of the CPU to only have to check and see if our result
> >>> would be signed rather than having to load two registers with the
> >>> values and then compare them which saves us a few cycles. In addition
> >>> it saves us from having to read both the size and the offset for ever=
y
> >>> page.
> >>
> >> I suppose the above is ok if we only use the page_frag_alloc*() API to
> >> allocate memory for skb->data, not for the frag in skb_shinfo(), as by
> >> starting at the end and working toward zero, it means we can not do sk=
b
> >> coalescing.
> >>
> >> As page_frag_alloc*() is returning va now, I am assuming most of users
> >> is using the API for skb->data, I guess it is ok to drop this patch fo=
r
> >> now.
> >>
> >> If we allow page_frag_alloc*() to return struct page, we might need th=
is
> >> patch to enable coalescing.
> >
> > I would argue this is not the interface for enabling coalescing. This
> > is one of the reasons why this is implemented the way it is. When you
> > are aligning fragments you aren't going to be able to coalesce the
> > frames anyway as the alignment would push the fragments apart.
>
> It seems the alignment requirement is the same for the same user of a pag=
e_frag
> instance, so the aligning does not seem to be a problem for coalescing?

I'm a bit confused as to what coalescing you are referring to. If you
can provide a link it would be useful.

The problem is page_frag is a very generic item and can be generated
from a regular page on NICs that can internally reuse the same page
instance for multiple buffers. So it is possible to coalesce page
frags, however it is very unlikely to be coalescing them in the case
of them being used for skb buffers since it would require aligned
payloads on the network in order to really make it work without
hardware intervention of some sort and on such devices they are likely
allocating entire pages instead of page frags for the buffers.