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: <f7a082ae-5ec8-484b-b602-559f6104c9d7@intel.com>
Date:   Thu, 18 May 2023 15:34:44 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [PATCH net-next 06/11] net: page_pool: avoid calling no-op
 externals when possible
Content-Language: en-US
To:     Jakub Kicinski <kuba@kernel.org>
CC:     "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Paolo Abeni <pabeni@redhat.com>,
        Maciej Fijalkowski <maciej.fijalkowski@intel.com>,
        Magnus Karlsson <magnus.karlsson@intel.com>,
        Michal Kubiak <michal.kubiak@intel.com>,
        Larysa Zaremba <larysa.zaremba@intel.com>,
        Jesper Dangaard Brouer <hawk@kernel.org>,
        "Ilias Apalodimas" <ilias.apalodimas@linaro.org>,
        Christoph Hellwig <hch@lst.de>, <netdev@vger.kernel.org>,
        <intel-wired-lan@lists.osuosl.org>, <linux-kernel@vger.kernel.org>
References: <20230516161841.37138-1-aleksander.lobakin@intel.com>
 <20230516161841.37138-7-aleksander.lobakin@intel.com>
 <20230517210804.7de610bd@kernel.org>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <20230517210804.7de610bd@kernel.org>
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?ZVVOWmRlMVozTUJQYWhRYnNLK0lXSngvQ0JlWmJ0dGRSQ1E5a1JZdUZvT1pw?=
 =?utf-8?B?emk0NDFaQTMvMFJNZTZ4ajlSZ2FvMEhKc1VJL3pFOFB4ZkFmZ0RZbUk4S29a?=
 =?utf-8?B?aW81YzZsYmtxeEJTaVZnck1DdlBBbmVrR2VsWkNwNHB1QjliS1ZxZVZRRTBP?=
 =?utf-8?B?SlM2SEFzTkQxSzZiaW9nNFBtT055TDUvTE9BSFE5WHpIWVNEWGdReVhoQ2NC?=
 =?utf-8?B?bUplSHZsRWZQbTllRzhUTDFxRmhxNTFLTHJidjlQZzV2NlFqQnFFU3NFYzlM?=
 =?utf-8?B?Y1lHU1I3RUJrNXlnZk9PaGd2N3laR3JqUTZMRm50T3VXbzRBN2JlSmRRZXk5?=
 =?utf-8?B?NFp0b2JQRmlZdVhiWlRIQjQ1SWFBRThzd3RYb0E2Zmhoa2lQN1pXUXBxbjQ0?=
 =?utf-8?B?b1pUSWpReDlMM3UxTG4xNGhpWi85dFZmeit5K2JLemxpZHhEUHBBNUZaNENS?=
 =?utf-8?B?Y1cyaGVybnV1VXpoVS95MlhXVDFLbWkrQytibEdVbTdZWmpid0t3N3BUNmxz?=
 =?utf-8?B?b2toaWZ2OXJ2QkZsYnNkbkx0aVdXUFF1TnRtd0paOTJEdG1PeWxLTVZxWVY0?=
 =?utf-8?B?cVNpYzFDK2FQQWxibnh3VlVUUTFlSFBaN3R3U0pqcGREdjcyVTQyQmx4UUhm?=
 =?utf-8?B?QzYzZitJUDVNaERIamYyRUE2eE0xNUQ1K1JDamJYYThRVVNmcityc0Rja3lz?=
 =?utf-8?B?WkFjRnFvR1ZnZ2RPR1lmN2ZiSEdpTjJ3QkhhUWE2N3FJTkhVd1AzYmNGcXBC?=
 =?utf-8?B?dmZ5V3FQYUZzekVpc1FmMEhTblYrNysyL3JEcDAzOEczZjNydmhWOHVBRGla?=
 =?utf-8?B?YlZsc1BJQ3QvZ2hkZUdzRWZSMEpSbkFHYUE3bko4MnBnc2NsYVE5bnhsQUZq?=
 =?utf-8?B?emc2UHFyb1YzTndRKzFiMlk0OWlYdE1kNUUvelovMnU5WS9kMS9KVEE4MVJB?=
 =?utf-8?B?Sk5LOXl4MzF4UXlMc1FlejhKM3Y3ek8zNzE5THd5Tkc4M0c0T1pNMXZNSTE3?=
 =?utf-8?B?Z0IyQUQ1WnQyc1FTSzhrQ3k4dEFQN1FXcjAyRDNSY3M5eUJiMmtDTEFjSlo5?=
 =?utf-8?B?Nk9MSFo4RFZwMnB3RHErSk43M3BBTVhORFlZV2ZCQm1IdklXN1kvb0h1ck1w?=
 =?utf-8?B?Y3dyVHBhYm9RNjhNTVFzRFJLUWluNFZqNDdUUks2MytEVEQ4cEROKzNiS3lh?=
 =?utf-8?B?QjRhOEhlUzQ4ZHlva3oySzh0Y3MwbnYwVzdoblN1SWlzZFZrR0RncWl3K0xh?=
 =?utf-8?B?M0J2M2dtaE9sQ2RzSFRURGRuSmUzM3pRV2JNN3RCa0lPSkN3cjJlVFV3Tzhs?=
 =?utf-8?B?NG1ITy9ET2Era0ltUGJmMlhvWVo5TVNxdms4WlgxZExLU2hnMTlhZndNZGk2?=
 =?utf-8?B?Q1dYMU5McnBDRlBYYVlGZ3NqSTl0Q0xZL1RrVFlBQTZUMUNVWTJ3MmhjbVpH?=
 =?utf-8?B?cVNxY2NTSGpEU3VKdWJmMHY3NGsydEdsNVhnTlAwNU1HQXhrQThlNGE0RGI4?=
 =?utf-8?B?OXlmWFBsQnZOVDlpZFFGdVlKM1dwWDV3dGlnQkd3WklKZFNPbUhzM1MyOExW?=
 =?utf-8?B?WHZkOTNuR3Z4Um1mdWE5SUpjWTZhQTZjekdoeHFCQVhEUzBhRHZmY1o5WThW?=
 =?utf-8?B?eGpWYTk5bnk1Z3R3N1JKdTg3YmRRSjQ5aW44ZExGRkhWd1FKT2IwRlZVb2wr?=
 =?utf-8?B?MFdkaWFnQUpKRUxHUEx2UGVTSnFtdU1pSkdLSldmVFJ4anZ6ZXdqWXpha0dt?=
 =?utf-8?B?NlpUUUo2MjVhdVpLdzJSaFZsWmpwaHVFR3lsQytUcm5ZbXNBTUt5K0p6RmRW?=
 =?utf-8?B?d1YxdmVFVHh3LzNLN3kvUVo0OVk3cFBmYTdFR3dHZG5DR2hmZC8vU3Vqck9X?=
 =?utf-8?B?Z3NxNEpHL1dxK2NZVkd1eFZSTFVZSE5UWEtzLzdrMGoyZ2FJTGhUZ0hLYmVr?=
 =?utf-8?B?dTBRYjM1eG8wMWVScUJmUUJmRWw5NWRncmJ5QllDMzB4YjlFTDhXcjJLNmdr?=
 =?utf-8?B?UGxCQS9sc095ejhhbUpxYlhQcEZKYURYamRwWGlzRnBYSHlFQ3lEU1VLRGFP?=
 =?utf-8?B?SnNnY3hZL2ZTV2VPejk0Vk5nWEdOT3p2Nm5majl1dk5Xa3h2bVp6cGJ1aCtm?=
 =?utf-8?B?WEM4TDllT2hvd01QWWh2MHJNQ0QrRStnUkZGNTNWVVowVGZEeWtMdXNtZllS?=
 =?utf-8?B?UGc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 1d2bb44a-b6bf-49a3-5170-08db57a4c7d5
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 May 2023 13:35:44.9578
 (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: +P9BXDtx3nQ2Duv6S7yejD3YIJ0wfYwm7c7qXPU/tEdk2Y/jzLRS8pJY2+bTGTA9cYogEwNGSoc96W/qxu+g+HD7whUS75k+zAl25EH6TZs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB6261
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.0 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,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,
        SPF_NONE,T_SCC_BODY_TEXT_LINE 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: Jakub Kicinski <kuba@kernel.org>
Date: Wed, 17 May 2023 21:08:04 -0700

> On Tue, 16 May 2023 18:18:36 +0200 Alexander Lobakin wrote:
>> +		/* Try to avoid calling no-op syncs */
>> +		pool->p.flags |= PP_FLAG_DMA_MAYBE_SYNC;
>> +		pool->p.flags &= ~PP_FLAG_DMA_SYNC_DEV;
>>  	}
>>  
>>  	if (PAGE_POOL_DMA_USE_PP_FRAG_COUNT &&
>> @@ -323,6 +327,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;
>> +	}
> 
> is it just me or does it feel cleaner to allocate a page at init,
> and throw it into the cache, rather than adding a condition to a
> fast(ish) path?

When recycling is on, not that fast -- new allocations occur relatively
rarely and it's allocations anyway, one `if` doesn't change anything there.

And seems like I didn't get the sentence regarding "allocate and throw"
:s This condition just disables the shortcut if any new page suddenly
requires real DMA syncs (and if it does, then the sync a line below will
take way more time than 1 more `if`  anyway).

Thanks,
Olek