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: <604d4f6c-a6e7-e921-2d9a-45fe46ab9e79@intel.com>
Date:   Fri, 28 Jul 2023 16:14:13 +0200
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH net-next 6/9] page_pool: avoid calling no-op externals
 when possible
Content-Language: en-US
To:     Yunsheng Lin <linyunsheng@huawei.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>,
        Alexander Duyck <alexanderduyck@fb.com>,
        Jesper Dangaard Brouer <hawk@kernel.org>,
        "Ilias Apalodimas" <ilias.apalodimas@linaro.org>,
        Simon Horman <simon.horman@corigine.com>,
        <netdev@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20230727144336.1646454-1-aleksander.lobakin@intel.com>
 <20230727144336.1646454-7-aleksander.lobakin@intel.com>
 <a79cc7ed-5355-ef7d-8865-0ba9673af5c6@huawei.com>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <a79cc7ed-5355-ef7d-8865-0ba9673af5c6@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?VlM2Tkh2d0MzN2k1N1B6alFNR2FSb0RBTStaemR0TzRUMmIxblhSSjNaa3lW?=
 =?utf-8?B?LzRBM3RRY0hJcGkyZ3FSZ2QyR0F0dFFSaURNVDlUd3FXYTJsdnM2OHErbkdF?=
 =?utf-8?B?Z3hvUlM3elFOMTNxZFJ0eVpRcldGU0pwYWZ3M2wrWE5TVG15c2YwdU5NaVFB?=
 =?utf-8?B?WTZ2UnZVM3FvelhlZENOSFZqT1h3cTdlMEJUR3ExTngySG16WUVuMHZEZ1BL?=
 =?utf-8?B?bVVabTYzQmNtVVJsR2Y0M2V5V01LSUZhSENTdFU3ZnVyL2pkMDVuZHNSem1w?=
 =?utf-8?B?Q1JmVmpiVDc3WFdnalF2a1ZVaUFTRXZkVEhBcmJDUEpNekZ2T1ZXVmpTYVFj?=
 =?utf-8?B?dytCaDJQSS9abnpWejQ0VFNBSVQwampoRjA5Sm01ZFFrb0MyblRRR2hBakpn?=
 =?utf-8?B?RXhobHg2U2xyRFN4aG1uamNTSWZWYzhiZEpQWEdKSllLS2hINDVHR3V5andj?=
 =?utf-8?B?OFJwWXkzdis3ai9qQmkvTXZXQnRTa3pRc1hkOTdyKzFOamEycVkxL1orUWll?=
 =?utf-8?B?SCszSTc4UmFUR3hwZjdrOXNvSmV3b01mcjBDbWVRZmZMS2ZvVHBxTzl2NlFS?=
 =?utf-8?B?d1FsTjVMZzlqTFI5QVp5VGovMGhNeU45OGxUTTB1by9LdnNieXJMOU5nRWF0?=
 =?utf-8?B?cFl3OE5LdTdRTFcvbFllNGN3T2JuYVljdEcyVnZOOTk1V1RDYmcrNkQ5TUN4?=
 =?utf-8?B?NFZuS2tLY3E3TWtDemtaS2RVN2RXVk83cm5vVGlLT25TM1ZpYnFRNHQ4cTVm?=
 =?utf-8?B?a3A5QW1IZUd4SVJxdEU0bGFCSWxYcXRGL3V6dXdKUkZFRERVTUpEL3piZUF3?=
 =?utf-8?B?VkhWYWJyZjU0L2V4eEoweFJUSmdvbStBQzh6RWZmY2ptMkRhZ3lETjdZS0ZR?=
 =?utf-8?B?dmNaWWVTWldKNVJXeU5wN2p2MVMzTmlOTG1WQXI1T0lDUjF0d0hOSWV4dVV6?=
 =?utf-8?B?U2RpWHpQR2xwUEhIc1pRQzl4dG9GK3p1VTZWSEFvTkZLUFVodms0SGIzd2JK?=
 =?utf-8?B?OTR1T09IZW9sM3dyZzJzM3pqaGdSMlNLSGtuQlkrZVllSDVQNDlodDdmNWFj?=
 =?utf-8?B?NGlNeFVCRXN1dFZCeDhsem01UU84VUVkcGtKeHpMZGRjWGhvTUVHSHdXVExM?=
 =?utf-8?B?NWF4Q0oxYkcrWVhRQWZtTUtFMHhzUjJWL0xEbmZWU0FrL2JRWWYrWmpORWNM?=
 =?utf-8?B?TWdzaU9TdFdYUysxRnJFSzU1L3E3VGhYOUJsYytGM2hVMDc5NG1Ud01saFBP?=
 =?utf-8?B?b3JSNFcrT1dYcVA2cTBJT3loNXRsMFRBOGdCZmg3Zk1pbXdYTkJPQUYyRGVK?=
 =?utf-8?B?U3JyRXBDckVld0NYSjZaTFVrUWIvYTlSczlVeGVGTk10UEd2WXZjVUZVemhC?=
 =?utf-8?B?UkdDaGk2YTVmaEtMZEF2VityOE5kdHVXZlVaZDMzamhpZDk5RFZ0YUlYRW84?=
 =?utf-8?B?UkI2YUVDelkxb2ZreW96anJtcStvZ0dmWXNDb2ZEZnBidTBFM1ZWcGdmQTNF?=
 =?utf-8?B?bFhlSElYNFBjSjlvVVU1bmgvUVZIWjBNWUpYTmpXSGZOdG9nYXN3RHJMVWRs?=
 =?utf-8?B?dVRRdXY1VGE1MlZNem5aU3FmQmRtUVVVMmNDajRLcXI1SWkxci9yY29JWVY0?=
 =?utf-8?B?SWxJekdXeUJGeUliUTNRbDcvc2RXcEJLeXdIblVzM2Uwek5KaVlIWDl3RVJu?=
 =?utf-8?B?ZytXcCtMUTdXR2pvQnFQeFBBV05zaVV2c1VkL2RONldXREk5ZG5sRzNZdHhF?=
 =?utf-8?B?a1M4eEtlRWVIejB1SUZYRWVhNVNyUTJPZ0o4TWh3UkY2dWVHZHhaZmoyVUZT?=
 =?utf-8?B?aWMyNkpKdk9KT0tzUG1qTWxiMzVySjUyNzg5Wk5nWHpEc242bWEyOEFZRHV4?=
 =?utf-8?B?ZmxZalkxaHJEVzgxamZPZGVSNjdKelFuLy82Z2dMZlBiVE1IUUoyZ1FObG5W?=
 =?utf-8?B?QWYzUGNCeG5NemdhTmJMeGJKT3RMUExTcE40Wlkra1BRMWVhV2x2Ky9xVVlu?=
 =?utf-8?B?R1gycWJiVXo4MmY5MUFFNExDdDVtMFc0ZGtLMkdDTXA1OU1tcmJiNzJFZnRO?=
 =?utf-8?B?b2Qyelg2bGZ0aEZ1YXpMWk1qRDNDdkk5S0tnRG9ZTG0xK3VNVHpnbitIdDRo?=
 =?utf-8?B?L0IzRmhVWjVSVE1seU81Nk1qRDZ3MDVMZS9UckRuMGdQQmpZdzA5dWZoeTJW?=
 =?utf-8?B?VGc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 33394bef-d678-4c65-9ea3-08db8f7527f9
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Jul 2023 14:15:55.0157
 (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: XmLrGtjbOM1IH8bpRFvWdmtTeEk6MKrlRNNfU18BUtLq+YQZe19BA1NMUCh2O+isWgTyn4prevfGUPfPb/26VNy7atbYI+PyR3pxT5KoTJE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR11MB8039
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,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,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: Yunsheng Lin <linyunsheng@huawei.com>
Date: Fri, 28 Jul 2023 20:39:24 +0800

> On 2023/7/27 22:43, 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
>>
>> page_pool_put_page()
>>   page_pool_put_defragged_page()                  <- external
>>     __page_pool_put_page()
>>       page_pool_dma_sync_for_device()             <- non-inline
>>         dma_sync_single_range_for_device()
>>           dma_sync_single_for_device()            <- external
>>             dma_direct_sync_single_for_device()
>>               dev_is_dma_coherent()               <- exit
>>
>> For the inline functions, no guarantees the compiler won't uninline them
>> (they're clearly not one-liners and sometimes compilers uninline even
>> 2 + 2). The first external call is necessary, but the rest 2+ are done
>> for nothing each time, plus a bunch of checks here and there.
>> Since Page Pool mappings are long-term and for one "device + addr" pair
>> dma_need_sync() will always return the same value (basically, whether it
>> belongs to an swiotlb pool), addresses can be tested once right after
>> they're obtained and the result can be reused until the page is unmapped.
>> Define the new PP DMA sync operation type, which will mean "do DMA syncs
>> for the device, but only when needed" and turn it on by default when the
>> driver asks to sync pages. When a page is mapped, check whether it needs
>> syncs and if so, replace that "sync when needed" back to "always do
>> syncs" globally for the whole pool (better safe than sorry). As long as
>> the pool has no pages requiring DMA syncs, this cuts off a good piece
>> of calls and checks. When at least one page required it, the pool
>> conservatively falls back to "always call sync functions", no per-page
>> verdicts. It's a fairly rare case anyway that only a few pages would
>> require syncing.
>> On my x86_64, this gives from 2% to 5% performance benefit with no
>> negative impact for cases when IOMMU is on and the shortcut can't be
>> used.
>>
> 
> It seems other subsystem may have the similar problem as page_pool,
> is it possible to implement this kind of trick in the dma subsystem
> instead of every subsystem inventing their own trick?

In the ladder I described above most of overhead comes from jumping
between Page Pool functions, not the generic DMA ones. Let's say I do
this shortcut in dma_sync_single_range_for_device(), that is too late
already to count on some good CPU saves.
Plus, DMA sync API operates with dma_addr_t, not struct page. IOW it's
not clear to me where to store this "we can shortcut" bit in that case.

From "other subsystem" I remember only XDP sockets. There, they also
avoid calling their own non-inline functions in the first place, not the
generic DMA ones. So I'd say both cases (PP and XSk) can't be solved via
some "generic" solution.

Thanks,
Olek