From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 33064C0502C for ; Tue, 30 Aug 2022 17:04:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9581F6B0074; Tue, 30 Aug 2022 13:04:11 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 8E0BB6B0075; Tue, 30 Aug 2022 13:04:11 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 75A5C8D0001; Tue, 30 Aug 2022 13:04:11 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 5F4A06B0074 for ; Tue, 30 Aug 2022 13:04:11 -0400 (EDT) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay07.hostedemail.com (Postfix) with ESMTP id 34D42160CB0 for ; Tue, 30 Aug 2022 17:04:11 +0000 (UTC) X-FDA: 79856881902.12.7DA47E0 Received: from mail-pf1-f176.google.com (mail-pf1-f176.google.com [209.85.210.176]) by imf18.hostedemail.com (Postfix) with ESMTP id BF55F1C0044 for ; Tue, 30 Aug 2022 17:04:10 +0000 (UTC) Received: by mail-pf1-f176.google.com with SMTP id q15so6573945pfn.11 for ; Tue, 30 Aug 2022 10:04:10 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date; bh=tQO12dYvngC9mNaN/BZ42zvmagfuPG4RYzzydLVE+lE=; b=V6vkSWMOYajqU3Mfcq2Pmad8Akaj2ymSrKpUstf2pgt/0ju/pSwfcJKg0LuTKJKeNL qfOMS50qg0qRi3eMNIBZrJjssZ6FkrzRq6PZ1Ndl5pDrlPI0meb0aztqL/+jON6YhoeZ MOnn8x6Jk16ZR2fZiceKysykZhJabWRl285AcQOT8gG8oT2qtLwjq8Bb6IfAeHQI04c+ +piAjsjrY9r175gR4M2K9sXPzC/hW7V8GTiI9MD5tFdb7WbsG2/8E9lPWKibcoKQkQQK HmBZqxk2VvybVCvOXbukhy1DHoMC7rMvDeULnnG/HD196fVwGmXY+9psZEWPLHSYhrM0 SyaA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date; bh=tQO12dYvngC9mNaN/BZ42zvmagfuPG4RYzzydLVE+lE=; b=cpRQm8PMUjLHqPl1QO/VKufquvH3k3ZMRMZQfIL47V5FpRV56r+4U2SqSVs12w9GDk wSqHXYA+BHmlKX4ghEOfszaJj408qDNOQ0PhKZUlP9dTK8ExfRvUEjx9FfT++K+5Z43w 1bJ4/A+Qnn557z51/4BUCO5gk67sUctXJMbSSBqAxzhVjy20Zq/M3M8O8r7MfRtoQdZ3 LC+6SrjC7/s4b8Kp9g0AtZaTMj/vP5l39xTcAbVibUsQVn4sfpAk18dHuLWg0ntlXj1h yqIsg1cQt6XcoH5v5i5ePOX5gN7r9SWJS5NVWd9zqbRqBGCevQv8skZGMJgnMJ2+tkRL 4R5g== X-Gm-Message-State: ACgBeo2IHO11/rSJ0wCUKw2BqVppn/Da4tkJ01F5ltEnVfANrfiAPrj7 iHDtDvAVcTuT3fXJDGX5XESbAAyWAmYZrNRBY2Y= X-Google-Smtp-Source: AA6agR4nBOdPWpI0F2tJveHx7dhr+fc4LiC9EapLKBjLe9N+2Ni2DumkG3Dzc/yqbSy1WKIMzy8dgidXDiF0n+qk4eg= X-Received: by 2002:a65:6a05:0:b0:42c:87a0:ea77 with SMTP id m5-20020a656a05000000b0042c87a0ea77mr4463421pgu.75.1661879049643; Tue, 30 Aug 2022 10:04:09 -0700 (PDT) MIME-Version: 1.0 References: <20220829143055.41201-1-zhengqi.arch@bytedance.com> <20220829143055.41201-2-zhengqi.arch@bytedance.com> <20220829125134.9b05f9b8caf5da4bec8f31e8@linux-foundation.org> In-Reply-To: <20220829125134.9b05f9b8caf5da4bec8f31e8@linux-foundation.org> From: Yang Shi Date: Tue, 30 Aug 2022 10:03:57 -0700 Message-ID: Subject: Re: [PATCH 1/7] mm: introduce common struct mm_slot To: Andrew Morton Cc: Qi Zheng , willy@infradead.org, vbabka@suse.cz, hannes@cmpxchg.org, minchan@kernel.org, rppt@kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Content-Type: text/plain; charset="UTF-8" ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=V6vkSWMO; spf=pass (imf18.hostedemail.com: domain of shy828301@gmail.com designates 209.85.210.176 as permitted sender) smtp.mailfrom=shy828301@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1661879050; a=rsa-sha256; cv=none; b=caa+6cCnlGIYPwDmqhKAjELG0ggWtVbsBJbcT/iwh0o2j8mTS8wZC7hhZByJGJXLW39EN/ CBw4Z03X/j8bdfdrRKmwWMSPrSlk5aB9mAJNXzIbSB5NBF3/82BfA1X2ShQYoKLDn0S5uz Gs0eMgfmqmah4iRAeD7z3chAc4o/1YI= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1661879050; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=tQO12dYvngC9mNaN/BZ42zvmagfuPG4RYzzydLVE+lE=; b=PaevNwp+6V4D+QvkshR8Y6H9ZScOqClu4y6dvkPGM63Iwl49oNDr1Z7ljrX4IpFhUnp0Vk QOWMehHtY4TcuAVsz8aPC6U4X/FsqxcxqSkfCrmNCXMZGFi4p+rNmjIO9HNntrcgtB5jN4 8gZt9H27/o2yiiTO4JH0gKRuu/6JzyA= X-Rspam-User: Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=V6vkSWMO; spf=pass (imf18.hostedemail.com: domain of shy828301@gmail.com designates 209.85.210.176 as permitted sender) smtp.mailfrom=shy828301@gmail.com; dmarc=pass (policy=none) header.from=gmail.com X-Rspamd-Server: rspam02 X-Stat-Signature: h5h5xhck1ecig3szkukrnxp8yu5ora3e X-Rspamd-Queue-Id: BF55F1C0044 X-HE-Tag: 1661879050-346493 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Mon, Aug 29, 2022 at 12:51 PM Andrew Morton wrote: > > On Mon, 29 Aug 2022 22:30:49 +0800 Qi Zheng wrote: > > > At present, both THP and KSM module have similar structures > > mm_slot for organizing and recording the information required > > for scanning mm, and each defines the following exactly the > > same operation functions: > > > > - alloc_mm_slot > > - free_mm_slot > > - get_mm_slot > > - insert_to_mm_slots_hash > > > > In order to de-duplicate these codes, this patch introduces a > > common struct mm_slot, and subsequent patches will let THP and > > KSM to use it. > > Seems like a good idea. > > > --- /dev/null > > +++ b/mm/mm_slot.h > > @@ -0,0 +1,55 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +#ifndef _LINUX_MM_SLOT_H > > +#define _LINUX_MM_SLOT_H > > + > > +#include > > +#include > > + > > +/* > > + * struct mm_slot - hash lookup from mm to mm_slot > > + * @hash: link to the mm_slots hash list > > + * @mm_node: link into the mm_slots list > > + * @mm: the mm that this information is valid for > > + */ > > +struct mm_slot { > > + struct hlist_node hash; > > + struct list_head mm_node; > > + struct mm_struct *mm; > > +}; > > It appears that the presence of an mm_struct in the hash list does not > contribute to the mm_struct's refcount? That's somewhat unexpected. I didn't find time to look into the series yet, but when the mm/mm_slot was added to the list, mmgrab() was definitely called if this was not changed by the series. > > It would be helpful to add some words here describing the means by > which a user of mm_slot would prevent the mm_struct from getting freed > while on the list. I assume "caller must maintain a reference on the > mm_struct while it remains on an mm_slot hash list"? > > > +#define mm_slot_entry(ptr, type, member) \ > > + container_of(ptr, type, member) > > + > > +static inline void *alloc_mm_slot(struct kmem_cache *cache) > > +{ > > + if (!cache) /* initialization failed */ > > + return NULL; > > + return kmem_cache_zalloc(cache, GFP_KERNEL); > > +} > > + > > +static inline void free_mm_slot(struct kmem_cache *cache, void *objp) > > +{ > > + kmem_cache_free(cache, objp); > > +} > > + > > +#define get_mm_slot(_hashtable, _mm) \ > > +({ \ > > + struct mm_slot *tmp_slot, *mm_slot = NULL; \ > > + \ > > + hash_for_each_possible(_hashtable, tmp_slot, hash, (unsigned long)_mm) \ > > + if (_mm == tmp_slot->mm) { \ > > + mm_slot = tmp_slot; \ > > + break; \ > > + } \ > > + \ > > + mm_slot; \ > > +}) > > Is there a reason why this must be implemented as a macro? That's > preferable, although this may be overly large for inlining. mm/util.c > might suit. > > > +#define insert_to_mm_slots_hash(_hashtable, _mm, _mm_slot) \ > > +({ \ > > + _mm_slot->mm = _mm; \ > > + hash_add(_hashtable, &_mm_slot->hash, (unsigned long)_mm); \ > > +}) > > Does this need to be a macro? > > > And the naming. Can we please have > > mm_slot_entry > mm_slot_alloc > mm_slot_free > mm_slot_get > mm_slot_insert > > Also, "get" usually implies that a refcout is taken on the obtained > object, so mm_slot_lookup() would be more appropriate. >