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: <7d484879-e256-fb06-45cd-1b61a429e6c2@intel.com>
Date:   Thu, 18 May 2023 15:29:34 +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:     Yunsheng Lin <linyunsheng@huawei.com>
CC:     Jakub Kicinski <kuba@kernel.org>,
        "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>
 <b84ad4ef-a996-4fd7-dc90-3b44f7acaf39@huawei.com>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <b84ad4ef-a996-4fd7-dc90-3b44f7acaf39@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?THBuWnM5L3FWNy92ZUVOWWJvM1Y2WHhJMVVDMHNNV0dsRWx0Tk12Qi9nV05r?=
 =?utf-8?B?T3p0VHU0WWtWQjhvakxLWjZ5V2N4OTBtWWR0ZFM2MUdza1FPcUU4MVFUYUJ4?=
 =?utf-8?B?Sm9xTXJ3NGFGL2xJaHBiajRqWVZDSkorSHUvdkdQMlhMdFo5YlUzR3ZyY2xK?=
 =?utf-8?B?QmM3NTNGamh0Z3RjNm9KcXgyYmpmeVNDaTJjbU51aE8xcjFYdHptaWdhTC84?=
 =?utf-8?B?T1NWOGlUNzlsVHBuKzhVVlpmai93TDh4TlhQMDFTcENtVUNNUUN2VkNsQ01n?=
 =?utf-8?B?Z056NFhJM3h3NnVVY1lscHVMTWVRaXR5dWkzNjlPZ1VUQzhuMzgwWHVPVkdG?=
 =?utf-8?B?L0hIRGUvR20zM3RML2d0TEIybEJvTVJMN1JrMVRyd2ZHNFhkQnVQVzJ3UFp3?=
 =?utf-8?B?dTdqVWdVeVozZ0FNbWJEc25vVmVKbGpzN1VkNHBMaVJjUjg3ZjFNNEdrWWx6?=
 =?utf-8?B?Sk15VnU4MGRBeFp6OExwUW1WWTVmU2M1K0IrdjBrM0tIc2QrdlNNa0JmcTJt?=
 =?utf-8?B?Q3pWRTBmU1kvTnhVL3lWY1hTT2U2czNyRXZNOTJ6b1dEdmtxdFJEb1VPSkxm?=
 =?utf-8?B?UGw1SzNRN1BrcjBxL0s5Mms2Q01xdFluUWNscmIzaGtMSTU2RnZZZVNEQ0FT?=
 =?utf-8?B?RlJQcGN4b3B2eXNQd0NsblZTV3loZkVxdlJiQmpFNWRSbjg2SVJRYU1wWTNS?=
 =?utf-8?B?ek9RdGlrN0RqRHQvWnMwVHlLS3JCWHF1Wmp3QmI3alIza1diV2dxYkN5bGdL?=
 =?utf-8?B?VjZQVWRoQk91Rkpxd1Q4VVhuMk8wMnFDSGlVUW0rK29GWFVhUkRXUlQxZ3hW?=
 =?utf-8?B?NWZtNFp6UmlBYklYNzc2VUJLeisranN5RnViWUp2UUR2bDc3bm5uM3piSUdl?=
 =?utf-8?B?ZTZkaklVMDdNMVN1aFpHQTA0SEV3aHROaTJ6dy8yOE9RQTV4MnhWcGRmRW9x?=
 =?utf-8?B?TkhCYW1FTkFkVHFZKy9ZT0ovSEo3RzQxZy9pcDVjZkVnbVdiOFNXRHBqcjVX?=
 =?utf-8?B?L2YvYWZuTE9GUVNaWXBwLytHTS81RjE3ZHdHUmpQUmpMMzkvWjE2U3lOSzAy?=
 =?utf-8?B?Vm1xTUhzVmovR2Q0NkV6Z29sOWk1S2Rwbm93dXVjMXVGdXJORDNicUpweS9E?=
 =?utf-8?B?cHlLeVhvK053K1N3Nnh2MFNsYUNwTytzRHhldFhxWDhnMUp0SlJ5SWhFbElB?=
 =?utf-8?B?UG5SbUl1V3BuZTZTcXNhTEluUS8xRGJYMGoyUERDM1lLZC9XYlByZkdteURs?=
 =?utf-8?B?ZzFWa3dIQzNLSjJPd1ZqcmRyZEV5U2xWakd3b1E2MUQxYmFVcEV3SHBnOThm?=
 =?utf-8?B?eTB0ankxUUh6dklIbHROS29zcGpsNTZnQjVET2s3Uk1sN2hSOXRhSkRBZnZr?=
 =?utf-8?B?SlNsencxZUlaY3cvWXdYTjhnTmtNK2VlRGZLUEZUTDgzN0gxdDZUZDRxYW5T?=
 =?utf-8?B?OFc1WCsrT0hZandvSjRQVkVaWGZZOGlYODE5WUp3VkNSbmQzSmhHR2NDMk9H?=
 =?utf-8?B?bWpJb0xxK3U5U2hGMEduU0l3eWdGb1VPczdPWklmTzcreXJNWnFxOWEzL2R6?=
 =?utf-8?B?WmV1ZE0yOUN5YVVvaitrT3NLQWo2SDMrZzUwNG4yazR6d2FoVXZmU3BLaHZL?=
 =?utf-8?B?d3MyQ01aZG5IekN3YTFCanoyQWdBM21VdkNvb2wrMDZxenl2TjZVYnhmNEJ3?=
 =?utf-8?B?K29vSHpXWnBtZWRCSWZkMFVVLzF0WGtWaHlGMVRvYnhiL2toMmh0NnRKTkJI?=
 =?utf-8?B?eldkc3JCNTcvOWJOTWd5dzN1ajRxVkdjWWVJL2IrMWw2VEJJWmhnTWNDdHkw?=
 =?utf-8?B?Y1R1dU8wc3JrbEJiNERvNko3UW0walROTnRDQVJteGVWVUo1R0UzeXEvRzlV?=
 =?utf-8?B?eVRQRGtpandtNiszWXE5b3Byd2FUNlV3WllxZURscEJuVGRoaFNJenFrbDdp?=
 =?utf-8?B?QjQyYVNUWDh2YXVrVHhNcWdZNmNOb21ObUhXRHFFcmJhMllhWmUveFl2RXJJ?=
 =?utf-8?B?eDZ0bndxUk4yU2JCKzVkZUJoL3ZYUllsdENpM2Y2VlUwR3dBamJNQVM5M1Zl?=
 =?utf-8?B?c29UVll3RVNZdXVpTlB5VFZXOEV2aXpsS0Y3L0YxOXpQbDZnNHRmZndCS0hM?=
 =?utf-8?B?b2h3WXdrUmpRc1V0Q2hqY0wvN05sV2tjMlRwZDhaYTJPcFVnTUhZNExZcElq?=
 =?utf-8?B?MlE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 6974fba8-be66-499e-183d-08db57a40eea
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:30:34.3686
 (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: XJHNheMwF+VsV0bO1OBOFiy9vZbOEUDSi4WXrjrIzWwORMHdXP9uRUqAaxoLvYqdlPJ+lIxCg9SC3EV/YLTqDvkldroAlS8t91yzwVCtoqs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB5037
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,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: Yunsheng Lin <linyunsheng@huawei.com>
Date: Thu, 18 May 2023 12:54:37 +0800

> On 2023/5/18 12:08, Jakub Kicinski wrote:
>> 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?
> 
> Is dma_need_sync() not reliable until a dma map is called?
> Is there any reason why not just clear PP_FLAG_DMA_SYNC_DEV if
> dma_need_sync() is false without introducing the PP_FLAG_DMA_MAYBE_SYNC
> flag?

We can't just clear the flag, because some drivers don't want PP to
synchronize DMA. Without a new flag, we can't distinguish those two.
Example:

1) Driver doesn't set DMA_SYNC_DEV
2) We check for dma_need_sync() and it returns true
3) As a result, we set DMA_SYNC_DEV, although driver does that on its
   own

Thanks,
Olek