From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from m16.mail.163.com (m16.mail.163.com [117.135.210.4]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 0E076385D9F for ; Thu, 14 May 2026 10:11:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=117.135.210.4 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778753472; cv=none; b=LBx1SLJPsjZVa49xzJhQSl53HZW40D6C4W41AdtGZtA+b2DhV7a0syEI+bKbwSKlfNklxwnufTwCyFPGpJX7Z+STwTOb3YVnT3DJIxUgC3lOuyY7KB6w0zayt31HNwLAVVWMMzF0eRKmGttF/ZULH4Ht9S9FmRMeo/gO2mbgIJc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1778753472; c=relaxed/simple; bh=ot7gfLORBkKj1EvYINiProejdn9TDlwPqubhouS9PvQ=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=RXETqfV+6sXhoR+8Chtng9Q9Ksjf3fK9o94ckBgn2yMAL9bCzN+ivqJsZOmnga6eJQ2nBPPsrK2N8D4lhAxBnDIObsBraY26+QMTRxQyROc+3Avmc9wYMLzbOm68W9U5gt/hYqHSVK9VC8l2yfrMJ+Sz9RW0iWdSA51FCpGCuiY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=163.com; spf=pass smtp.mailfrom=163.com; dkim=pass (1024-bit key) header.d=163.com header.i=@163.com header.b=LnSmt210; arc=none smtp.client-ip=117.135.210.4 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=163.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=163.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=163.com header.i=@163.com header.b="LnSmt210" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=From:To:Subject:Date:Message-ID:MIME-Version; bh=H0 BiJi9b4Pe73gtyOaUPtKrNPLIwMUgnAn24gRrwgVI=; b=LnSmt210mtjuTOX/ju RiWUHRgNLih/hj/9rkL1BocOExZD7gheSVUu1NpkPLWvakbLrfC51aR8sYBibaRW gQXdgoK0hfvk3h2hGM1afeMrBqeddMnau8H7kDIWYCkMNMIgDMBQcizhqpafxg93 +iwZWkHcRJ1pmsSLrGPxSqjIo= Received: from ubuntu24-z.. (unknown []) by gzga-smtp-mtada-g0-4 (Coremail) with SMTP id _____wAXcsiLnwVqEuRYBQ--.10592S2; Thu, 14 May 2026 18:10:20 +0800 (CST) From: ranxiaokai627@163.com To: baolin.wang@linux.alibaba.com Cc: akpm@linux-foundation.org, hughd@google.com, leitao@debian.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, ljs@kernel.org, ran.xiaokai@zte.com.cn, ranxiaokai627@163.com Subject: Re: [PATCH v2 1/2] mm: huge_memory: refactor thpsize_shmem_enabled_store() with sysfs_match_string() Date: Thu, 14 May 2026 10:10:19 +0000 Message-ID: <20260514101019.52078-1-ranxiaokai627@163.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <6d6b1949-fb11-4b05-a01a-9427019b3ae0@linux.alibaba.com> References: <6d6b1949-fb11-4b05-a01a-9427019b3ae0@linux.alibaba.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-CM-TRANSID:_____wAXcsiLnwVqEuRYBQ--.10592S2 X-Coremail-Antispam: 1Uf129KBjvJXoWxJF47Jry7CFy3CF45Ww1kGrg_yoWrWr4UpF Z3G34ftFyjqF9rKrZrXF4rtryFqrs3t3W8K347Ga4fJ3Z5ZwnIkFn8Kry09a4kZryxXr4I kr18WasxWa4DArJanT9S1TB71UUUUU7qnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x0JUpa0kUUUUU= X-CM-SenderInfo: xudq5x5drntxqwsxqiywtou0bp/xtbCxg3mM2oFn43z0AAA3A >The subject line should be: "mm: shmem: xxx" Yes, of course. Thanks for your review. >On 5/13/26 5:45 PM, ranxiaokai627@163.com wrote: >> From: Ran Xiaokai >> >> Inspired by commit 82d9ff648c6c ("mm: huge_memory: refactor >> anon_enabled_store() with set_anon_enabled_mode()"), refactor >> thpsize_shmem_enabled_store() using sysfs_match_string(). >> This eliminates the duplicated spin_lock/unlock(), set/clear_bit(), >> calls across all branches, reducing code duplication. >> >> Tested with selftests ./run_kselftest.sh -t mm:ksft_thp.sh, >> all test cases passed. >> >> Signed-off-by: Ran Xiaokai >> --- > >You should document your changes under the '---' for the new version, or >ideally, include a cover letter describing them. Thanks for the reminder! Will do in the next version. >I did some togging and works well (some nits as below). Feel free to add: >Reviewed-by: Baolin Wang >Tested-by: Baolin Wang > >> mm/shmem.c | 94 +++++++++++++++++++++++++++++------------------------- >> 1 file changed, 51 insertions(+), 43 deletions(-) >> >> diff --git a/mm/shmem.c b/mm/shmem.c >> index 3b5dc21b323c..60cb10854f11 100644 >> --- a/mm/shmem.c >> +++ b/mm/shmem.c >> @@ -5526,6 +5526,29 @@ static ssize_t shmem_enabled_store(struct kobject *kobj, >> struct kobj_attribute shmem_enabled_attr = __ATTR_RW(shmem_enabled); >> static DEFINE_SPINLOCK(huge_shmem_orders_lock); >> >> +enum huge_shmem_enabled_mode { >> + HUGE_SHMEM_ENABLED_ALWAYS = 0, >> + HUGE_SHMEM_ENABLED_INHERIT, >> + HUGE_SHMEM_ENABLED_WITHIN_SIZE, >> + HUGE_SHMEM_ENABLED_ADVISE, >> + HUGE_SHMEM_ENABLED_NEVER, >> +}; >> + >> +static const char * const huge_shmem_enabled_mode_strings[] = { >> + [HUGE_SHMEM_ENABLED_ALWAYS] = "always", >> + [HUGE_SHMEM_ENABLED_INHERIT] = "inherit", >> + [HUGE_SHMEM_ENABLED_WITHIN_SIZE] = "within_size", >> + [HUGE_SHMEM_ENABLED_ADVISE] = "advise", >> + [HUGE_SHMEM_ENABLED_NEVER] = "never", >> +}; >> + >> +static unsigned long * const huge_shmem_orders_by_mode[] = { >> + [HUGE_SHMEM_ENABLED_ALWAYS] = &huge_shmem_orders_always, >> + [HUGE_SHMEM_ENABLED_INHERIT] = &huge_shmem_orders_inherit, >> + [HUGE_SHMEM_ENABLED_WITHIN_SIZE] = &huge_shmem_orders_within_size, >> + [HUGE_SHMEM_ENABLED_ADVISE] = &huge_shmem_orders_madvise, >> +}; >> + >> static ssize_t thpsize_shmem_enabled_show(struct kobject *kobj, >> struct kobj_attribute *attr, char *buf) >> { >> @@ -5551,57 +5574,42 @@ static ssize_t thpsize_shmem_enabled_store(struct kobject *kobj, >> const char *buf, size_t count) >> { >> int order = to_thpsize(kobj)->order; >> + int mode, m; >> ssize_t ret = count; >> + int err; >> + bool changed = false; >> + /* Do not override huge allocation policy with non-PMD sized mTHP */ >> + if (mode == HUGE_SHMEM_ENABLED_INHERIT && >> + shmem_huge == SHMEM_HUGE_FORCE && !is_pmd_order(order)) >> + return -EINVAL; >> >> - if (ret > 0) { >> - int err = start_stop_khugepaged(); >> + spin_lock(&huge_shmem_orders_lock); >> + for (m = 0; m < ARRAY_SIZE(huge_shmem_orders_by_mode); m++) { >> + if (m == mode) >> + changed |= !__test_and_set_bit(order, huge_shmem_orders_by_mode[m]); >> + else >> + changed |= __test_and_clear_bit(order, huge_shmem_orders_by_mode[m]); >> + } >> + spin_unlock(&huge_shmem_orders_lock); >> >> + if (changed) { >> + err = start_stop_khugepaged(); >> if (err) >> ret = err; > >Nit: just return err. > >> + } else { >> + /* >> + * Recalculate watermarks even when the mode hasn't changed >> + * to preserve the legacy behavior, as this is always called >> + * inside start_stop_khugepaged(). >> + */ >> + set_recommended_min_free_kbytes(); >> } >> + >> return ret; > >Nit: return count, then you can remove the 'ret' variable. Agreed. I'll wait for more feedback and send a new version.