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: <24583594-4ccc-48aa-d468-9325c97df9a4@intel.com>
Date:   Wed, 5 Jul 2023 16:41:11 +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:     Jakub Kicinski <kuba@kernel.org>
CC:     Alexander Duyck <alexander.duyck@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        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>
 <ac4a8761-410e-e8cc-d6b2-d56b820a7888@intel.com>
 <CAKgT0UfZCGnWgOH96E4GV3ZP6LLbROHM7SHE8NKwq+exX+Gk_Q@mail.gmail.com>
 <413e3e21-e941-46d0-bc36-fd9715a55fc4@intel.com>
 <20230703133207.4f0c54ce@kernel.org>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <20230703133207.4f0c54ce@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?d3pvL0xsVHFWWmhtNStrNWc0QnpDckZWVFM4aDlPc0w0TEd4aWxxUkRrRUh1?=
 =?utf-8?B?SWFaMzlYR1o0S1IrS3NNMzZlWjNwekFiVUplRUZFZ3FtRHlGZGFmU20ya1NL?=
 =?utf-8?B?RUkzSmlLNlB1RStiazVHWk82bGhEMDFXamliYlZxSUpTNlRzSGg5dExGemE0?=
 =?utf-8?B?TFRTTXN4Vkk5YXdtbmREQS9NUVdVRWV2L3pYS3RqSC9DTzk3ZXFxWUphaGR1?=
 =?utf-8?B?Zk9SUzhHbTBSMDdFbCtYTVdqTElUZXJ5b2hjZE5DWTA5VnlMSEMxSkh4b2FQ?=
 =?utf-8?B?UnpHVjBKenk5NDBBekVwSStXa1pyYkZFdThzMFFOb2k1WDUwc3IvcWhlZ0JE?=
 =?utf-8?B?UDhsU0ZQRkhTVS9LZTVUM0tJQ08ybW15cDgxQzJiUW13NDliSWxrVTVCalBU?=
 =?utf-8?B?VStZLytpYnBIZ2hpVzJHME5MNjhlQUxnYnM2dlJWaTBIMzEwWGxPbE40aTFH?=
 =?utf-8?B?dUtwSVdxRkhKMHBkTUtZb0RHRTVRQUhpYXJpUHRXejJiaXRiUEJpRnJ5RkNL?=
 =?utf-8?B?eWd5Vk40d0xQNWlzclY1eDR1UmhyZmlQdm1BUU4zd0RSOENyejlLQWl6UEpt?=
 =?utf-8?B?N1NVQjU3ZExxc3dITDJHNkRadFhrYW5Gd0ZpdS9UckFlNzRna3h2ZDdxNm05?=
 =?utf-8?B?SjBDZ0FpeXBnc1NsMzBaMGhJV09kTHhsenduajNWYm9MYUp5ZEdrbUZFR2c3?=
 =?utf-8?B?NkpMZ2Y1RnB5N1l0MGFXSFhlQ1p3bklrY1Facy84RHFQWTdkaTl5WVVoR3hW?=
 =?utf-8?B?c3ZyMVFHSWZZWlFvQVNkZ1VFem1nNmMybkxRK2hxaVhLUmZXNTAvZ0NPaE5F?=
 =?utf-8?B?TDVla1dvMUFqYWVibit6UnlXcnF6V1VNakozelhmbllBcUZNUytKRC9jQ2ll?=
 =?utf-8?B?SDUvL201UWxHYTkzVk5ISkduVUJSY013bjNYYVBzZHEvZEtLMmtBcHZkZVV5?=
 =?utf-8?B?M1p1WEQrZE9IVkltckRKSU1IQmpJRi80VVAvWHMzY1MxbnNKekJzSFF6eXNa?=
 =?utf-8?B?S1p2UkdsSzFSWTFzSVROalVVS2pKU01IYVRXZENCT1c3cFljaXlwREkzUGZ3?=
 =?utf-8?B?UGRXaHAvcWlFenVkSC80ZytUVmtOSko1UG9MNDM2VDhlV1ZNbXBEaU16ZklX?=
 =?utf-8?B?ZU9TWVRlM0hNYXhNV1BpZlhhQXZkL3FNZ1hCbjNPZEs0OThDank5TXFUSWVR?=
 =?utf-8?B?bDh2K054bko4L2s3d2lxUEFHS0sxbXBFSGJFaFRhWVRhTi8xQ0tud0NpcjlH?=
 =?utf-8?B?b29oM2JIZGZyZlY4SHJsQkh1Z3R1bjljc25vbHVYMFhGRE9Ec21ZUVRJNXRh?=
 =?utf-8?B?Q3IzeXlpLzZ3VEd5aURsTXozS2E3anRuMjUrSW1JZitPT2pMb1B6ZkYzRFZ5?=
 =?utf-8?B?S21QOEdxY2I0WTl4QU0wd0dEVW1vSkZzRFFzMHdXVTdGWEFUYnNDOXEyY0x3?=
 =?utf-8?B?SmRveVpqUk5XQ0M5NUJXUDFYTS9PaDI4RUNRNGpvblRkU2V6QjdBYWQ4Y0l6?=
 =?utf-8?B?clk5d0VFWDRnR2pOMGtURFpFdnlrdUhXcmtaZ1dEMEp6Y2NZeVlYdG01VGlo?=
 =?utf-8?B?UGc4V1BDMDJ1Z05heDR4QXF1VGoveUpBYWE1WkUrTkdIYWNHVVJHaklOU0wz?=
 =?utf-8?B?Umdtbi9WNGdoNHNXZzhndFhZakZxV0dOeFJKOERQUmZMV2FiRTFFc3Y2YW15?=
 =?utf-8?B?UVpqZnJmK3cwaGVMTXVZSDNlNXc4YVl5YXhiYWdSang4dWtpQVhXdXNxMTJa?=
 =?utf-8?B?bG9qTkJHR3E1emtqeGIvR3d6TXhONThJWm9FY2hSVXcyNDhoWDlxakFDSHFU?=
 =?utf-8?B?cko5dHF3WFUwZGx6eGpXNGdySzlIV3FzWEdBamZtMzQ5cjBZMFFRMEU5aEt5?=
 =?utf-8?B?QVI5Vlo0clJieGZxKzMvL3B3N1pCNGdJNGY1TlZzenBDVUp4Q0gwNmxvb0FB?=
 =?utf-8?B?SC84OWcxOFRrb2UyMU1DODRpREczTkhFOFpXbjdCSVI1eVZZNzFiTTU5Mnov?=
 =?utf-8?B?QXBDbmYvOTdtcEQ3RHJIRW94ckhKUDFjQzJzTkxLa3pJZkxnSEh3ekJHUzIr?=
 =?utf-8?B?UXJCS0h3WW9JOUlhTVNuSnRDbWVrenV3bDNKSmNjQjFxU0xSS0R3QUFDZlRQ?=
 =?utf-8?B?ME16WVhHYmVtMDRJUnA1bmdyeUJyN0ZCdlhmMVFtcEZ6U1FmcUE5QnNGWmsw?=
 =?utf-8?B?L3c9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 2ccb7fca-8f3a-4dd4-2008-08db7d661055
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Jul 2023 14:42:32.0052
 (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: ZhNKqdgyKtXLdHIRl/cELFl/DtmsHJ5/N8e5VTGuSuAUgsrDvBPZaUgfPwCREr+YRXOrhHoF4Iiz1ugTxb3DRU4yTzqjTUqkIkKqG7XAvW8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR11MB6618
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: Jakub Kicinski <kuba@kernel.org>
Date: Mon, 3 Jul 2023 13:32:07 -0700

> On Fri, 30 Jun 2023 17:34:02 +0200 Alexander Lobakin wrote:
>>> I am not a fan of having the page pool force the syncing either. Last
>>> I knew I thought the PP_FLAG_DMA_SYNC_DEV was meant to be set by the  
>>
>> Please follow the logics of the patch.
>>
>> 1. The driver sets DMA_SYNC_DEV.
>> 2. PP tries to shortcut and replaces it with MAYBE_SYNC.
>> 3. If dma_need_sync() returns true for some page, it gets replaced back
>>    to DMA_SYNC_DEV, no further dma_need_sync() calls for that pool.
>>
>> OR
>>
>> 1. The driver doesn't set DMA_SYNC_DEV.
>> 2. PP doesn't turn on MAYBE_SYNC.
>> 3. No dma_need_sync() tests.
>>
>> Where does PP force syncs for drivers which don't need them?
> 
> I think both Alex and I got confused about what's going on here.
> 
> Could you reshuffle the code somehow to make it more obvious?
> Rename the flag, perhaps put it in a different field than 
> the driver-set PP flags?

PP currently doesn't have a field for internal flags or so, so I reused
the existing one :s But you're probably right, that would make it more
obvious.

1. Driver sets PP_SYNC_DEV.
2. PP doesn't set its internal one until dma_need_sync() returns false.
3. PP-sync-for-dev checks for the internal flag.

Although needs more lines to be changed :D

Thanks,
Olek