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 D4AE1ECAAD2 for ; Mon, 29 Aug 2022 19:51:40 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 381FA6B0072; Mon, 29 Aug 2022 15:51:40 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 3312B6B0074; Mon, 29 Aug 2022 15:51:40 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1F89C940007; Mon, 29 Aug 2022 15:51:40 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 10CBF6B0072 for ; Mon, 29 Aug 2022 15:51:40 -0400 (EDT) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id D2F1F40A23 for ; Mon, 29 Aug 2022 19:51:39 +0000 (UTC) X-FDA: 79853675118.05.607487F Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by imf30.hostedemail.com (Postfix) with ESMTP id 6070480057 for ; Mon, 29 Aug 2022 19:51:39 +0000 (UTC) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ams.source.kernel.org (Postfix) with ESMTPS id 5FA16B811F5; Mon, 29 Aug 2022 19:51:37 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id A8C33C433D6; Mon, 29 Aug 2022 19:51:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=linux-foundation.org; s=korg; t=1661802696; bh=3VYkqk3VFoG/vKpioLycubxC4ZQf50jtJOntBowpvHg=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=h300GN4ysd1qtqvDuwLs8vadtrSoMxdUQ4dAmbqIe8JPsydXnOnUmq8tauBPf2iNm LyArQsObh3c3iJJ3AOQDyZuboV5ZTuAAr2jiP4tLLC+H1oHftJM+dd8r2Rxlf/WPdj CkfqKO7MQJjL5EDK4UC67LM9C6BMBe/3hRPi/990= Date: Mon, 29 Aug 2022 12:51:34 -0700 From: Andrew Morton To: Qi Zheng Cc: willy@infradead.org, vbabka@suse.cz, hannes@cmpxchg.org, minchan@kernel.org, rppt@kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/7] mm: introduce common struct mm_slot Message-Id: <20220829125134.9b05f9b8caf5da4bec8f31e8@linux-foundation.org> In-Reply-To: <20220829143055.41201-2-zhengqi.arch@bytedance.com> References: <20220829143055.41201-1-zhengqi.arch@bytedance.com> <20220829143055.41201-2-zhengqi.arch@bytedance.com> X-Mailer: Sylpheed 3.7.0 (GTK+ 2.24.33; x86_64-redhat-linux-gnu) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b=h300GN4y; spf=pass (imf30.hostedemail.com: domain of akpm@linux-foundation.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org; dmarc=none ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1661802699; a=rsa-sha256; cv=none; b=UA6UhthVs+sUnzaVZyl39tJ1jtQzGtn3VNLAP8MrBOvBK7GGhr4P+7SBEvmWS97+hjkcPu GPm470mZlNZn1UURs0Ef23rWyM36haY2pfM+oDm7q36JzqzRr0yLDMWpO6b24cF5oTL6Z0 QyVKlQ9/cRb9ZSL/97Rwkm7WQE4BOpw= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1661802699; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=xpKqQJm4RaUr1JB7IMxc4hMG51G6VfSh7MgkjAynKSs=; b=46JLlSUP7rNo0I6p+JPf7HA1dy/PzldwAHCziXU+/RTz8+iQjM+uvoerHs4F9BnABVud7p S+m8OX1/sUWwiOGZ+SnliM2YhJsma919cNthLBOah1jiZycNy+W07ZGFslEp27JNBNsyIa uOrcDahWOb3qHr338SGeaSBqWG5M9o4= X-Stat-Signature: 7ctj5jhdarduzsdrcik949iwt1i6big7 X-Rspam-User: Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=linux-foundation.org header.s=korg header.b=h300GN4y; spf=pass (imf30.hostedemail.com: domain of akpm@linux-foundation.org designates 145.40.68.75 as permitted sender) smtp.mailfrom=akpm@linux-foundation.org; dmarc=none X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: 6070480057 X-HE-Tag: 1661802699-99620 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, 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. 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.