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 601E2ECAAD5 for ; Tue, 30 Aug 2022 03:57:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DAEAA940008; Mon, 29 Aug 2022 23:57:37 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D5D086B0074; Mon, 29 Aug 2022 23:57:37 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C2530940008; Mon, 29 Aug 2022 23:57:37 -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 B4BA26B0073 for ; Mon, 29 Aug 2022 23:57:37 -0400 (EDT) Received: from smtpin10.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 72538A0D1A for ; Tue, 30 Aug 2022 03:57:37 +0000 (UTC) X-FDA: 79854899754.10.58DEE03 Received: from mail-pl1-f176.google.com (mail-pl1-f176.google.com [209.85.214.176]) by imf13.hostedemail.com (Postfix) with ESMTP id 146412000B for ; Tue, 30 Aug 2022 03:57:35 +0000 (UTC) Received: by mail-pl1-f176.google.com with SMTP id j5so6016655plj.5 for ; Mon, 29 Aug 2022 20:57:35 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance-com.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :from:to:cc; bh=gkm9ZF11qnsrXUp6mwF300lhXi7QKePgAYd+Mxulk6k=; b=2Fd+CNOvbQZUVJI5uw9mDQDrU0f0myoKMYM9K6WPSupS2wBXw1tQYLhPYNXAcscIjJ insioEzVOSwIGGtiPdbICGXI3AO+2pyAGrN5X3nNtcjHe/OcAlKey87XfUILA35nD2Sx doF4rhCYcsTzLGI7PXdLK3hOdLEw1AvJmaq9FRWU1mkZRqpzod5dq+BK+oT+yEMBVYRz X3VA4qvKkuiE7+QXQtKbMx0inlmzWFlXe6TNQpX3tuoCCnx82TbMk7rI87BpAGOUzdEw YkriaqTq8SnfII9CPAlLHlYK9CCTrnrur5HKyCGBumLPD2dke99sAkZd5E18x8vXlA4S hXpw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:references:cc:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc; bh=gkm9ZF11qnsrXUp6mwF300lhXi7QKePgAYd+Mxulk6k=; b=G9iylWRF2a/tDb0t/9v+eK4evDgW7WAH3KxgjXEgVS/i8IYvGBIn6QoIirZxnV0c/R wAG135WX3JRhVZlC8curZxX26W6T6gW/xpGAndhaMqQyrKGemiBggFx4rHnhzB+tY1L2 t6frYXxAbKnFOV71K7uqBRlKwU/La1jyLmlUfGIL83yO0nkP9/tGs/i1MStgiDlhEves hH98hTWtmJIF+9N92xQH1latYrQrGks8uS+h6bMmLEPR9DW7bjgPk2u3NERAvqs2gXmw UftwAWTx+8XFGDWHXTdRW+Z/FE1XSROgV818i77PXNwvlpsiR2EBZHJBRKAi1R5KU8ko cnzw== X-Gm-Message-State: ACgBeo3NjdAc5hW0dpzZpXNTFka2sdS9lVcfm5o7rPQQbr/jCarsIREz U3LC38II975f41IOMRi2S2F8OA== X-Google-Smtp-Source: AA6agR5184wQdD03/aZ2LVPrLnbPfcwT3Ga+pxSmo5FI+ZocRGP6fwtukImQ3Q46lKVrLcXny2tFbw== X-Received: by 2002:a17:902:9a44:b0:171:3541:2c75 with SMTP id x4-20020a1709029a4400b0017135412c75mr18957412plv.15.1661831854700; Mon, 29 Aug 2022 20:57:34 -0700 (PDT) Received: from [10.4.115.67] ([139.177.225.244]) by smtp.gmail.com with ESMTPSA id k18-20020aa79732000000b00539aa7f0b53sm418163pfg.104.2022.08.29.20.57.30 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 29 Aug 2022 20:57:34 -0700 (PDT) Message-ID: <0752da43-0e5c-54c9-4c82-bb966ff93b43@bytedance.com> Date: Tue, 30 Aug 2022 11:57:24 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.1.2 Subject: Re: [PATCH 1/7] mm: introduce common struct mm_slot Content-Language: en-US To: Andrew Morton 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 References: <20220829143055.41201-1-zhengqi.arch@bytedance.com> <20220829143055.41201-2-zhengqi.arch@bytedance.com> <20220829125134.9b05f9b8caf5da4bec8f31e8@linux-foundation.org> From: Qi Zheng In-Reply-To: <20220829125134.9b05f9b8caf5da4bec8f31e8@linux-foundation.org> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1661831857; a=rsa-sha256; cv=none; b=joUph52e8xNztveZSjmVmvFOZ4CpjZPYnrh4pDUZQ9Uh1mj12JccFj1euw7uoLZHE3vbPl mis/q+s5gK3A4+njep8HWZKXKc0J+F75cXSDo++0g1opfwGsYxzhbUjfg37LsmWQs1eo44 5ZF3IOzL16ooo77Mz5L8yTG1pzG17jc= ARC-Authentication-Results: i=1; imf13.hostedemail.com; dkim=pass header.d=bytedance-com.20210112.gappssmtp.com header.s=20210112 header.b=2Fd+CNOv; dmarc=pass (policy=none) header.from=bytedance.com; spf=pass (imf13.hostedemail.com: domain of zhengqi.arch@bytedance.com designates 209.85.214.176 as permitted sender) smtp.mailfrom=zhengqi.arch@bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1661831857; 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=gkm9ZF11qnsrXUp6mwF300lhXi7QKePgAYd+Mxulk6k=; b=w2hZrpysXB2KXfy5IRdh7JwJGA5Db3JSLN8HHYaJZX6BgaaqZUUJexwEHyrn0GXkKblnqn DBuNe0X0j45QXslEo91u/ApKSod7QldcNyzgdUV94+ceVATPz3QUDwlhjV7LLDr0onourG /qxzhYzxuALqDxb+kNytvP/fzEeSq70= X-Rspam-User: X-Rspamd-Queue-Id: 146412000B X-Rspamd-Server: rspam03 X-Stat-Signature: eh643e1n4rt7yp43ohwo31jaotpbsunj Authentication-Results: imf13.hostedemail.com; dkim=pass header.d=bytedance-com.20210112.gappssmtp.com header.s=20210112 header.b=2Fd+CNOv; dmarc=pass (policy=none) header.from=bytedance.com; spf=pass (imf13.hostedemail.com: domain of zhengqi.arch@bytedance.com designates 209.85.214.176 as permitted sender) smtp.mailfrom=zhengqi.arch@bytedance.com X-HE-Tag: 1661831855-757311 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 2022/8/30 03:51, 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. Hi, The reason is that khugepaged_exit()/ksm_exit() will be called first in __mmput() to remove mm from the linked list. So it is prevented the mm_struct from getting freed while on the list. > > 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 Since _hashtable is an array name, IIUC, this cannot be passed as a function parameter, so I chose to implement it as a macro. > 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? Ditto. > > > 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. These names are better, will modify to it in the next version. Thanks, Qi -- Thanks, Qi