Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Message-ID: <8c6d09be-78d0-436e-a5a6-b94fb094b0b3@intel.com>
Date:   Wed, 29 Nov 2023 14:17:26 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [Intel-wired-lan] [PATCH net-next v5 03/14] page_pool: avoid
 calling no-op externals when possible
Content-Language: en-US
To:     Yunsheng Lin <linyunsheng@huawei.com>,
        Christoph Hellwig <hch@lst.de>
CC:     Paul Menzel <pmenzel@molgen.mpg.de>,
        Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
        Jesper Dangaard Brouer <hawk@kernel.org>,
        Larysa Zaremba <larysa.zaremba@intel.com>,
        <netdev@vger.kernel.org>, Alexander Duyck <alexanderduyck@fb.com>,
        Ilias Apalodimas <ilias.apalodimas@linaro.org>,
        <linux-kernel@vger.kernel.org>, Eric Dumazet <edumazet@google.com>,
        Michal Kubiak <michal.kubiak@intel.com>,
        <intel-wired-lan@lists.osuosl.org>,
        David Christensen <drc@linux.vnet.ibm.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        "David S. Miller" <davem@davemloft.net>
References: <20231124154732.1623518-1-aleksander.lobakin@intel.com>
 <20231124154732.1623518-4-aleksander.lobakin@intel.com>
 <6bd14aa9-fa65-e4f6-579c-3a1064b2a382@huawei.com>
 <a1a0c27f-f367-40e7-9dc2-9421b4b6379a@intel.com>
 <534e7752-38a9-3e7e-cb04-65789712fb66@huawei.com>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <534e7752-38a9-3e7e-cb04-65789712fb66@huawei.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RW95Rnc5WEIzUGVPTkZibmErWmN5Z0dqVDVnUENCMENqS0hmL2xRTTQ5bFd2?=
 =?utf-8?B?NmhrSW1TRTVlVnZzVVhKcDdVMFhXaUZ5aXBha0FPWmtUUE1KMnlmR2NpZlg0?=
 =?utf-8?B?S3R3YnE2UlM4WXlOdklWOXVSY0NnYXZZWXdsaUtwRDB4M0swY20zc0o5Y2pT?=
 =?utf-8?B?dVdPV0NBKzQ0a1hWend6WWFoaUg0R3d3QmQwMkNKeXVPeU8yVVlFa3NBZFJL?=
 =?utf-8?B?RXhCaXBKY2svQkx3TDVncU03bmlHaEFJTm9BdFQrNUpsa1NMWHhneEsyZjdZ?=
 =?utf-8?B?Z3NQanlQSjRUWHdvclhMeG84RGIzU28yRnREaHVodzNzamh3RnA2ZlR5YW5L?=
 =?utf-8?B?bU0xaUJJKzFTd1JBbzNIN2l5N0pQTkRqaXVzR0xlOE1CM2QzVmpsZEZ3N253?=
 =?utf-8?B?V21ucDExTUxXa0FYaWQwMWw2bGV1dWtlckt1WkFwajhYdWVybGFNaGxUL20x?=
 =?utf-8?B?Ums0eHpsdENleHB3bEg1aXRwQy9Yczk3eklNVzd5K1R2eDJuZ3BBWUNRQXlp?=
 =?utf-8?B?ZkxWb3JBdVA0UTdpV2gxVkV3WVozeGZyam5ZTVFpcHBxVktLL0tHdXhtdC8y?=
 =?utf-8?B?UjhaT0pGcFlOeDFFNnoxRVdjUVkzUHpJWVdZNURza05CN0prc3NCcHdrdmZU?=
 =?utf-8?B?VnMwVHY1WXBiSmZJWWhNeHRxc2VNN3QwVkp1ZU5QVFdEbk1Mc0RyalJYMnVK?=
 =?utf-8?B?WkxEbUQ2MWdDT3psZzBFS3g3Q2xNQ0NkbXBOWEMybVpDVjAxQW1NMjlqZjNs?=
 =?utf-8?B?ZmF4eTFLclpkdS9jMXVqSnZHWjdsVVI2Z0gzM3RkRFdzTE1NbEhONXpScFI3?=
 =?utf-8?B?QW5iN1I4dWc3dk8rbm9aRGp0emZ3Vm5DUXlMbnBkNG1jaEZOU294eklMdHNO?=
 =?utf-8?B?eWttUE9NNzhxRGR3WG1jUTY3QmVza3ZENUFmUXVKZmkxbXJodE9Da01TMi9M?=
 =?utf-8?B?WTJVOGo5Q3JQUFVtZjZpek80d2NVQXUvRkVsVE0wWWRaTGdIS2IyN2Mzdisv?=
 =?utf-8?B?U2ZpQUsycm8rNVlDb1hYZDUzQVhzT21YeCs5R1NiSFBMMmZqV2JmU0hsRzdH?=
 =?utf-8?B?by81emJpTnVZMGl0QmU2LzBVQ1AzYkxyVzhrTFRjbjRxa25ZSFpnbzd6R2hK?=
 =?utf-8?B?emhFWGdjdy90Y0RlR1pHREh2bHR2ZU9zV2xMenE4bmJzU0tGS2hJem9hMGIv?=
 =?utf-8?B?SjFLZEZaeXZ2dDdERDROYWFMaUsvZFRyWmJJbVRMVVRPVytzNEl1M0w0NXlT?=
 =?utf-8?B?SElMYmw4MmR1VG1QTnR2aHdsQU1SNU4xNTErakE4RS9nVWllMmhKOFJ4Z21V?=
 =?utf-8?B?YmNxYVFpeGJYRWhDQVBCTUgwOHJUV3RiYStYbmZCQmE3ZmVqb3NVVEpZdW8r?=
 =?utf-8?B?WmtqaWpjVmpaZ1MwMk4ybjlDREZpWkFiY2MrMS9rZGVtMEp6N2g2SXg0UTha?=
 =?utf-8?B?LytNMm1oSVV3dmE0NGkwZWg2TTBXa2lTOEZVUnRvZy9lM3lCbjFvdWovcUtj?=
 =?utf-8?B?Tnp2OC9SeGxMVXVUMjkvYlh0cEhUajdIRklmREp4VVc2K1EyMDluOVJUTFFN?=
 =?utf-8?B?UGhMckRYN3pEWkJSQ0tRd2Z0cmFUaHhHUjBJaWtHcUxYNllJOVd2UG53SlI0?=
 =?utf-8?B?blVQazJ3b1d4aUMyVGtIK1pyRXlOc2dxL2xNQ3kxd0o5cGlUUjdwa3JsN2k4?=
 =?utf-8?B?bHhtMzlFTHRzNFR5MCtHQ2R5TUxsbzRjWVQrWjl6ZGYwOEFUSmFpdXZNQnF0?=
 =?utf-8?B?YVIvT2NvODA5Zlc2bmlDQlBYeEx6QVQxVVFsd3BEMW5JWVJTME9FODZqTzM1?=
 =?utf-8?B?Y042ZEtRSXZldWZuTk44UkFNRjJYWSthWERNWWEyQmRiWDhqMjE4empxRU9w?=
 =?utf-8?B?M0NUUmJ4QWRWUUt2MlNtUzlUZzdNSGhtMnk4WUMzNzE2TzVnWTFZZ3hZK1Zt?=
 =?utf-8?B?TVlHN0F5ZE9YNCsvRmZBbUg5WENDRVJhNFNwRzVSaFhUclBLREE0eXpqS05v?=
 =?utf-8?B?Y3FpVVI4aG5McHExSFdGYW12b21BSEZEbFhvcE1FT0w2aVBqWmJ2T2ZXdEZ5?=
 =?utf-8?B?ZUNkbzAzN052clFKaTVtR1MzZ3laOENMTjNyazlXZGVpZFFCT1NrSUxTZGhK?=
 =?utf-8?B?YnlzQTNFUkhFWlJIL3FaWnZMWHNmMk40Vk1JV1pCUUtjcXpSTFQ1VGhPSWVq?=
 =?utf-8?B?cWc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: faa2c98c-0599-47c2-095b-08dbf0dda1c8
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Nov 2023 13:18:09.8217
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: FO2qM7ZXj7PU6SVeOWMXiYXTxZWkoGikLOhWJjon5J9mGCFhki4JuVxzSK/c8AJdXgHazFNASSomzMFrhrjnODrOpCcFTLmmYrDgdSP6xZE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB4788
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Wed, 29 Nov 2023 05:18:34 -0800 (PST)

From: Yunsheng Lin <linyunsheng@huawei.com>
Date: Wed, 29 Nov 2023 11:17:50 +0800

> On 2023/11/27 22:32, Alexander Lobakin wrote:
>>
>> Chris, any thoughts on a global flag for skipping DMA syncs ladder?
> 
> It seems there was one already in the past:
> 
> https://lore.kernel.org/netdev/7c55a4d7-b4aa-25d4-1917-f6f355bd722e@arm.com/T/

It addresses a different problem, meaningless indirect calls, while this
one addresses meaningless direct calls :>
When the above gets merged, we could combine these two into one global,
but Eric wasn't active with his patch and I remember there were some
problems, so I wouldn't count on that it will arrive soon.

> 
>>
>>>
>>>
> 
>>>> +static inline bool page_pool_set_dma_addr(const struct page_pool *pool,
>>>> +					  struct page *page,
>>>> +					  dma_addr_t addr)
>>>>  {
>>>> +	unsigned long val = addr;
>>>> +
>>>> +	if (unlikely(!addr)) {
>>>> +		page->dma_addr = 0;
>>>> +		return true;
>>>> +	}
>>>
>>> The above seems unrelated change?
>>
>> Related. We use page_put_set_dma_addr() to clear ::dma_addr as well
>> (grep for it in page_pool.c). In this case, we don't want
>> dma_need_sync() to be called as we explicitly pass zero. This check
>> zeroes the field and exits as quickly as possible.
> 
> The question seems to be about if we need to ensure the LSB of
> page->dma_addr is not set when page_pool releases a page back to page
> allocator?

But why do we need to call dma_need_sync(0) when freeing a page wasting
CPU cycles on relatively hot path?

> 
>> In case with the call mentioned above, zero is a compile-time constant
>> there, so that this little branch will be inlined with the rest dropped.

Thanks,
Olek