Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <ac4a8761-410e-e8cc-d6b2-d56b820a7888@intel.com>
Date:   Fri, 30 Jun 2023 14:29:52 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH RFC net-next 2/4] net: page_pool: avoid calling no-op
 externals when possible
Content-Language: en-US
To:     Alexander H Duyck <alexander.duyck@gmail.com>
CC:     "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
        Larysa Zaremba <larysa.zaremba@intel.com>,
        Yunsheng Lin <linyunsheng@huawei.com>,
        Alexander Duyck <alexanderduyck@fb.com>,
        "Jesper Dangaard Brouer" <hawk@kernel.org>,
        Ilias Apalodimas <ilias.apalodimas@linaro.org>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20230629152305.905962-1-aleksander.lobakin@intel.com>
 <20230629152305.905962-3-aleksander.lobakin@intel.com>
 <69e827e239dab9fd7986ee43cef599d024c8535f.camel@gmail.com>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <69e827e239dab9fd7986ee43cef599d024c8535f.camel@gmail.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?YzVuWUlDZkRxeVpZa3ZlNHV4UTVhZG5DNHBBUWQveE1ab293WHdmUHdkMzBB?=
 =?utf-8?B?L2VXOW1rM2RSYURLSVRvKzdzMTQwaVRGblBGL0JwdVY2TVN0cTlybE01N1JK?=
 =?utf-8?B?Z0hwakorcnNabnQ3bFdHNnl3WGVKa3oxWWRjdktyckdwWnV3cW1QVWczUXJ3?=
 =?utf-8?B?NnRpdHAvRmxyM284ZlVlQnlQUTF3bHZhUTdZdUs3Z09lWm9xSkVhYk4wcDFv?=
 =?utf-8?B?L1JTOHBLaEZtSy9Ed1ZlVnl2S2s0M2gxYm9MQStWbzloYkhGQnh5bVlBemR6?=
 =?utf-8?B?cFFMa3NxSkdaRVdBbGZSampxUit3SlZTcGRNYjdKcGdpa2xJakg5MVd4cHZG?=
 =?utf-8?B?clRZam5IMFRVT2tRM1dpdVhFZVlSd1pVV2hpYVFZNG9IdXp5MFFQbU0zdU9M?=
 =?utf-8?B?VUROa2JzWk04ZkpQMTkzcDVrczVZNFA0Nml4anVCOWxncGFkdkluTkhpWkRS?=
 =?utf-8?B?bUU1dmNlU2I0aGNlQ295U3VTMHR5Vm8reHRLOVpTa0xXSzFJbkRVOTBwcHBy?=
 =?utf-8?B?eTJMVTExM0F4S25mMVJ4NVp3WmxKNGNCdG4rcmdGY3lPelRlejZqZ1ZGM0lM?=
 =?utf-8?B?WWZ0RkFXejhrOERSVGlFNmVydW00U0RJK1JwU3ZtM0ZWd3d4RnJrWFAzL2xW?=
 =?utf-8?B?WW9QOC8rKytoTzlHdHZsbGlqZ1JaSzNmL0hwTWduUEVBY1MxY3BUZk1BWUph?=
 =?utf-8?B?SncxVmhERk1EeGJyUmo4eDVWTEhXbEQxdEtVNGJaUHlhR0c1ZVBhM0M0aUZO?=
 =?utf-8?B?K1hpWWo0WjFkSzdoZUF3UDdTaEJTZVVNQUhINThxNkFjeW91TDQvalhWYlJ6?=
 =?utf-8?B?VDlhYzJRWkd2QUpJdUMxUG9saTNJUElLamVmMkI4OHUwVERDNjZqT1d3d2lC?=
 =?utf-8?B?Q3hBU2o3RFZYN2VLdUoyR3VmMWxnd0prdmJKVmY4bXpKZGova2VTS0I0cnQr?=
 =?utf-8?B?dlZrV2JocWtQcWY5SThGTHVmZGFFOUtUTS9QTEd6SzZLQXJ5OFZsbGhMUjQ2?=
 =?utf-8?B?RXpjK1BlQ1B2dTVPMll3QVM2Y2dHUE1BN1dNT1ZCS1VmcDFJaGVJTG9LRkFo?=
 =?utf-8?B?bW1QRXJlbExjYU8rM3p6R04rQUJjRWJUWklnaExQMTUydmlMUTZJRk1tR0FE?=
 =?utf-8?B?RUdSMDhNNDdpNXhMZXZ1R21OT05ZTzZjM1ZWMGV4ajZybVh0Ujd0UzhTWjZx?=
 =?utf-8?B?T0FQcEhXN2pudThaZ3NiaUxCU2hPbW05b0IyWEhhQlFxc1JYRFFJL3NHZkZu?=
 =?utf-8?B?MTI4MnhkcFBoUG12VFVQS2JUc3d4azh5d1QzRktrcUFrTzBmN3V3clQveG9z?=
 =?utf-8?B?cUtEYmpnK0JDRUZyc010bXJ2RS94ekdCNStESzRqZGI5S1U0dno2SHJuUGxT?=
 =?utf-8?B?OGhZeUlUTG8vR20yTTd3RzY4Ujl5ZTFaMnU0dGF4c3czS3VSOXVuREFLOTAz?=
 =?utf-8?B?NURCaU4wWlNlRUFYaXFqaDFDK1c0UlBaUEtNLzg4SW8xSzBCL1hSdEJpLzlP?=
 =?utf-8?B?a21VODhrTXRjaE91M1lZdXZZSm9JNEFWR0I4WFY1ajExbnRRU2FZMzdEUUxh?=
 =?utf-8?B?a0RFMURtdUZJZTNxcHozREtGOE1wS3N2R1VoTjEzc2tZMno4eDV3Zmttb1kr?=
 =?utf-8?B?R3NudnY0b2pXSk5icnk0WElhOWNDNHVDaUZIVXBmaysrYUh1VVdNUDZTdnNB?=
 =?utf-8?B?eWg3MG9Qc3UwbjRGb0t5aXA4cDZjVVV3TWdMc3JrdmdRYWFlTWpESXZuRDZV?=
 =?utf-8?B?c0RRS1BnTjhWQU9Mb084WnYzVlFRQ1BUNk5vbHE0Vk93MkNQODNzbFowQ1lZ?=
 =?utf-8?B?WG8xOGFzN2ZsTm5IMGVwbGRaU0RDNFlTNEJxS0NFQmE5ZGpyVENTVC82MjJG?=
 =?utf-8?B?ZUFJUUhsbE1tYXhVRkJ0SklwZFBEUllWUWdWb0tIUDY1d091QS9ka1ZqVVFE?=
 =?utf-8?B?K04yL3JycmI5Q2ozbFJ3RkRnVzlRRmtEcXN3L0JNODMwbDcwTlpTTXRsL0dv?=
 =?utf-8?B?anJ3Vml6RWpEUmZkRnVnR21KOFBsc3JISzBDeG9WTWNXTGRVVFNnOHVMUUJU?=
 =?utf-8?B?UFdkbDVXTWtpSm54Z2dhZmQ2bnpiZVU4ellBKzdzS2lIODJsb0QzNWJoOTR4?=
 =?utf-8?B?RFpvT0tFQjcrR0JZWHIwbDkwMnlja0FMWjdSUGk3Wkt6WlZiLzRjM2hTU1NH?=
 =?utf-8?B?ZGc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: d1c55ecd-487c-4df5-ae2f-08db7965c9f6
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Jun 2023 12:30:29.2439
 (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: jcj7qQqCRnXK0Wya+bNa5I/suR7ILANUIbdMpsBKVbB9Lrhhpf/HcPsFEZZK7i3XIOfI/0XfQKA6ucJRHYpSJewpimEEIJjeEySDt7dqcBs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB5616
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.5 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_MED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,
        URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

From: Alexander H Duyck <alexander.duyck@gmail.com>
Date: Thu, 29 Jun 2023 09:45:26 -0700

> On Thu, 2023-06-29 at 17:23 +0200, Alexander Lobakin wrote:
>> Turned out page_pool_put{,_full}_page() can burn quite a bunch of cycles
>> even when on DMA-coherent platforms (like x86) with no active IOMMU or
>> swiotlb, just for the call ladder.
>> Indeed, it's

[...]

>> @@ -341,6 +345,12 @@ static bool page_pool_dma_map(struct page_pool *pool, struct page *page)
>>  
>>  	page_pool_set_dma_addr(page, dma);
>>  
>> +	if ((pool->p.flags & PP_FLAG_DMA_MAYBE_SYNC) &&
>> +	    dma_need_sync(pool->p.dev, dma)) {
>> +		pool->p.flags |= PP_FLAG_DMA_SYNC_DEV;
>> +		pool->p.flags &= ~PP_FLAG_DMA_MAYBE_SYNC;
>> +	}
>> +
>>  	if (pool->p.flags & PP_FLAG_DMA_SYNC_DEV)
>>  		page_pool_dma_sync_for_device(pool, page, pool->p.max_len);
>>  
> 
> I am pretty sure the logic is flawed here. The problem is
> dma_needs_sync depends on the DMA address being used. In the worst case
> scenario we could have a device that has something like a 32b DMA
> address space on a system with over 4GB of memory. In such a case the
> higher addresses would need to be synced because they will go off to a
> swiotlb bounce buffer while the lower addresses wouldn't.
> 
> If you were to store a flag like this it would have to be generated per
> page.

I know when DMA might need syncing :D That's the point of this shortcut:
if at least one page needs syncing, I disable it for the whole pool.
It's a "better safe than sorry".
Using a per-page flag involves more changes and might hurt some
scenarios/setups. For example, non-coherent systems, where you always
need to do syncs. The idea was to give some improvement when possible,
otherwise just fallback to what we have today.

Thanks,
Olek