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: <ebd284ad-5fa9-2269-c40e-f385420b27c3@intel.com>
Date:   Fri, 21 Jul 2023 17:37:57 +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 v2 7/7] net: skbuff: always try to recycle PP
 pages directly when in softirq
Content-Language: en-US
To:     Yunsheng Lin <linyunsheng@huawei.com>,
        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>,
        Larysa Zaremba <larysa.zaremba@intel.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: <20230714170853.866018-1-aleksander.lobakin@intel.com>
 <20230714170853.866018-10-aleksander.lobakin@intel.com>
 <20230718174042.67c02449@kernel.org>
 <d7cd1903-de0e-0fe3-eb15-0146b589c7b0@intel.com>
 <20230719135150.4da2f0ff@kernel.org>
 <48c1d70b-d4bd-04c0-ab46-d04eaeaf4af0@intel.com>
 <20230720101231.7a5ff6cd@kernel.org>
 <8e65c3d3-c628-2176-2fc2-a1bc675ad607@intel.com>
 <20230720110027.4bd43ee7@kernel.org>
 <988fc62d-2329-1560-983a-79ff5653a6a6@intel.com>
 <b3884ff9-d903-948d-797a-1830a39b1e71@intel.com>
 <20230720122015.1e7efc21@kernel.org>
 <e542f6b5-4eea-5ac6-a034-47e9f92dbf7e@intel.com>
 <20230720124647.413363d5@kernel.org>
 <406885ee-8dd0-1654-ec13-914ed8986c24@huawei.com>
From:   Alexander Lobakin <aleksander.lobakin@intel.com>
In-Reply-To: <406885ee-8dd0-1654-ec13-914ed8986c24@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?ZkVCeVlWNXcyK1dWSC9rdVAzSEtZVVhkWlFaV2xodEtrSHN6TUZLd3czVXpi?=
 =?utf-8?B?K2NZTEpEVXlRQ2JTaG5Lbkk2NUxmUXZPZUdyYld4S1Fxb2NXY1ZNS2FXeEdS?=
 =?utf-8?B?RWZMbGFPbFBSS0xGQkVaOGREcHBaZWdjQkpnSWxWWWU1ZHNOMW4xdzBNUzNk?=
 =?utf-8?B?T0s2b0xLR2UrTk00dVYzVjVXTjhQaFNHSDc3UjMzUkNJeVFlTDFnZS9uK1ZU?=
 =?utf-8?B?c3FRTGExaldrNWJ4ck5aR2xBell0YjNVU3ZrL1hPMFBaMTFJa016b2J4VHJ6?=
 =?utf-8?B?ZEJzNzkxaWVFbVdnMzlTT01NTmE4OXlBSTZ3OHZsa0pVSldwT241NEVIb1NS?=
 =?utf-8?B?UG9qR0lJRlhLU3VKT2ppbDNrZittaWJaVk5VSHB3SjZDN050VFRxOXNyaFRw?=
 =?utf-8?B?SnAxUy96ek5nMys0T1c3aUR5ejQ1TFNsblc3SXlucGE5R1RqMmxNalhuWUVa?=
 =?utf-8?B?WnBZT25PVVhucU5rN3lockdCUFQ5ZVlTcEtDdkxaakhlY0w2TjY5dndWTUJR?=
 =?utf-8?B?NDhFdGttN0ZwSTNOc2NFTTI4a3RFQnFpYlNySk83SS91RkpadUh2VWliQzFy?=
 =?utf-8?B?cFJ3ZEhlM0FIVEx4alk3UEJ1TTlMSWdFUnpIUzdaeVRRa0UvZmczUU9rUGd4?=
 =?utf-8?B?V3pDQnZwVWx6YWdYbmNQYzcrL0ZKd24yMDVuMXNEeE13a1FtNWR4WjcxcGl4?=
 =?utf-8?B?OXptazJaSU5NNXgvdFpvM3JBTFYyMlhlTEczUHZlQUlNNURKbTNDU0cwR2hR?=
 =?utf-8?B?b2VtMytGa3dDbURPT2dnWi9jM1NZemtVayt0NUJzcnBGZEJOU202Q3dHUmQw?=
 =?utf-8?B?alE5UmY2ak5XQU5CeG56Z2NvU3grVlJsTTZrRWZlVzluZ3Z2WWFnYUl0ZUNx?=
 =?utf-8?B?Z2R2QjA4N2dRT3Q0ekF6TjhjcGpVV2tHME9IMUhJbDJnakxTeldDV0VJbytS?=
 =?utf-8?B?UHM0SituTkR4VVhMclprQUsvaytZL3NuNHc5V0x4U2NKUGxsSEZ1Qmo4WmJS?=
 =?utf-8?B?bzVaK1h1UGJzZ3FERFJ0NkR1NmNmZFppS1E4N01sbytValF1dlMzZ2R1WWlw?=
 =?utf-8?B?ZElOcEJ2VVVmL1ZmZUEyL2xqZkpaWFFIYkRXbXJGcU12ekNFR2N3ZGpEVU9l?=
 =?utf-8?B?Sm1PdVlNV3gwQnhXNmhBSnluQkRiSTZ3YjJLTVY0TUlxZHZTb1JjY01GeFYx?=
 =?utf-8?B?L25sSFkwdGZXWW9yeEZoMDN5aTRyaHVPQ0dMRDBlNW5GOGhCTXBNaHNFRWtK?=
 =?utf-8?B?eW0zTVFJNDFycklOUnV5VXpHWW5CTWZ1WFp2bkdINEVuRGp3MlNEYitWdVM5?=
 =?utf-8?B?MmVUWTdTNVplT3B3VCt5WGUyRUQ0U2F0bk5vcWZiRmlXbENBdzd3ZlBQWmd1?=
 =?utf-8?B?N0R0RVkvam9KYjlxaUVCeVFoTlJUV2doL3cvZDdDVUlwSWxUOHdZdkt3dGVn?=
 =?utf-8?B?T0VHQkZkZ3RpTFFJK3NJazFkdmpkak9UQ291ZzRFeVpOb3QzZXF2RDl3RGJG?=
 =?utf-8?B?aURFbVdVTHVmZFo4M3Z5anFtZ2ZON0QyWEt4Yng3OCs0VTNkYmF5a1h2enlV?=
 =?utf-8?B?ODcrUmZXRDg1RUg5UUlOaGVqQnhRcDBBT0JPeFQ3M29QZVA2VWRXOU5GWFRa?=
 =?utf-8?B?cXBjamhRbnBmZVA4RSsyU0FBY2J3NXMzWTdUM0F0ZEN1SHlOWWprc0lBcUh1?=
 =?utf-8?B?a2dweHpQdko4d1pwcmdHa0RiL290b2xSREU4UzdOY2tDTGtHYUhJeWJ5Smxx?=
 =?utf-8?B?bjEyc1F2T0NtKzdOYUczSDhGOEFzL3pBU2tES1hNYWpqSzZsdTVRRVQzVFBS?=
 =?utf-8?B?RVdTWncrbTluZ3BKRWhTRUhvRE5ydG00YVMyKzl3aWd6cTB4QzBJMit2Uy9V?=
 =?utf-8?B?ZkJLbXcyRlRFejVPWGlaamdBeWNGTGppMWd2TTlVU1FVY2pkelFHRTVuMEhJ?=
 =?utf-8?B?UnBtbmgxYWFTcWYzSWdDeWd1VVphWVZBaWxobVRlUStsTG1UWDN1RDVUajBj?=
 =?utf-8?B?Z3MvdVZSWWNwQks2RXlJeERGN1NJd2R1L2NrWEFlZ25mWitUek1xV0M1NXlJ?=
 =?utf-8?B?T3hYZ2FHQllzQ1ZTWUU4U1lYUVlaaWNIM1BZMkcyMTFybGYzTzZNMFNsODZM?=
 =?utf-8?B?UXplNzQ4eHUySStFY0NmU1Z6aG1jWW4yamZzMCs1VnM3MklacnRZRVJaOUhX?=
 =?utf-8?B?dkE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 3ed8dbb8-0b24-46df-2953-08db8a009f2e
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3625.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 21 Jul 2023 15:39:08.1469
 (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: FzzA3ooXE5xnbVyPknrJcbo9XGTjlAi9t+digRc8v3oxF3qztUJOTh+0DJoy+pPgdctokbVIQAhZHw8uX2hpAfcnaI/Nld4lNQq9py/s3mE=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH3PR11MB8210
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_PASS,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, 21 Jul 2023 19:53:33 +0800

> On 2023/7/21 3:46, Jakub Kicinski wrote:
>> On Thu, 20 Jul 2023 21:33:40 +0200 Alexander Lobakin wrote:
>>>> We can as well check
>>>> 	(in_softirq() && !irqs_disabled() && !in_hardirq())
>>>> ?  
>>>
>>> Yes, something like that. Messy, but I see no other options...
>>>
>>> So, I guess you want to add an assertion to make sure that we're *not*
>>> in this:
>>>
>>> in_hardirq() || irqs_disabled()
>>>
>>> Does this mean that after it's added, my patch is sane? :p
>>
>> Well... it's acceptable. Make sure you add a good, informative
>> but concise comment :)
>>
> 
> Does it mean ptr_ring_produce_any() is needed in
> page_pool_recycle_in_ring() too?
> 
> As it is assumed that page pool API can be called in the context with
> irqs_disabled() || in_hardirq(), and force recylcling happens in the
> prt_ring.
> 
> Isn't it conflit with the below patch? as the below patch assume page
> pool API can not be called in the context with irqs_disabled() || in_hardirq():
> [PATCH net-next] page_pool: add a lockdep check for recycling in hardirq
> 
> Or am I missing something obvious here?

No, Page Pool is *not* intended to be called when IRQs are disabled,
hence the fix Jakub's posted in the separate thread.

Thanks,
Olek