From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <netdev-owner@vger.kernel.org>
Received: from mail-pl0-f53.google.com ([209.85.160.53]:36522 "EHLO
        mail-pl0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
        with ESMTP id S1752490AbeCWNhd (ORCPT
        <rfc822;netdev@vger.kernel.org>); Fri, 23 Mar 2018 09:37:33 -0400
Received: by mail-pl0-f53.google.com with SMTP id 61-v6so7450870plf.3
        for <netdev@vger.kernel.org>; Fri, 23 Mar 2018 06:37:33 -0700 (PDT)
Subject: Re: [bpf-next V5 PATCH 11/15] page_pool: refurbish version of
 page_pool code
To: Jesper Dangaard Brouer <brouer@redhat.com>, netdev@vger.kernel.org,
        =?UTF-8?B?QmrDtnJuVMO2cGVs?= <bjorn.topel@intel.com>,
        magnus.karlsson@intel.com
Cc: eugenia@mellanox.com, Jason Wang <jasowang@redhat.com>,
        John Fastabend <john.fastabend@gmail.com>,
        Eran Ben Elisha <eranbe@mellanox.com>,
        Saeed Mahameed <saeedm@mellanox.com>, galp@mellanox.com,
        Daniel Borkmann <borkmann@iogearbox.net>,
        Alexei Starovoitov <alexei.starovoitov@gmail.com>,
        Tariq Toukan <tariqt@mellanox.com>
References: <152180742196.20167.5168801400337773178.stgit@firesoul>
 <152180753479.20167.856688163861554435.stgit@firesoul>
From: Eric Dumazet <eric.dumazet@gmail.com>
Message-ID: <9c7b996c-48ea-bd54-1328-4682ec6bd4cb@gmail.com>
Date: Fri, 23 Mar 2018 06:37:30 -0700
MIME-Version: 1.0
In-Reply-To: <152180753479.20167.856688163861554435.stgit@firesoul>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: netdev-owner@vger.kernel.org
List-ID: <netdev.vger.kernel.org>



On 03/23/2018 05:18 AM, Jesper Dangaard Brouer wrote:


> +#define PP_ALLOC_CACHE_SIZE	128
> +#define PP_ALLOC_CACHE_REFILL	64
> +struct pp_alloc_cache {
> +	u32 count ____cacheline_aligned_in_smp;
> +	void *cache[PP_ALLOC_CACHE_SIZE];
> +};
> +
> +struct page_pool_params {
...
> +};
> +#define	PAGE_POOL_PARAMS_SIZE	offsetof(struct page_pool_params, end_marker)
> +
> +struct page_pool {
> +	struct page_pool_params p;
> +
> +	struct pp_alloc_cache alloc;
> +
>...
> +	struct ptr_ring ring;
> +
> +	struct rcu_head rcu;
> +};
> +


The placement of ____cacheline_aligned_in_smp in pp_alloc_cache is odd.

I would rather put it in struct page_pool as in :

+struct page_pool {
+	struct page_pool_params p;

+	struct pp_alloc_cache alloc ____cacheline_aligned_in_smp;;

To make clear the intent here (let the page_pool_params being in a read only cache line)

Also you probably can move the struct rcu_head  between p and pp_alloc_cache_alloc to fill a hole.

(assuming allowing variable sized params is no longer needed)