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;
Date:   Wed, 23 Nov 2022 20:17:05 +0800
From:   Feng Tang <feng.tang@intel.com>
To:     Vlastimil Babka <vbabka@suse.cz>,
        Andrew Morton <akpm@linux-foundation.org>
CC:     Christoph Lameter <cl@linux.com>,
        Pekka Enberg <penberg@kernel.org>,
        David Rientjes <rientjes@google.com>,
        Joonsoo Kim <iamjoonsoo.kim@lge.com>,
        Roman Gushchin <roman.gushchin@linux.dev>,
        Hyeonggon Yoo <42.hyeyoo@gmail.com>,
        Andrey Konovalov <andreyknvl@gmail.com>,
        Dmitry Vyukov <dvyukov@google.com>,
        Andrey Ryabinin <ryabinin.a.a@gmail.com>,
        "Alexander Potapenko" <glider@google.com>,
        Vincenzo Frascino <vincenzo.frascino@arm.com>,
        <linux-mm@kvack.org>, <kasan-dev@googlegroups.com>,
        <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH -next 1/2] mm/slab: add is_kmalloc_cache() helper macro
Message-ID: <Y34PQan6HOucgj4M@feng-clx>
References: <20221121135024.1655240-1-feng.tang@intel.com>
 <20221121121938.1f202880ffe6bb18160ef785@linux-foundation.org>
 <Y3xeYF5NipSbBFSZ@feng-clx>
 <74d14df1-faa7-dc12-d406-ba038682e134@suse.cz>
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <74d14df1-faa7-dc12-d406-ba038682e134@suse.cz>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?dFNHTWVQNVZMZ2MvRzkvTzJqL3VkME1pNVBBemdEc1RVYzQ1YllzK0M5TEJC?=
 =?utf-8?B?bGJSSmRhTkhZWVEycU5sMGx0cmdGUGEreWxJVmtkQWJiODZoemVHb01yL05G?=
 =?utf-8?B?akhIZHhZTEJlN3RmMExqSmhwcDhWc2tEWERycXdDZGZCU2RXZFl5ZXgvK3dW?=
 =?utf-8?B?QTNjaC9MR1h5S0JwT2dmY01EbXlZNEdHREJYK0FLckZGSU9DWU9LK2cxc0Yr?=
 =?utf-8?B?bTl1UTFFU3EyVHhOWU45OFh0VGF3ODdxUDQyZ3RKa3hqQWdCVG5ISEt1bm9S?=
 =?utf-8?B?QVNONG9oWGtoaXhHdndrdFU4TC9nWmI0NzhQQUYzRnhSSG1GemRhQitOeEpy?=
 =?utf-8?B?UWY0TkR6WjJ1amNyd1RXRGd6WkErNUNCd0xQMjN1ZGVPeDlsZE1Gd0h2TkNu?=
 =?utf-8?B?cWJERCtrWVZua20wZzY1OTVLUnRuRGRheDBzU0o2M1RxMDQxU2puM0JhU3Fa?=
 =?utf-8?B?bEt6Mlg4YVVjNTFIU1JFOTB4TFY4eitpTW13elhneTZPRFhOYkl3aG56WGlu?=
 =?utf-8?B?WWFQMFYzbjVIZlBrTVkyVlM2RjZCaHM4WXYrN1gzTWFoWUFRcmtLQWVtdW44?=
 =?utf-8?B?Nm1KaHFiZDcxelpxSFJWZTd0ajR3ZTVVZ3JNTUllTnpOVmw3RmN0SEFpWTN1?=
 =?utf-8?B?Y0JldkxYZ2psdGdtSmduRmRwZi9JcHhTRnp4bEU5eldlMWh5ZUxhcWNUVSs0?=
 =?utf-8?B?SjgvSFVSbXdkTjkzNGVha3FEQXZKd2xpWXE0RG9MNHowNExMZno5QjFtZG55?=
 =?utf-8?B?dWR0elRiUVZqVk8xdStncjY2K041MVlyQzIrZVZ4cUFEdWNiNEtNVmlBLzV2?=
 =?utf-8?B?VDBZUWZONnljeFh3bGhLa1FsSjc0bkI4b3h6ZkRmOXJXc2djS0lLdk4rRmRh?=
 =?utf-8?B?NEY2RXdNVWZ1dnlwbGhPditDS2VIVThpdi9uTFpsay9uSUxpU1BoY0pHV2FS?=
 =?utf-8?B?cW5SOVdwN0k5VXc3YXloZGJlU0pNQk82UTJnRUFKTFlFWWhWTzZNenNZcUR3?=
 =?utf-8?B?Nkxac2tFUjRhYjdiL3o2M0pEMDdmSS9OdXhIaHE5T2JzMXJMQ0lyUGNyeVFm?=
 =?utf-8?B?ZmhhRXVSWFJ1ZForbzlKWWxpTVRGeThVd2E0MW1YNWNIZEJNR2RWekc2UUNT?=
 =?utf-8?B?U1V3OTdQNkRZbnJRbTZ3cWgwckQwZnd6Q1ZmeGw2NmsvMmpXbEhreWRsRzBw?=
 =?utf-8?B?OUFCYnVOYU9TMmE0OTZLUUhZNC9KZkFDWE1kTWErSG1KQzRnSGtJS1RiemNQ?=
 =?utf-8?B?dGVONlZkQlp6ckpXWHJaM0tteFkzRlFNYWQzK1B2SVpVRXZXTmRPTEJQc0ZH?=
 =?utf-8?B?Qkw4eFN0KzRWZWRwTW9XUTB4Q3RML0VUaXRMbGRnZnYyTkJqZkZNVUdSSWNG?=
 =?utf-8?B?MlNHOTVlNm1Cb2FkTXdqSU5TcjIzMG1yRFpnejI4YllvUDFoN05RYjBTcERX?=
 =?utf-8?B?QjN6YXd6SUhISHdSR3ZUbFJiTzBYbnVtYUppN056VWd2dmNUYnVrejJ5MEtu?=
 =?utf-8?B?M0t5Z0JkRDBMUlAya2xYM1JPYUFINWRsQWUyT216TDVackFRb1ovQ3ltbDVn?=
 =?utf-8?B?M2tiMmhrTkJJRWJhQmVQYnFJbklacHdiemlYV3dJdXI3YmtucGlVVGlBRFli?=
 =?utf-8?B?aHZTRHZhbnp1RGtXcmxvKzN4enZ1dmZ2bHZjbW9Dem9uNkg0MXFYdDQ2VWZM?=
 =?utf-8?B?Vi9KelFpM0RXZWtIRm5xOFFNWGdyWEYvNGhyWUtkZytMQTBVNm5IK2prV0hz?=
 =?utf-8?B?ck81blFyeDMrMWdVMkFWVExaUmFXRlVXVis2UUNXUVRPLytEWWQ4Qm5Hd3BF?=
 =?utf-8?B?VW1rbzd3K3BQWUtEME8wMmRocGZLdkhiNjJ2UGFGakp3a1dhcENGQ2IramI3?=
 =?utf-8?B?Mk9VdkFoUmlaajVsMTJWVVpuMTI3c2tHQ2VnaHl3T3E2dEcxSjQzM0tFSnBJ?=
 =?utf-8?B?QmdHZEdHSlF4RElLM01VNTg0TnAvbUNLN0hobWlZdmRLM1lNSWFmc2plbW1E?=
 =?utf-8?B?V285Qjg4NHlUOEtCcHN5WDliVmRFOE5NeDJERTlxMnJCdm9GUitZdkJoVjUx?=
 =?utf-8?B?WXh2VUJkOGdCRnVxK09GMEVQQ2lOREdWbkJUUEI0TVMrM05FQUNKenJYbGRN?=
 =?utf-8?Q?+vyjlghcQZd/BMzET6Mn1C2eX?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 8b8cd41c-1e83-45be-00f8-08dacd4d1353
X-MS-Exchange-CrossTenant-AuthSource: MN0PR11MB6304.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Nov 2022 12:20:14.8290
 (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: UCs6aN2lq543Ij+7SsxXSoV5rlI7xRFvyTr1AFubfIRSK4HHfVRfw9/F2Y1KcWBVKuMh3m+V2u6evMyrRH3LQA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ1PR11MB6177
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE
        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

On Wed, Nov 23, 2022 at 10:21:03AM +0100, Vlastimil Babka wrote:
> On 11/22/22 06:30, Feng Tang wrote:
> > On Mon, Nov 21, 2022 at 12:19:38PM -0800, Andrew Morton wrote:
> >> On Mon, 21 Nov 2022 21:50:23 +0800 Feng Tang <feng.tang@intel.com> wrote:
> >> 
> >> > +#ifndef CONFIG_SLOB
> >> > +#define is_kmalloc_cache(s) ((s)->flags & SLAB_KMALLOC)
> >> > +#else
> >> > +#define is_kmalloc_cache(s) (false)
> >> > +#endif
> >> 
> >> Could be implemented as a static inline C function, yes?
> > 
> > Right, I also did try inline function first, and met compilation error: 
> > 
> > "
> > ./include/linux/slab.h: In function ‘is_kmalloc_cache’:
> > ./include/linux/slab.h:159:18: error: invalid use of undefined type ‘struct kmem_cache’
> >   159 |         return (s->flags & SLAB_KMALLOC);
> >       |                  ^~
> > "
> > 
> > The reason is 'struct kmem_cache' definition for slab/slub/slob sit
> > separately in slab_def.h, slub_def.h and mm/slab.h, and they are not
> > included in this 'include/linux/slab.h'. So I chose the macro way.
> 
> You could try mm/slab.h instead, below the slub_def.h includes there.
> is_kmalloc_cache(s) shouldn't have random consumers in the kernel anyway.
> It's fine if kasan includes it, as it's intertwined with slab a lot anyway.
 
Good suggestion! thanks! This can address Andrew's concern and also
avoid extra cost.    

And yes, besides sanity code like kasan/kfence, rare code will care
whether other kmem_cache is a kmalloc cache or not. And kasan code
already includes "../slab.h".

> > Btw, I've worked on some patches related with sl[auo]b recently, and
> > really felt the pain when dealing with 3 allocators, on both reading
> > code and writing patches. And I really like the idea of fading away
> > SLOB as the first step :)
> 
> Can't agree more :)
> 
> >> If so, that's always best.  For (silly) example, consider the behaviour
> >> of
> >> 
> >> 	x = is_kmalloc_cache(s++);
> >> 
> >> with and without CONFIG_SLOB.
> > 
> > Another solution I can think of is putting the implementation into
> > slab_common.c, like the below?
> 
> The overhead of function call between compilation units (sans LTO) is not
> worth it.

Yes. Will send out the v2 patches. 

Thanks,
Feng