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 087F5D132AF for ; Mon, 4 Nov 2024 11:16:38 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 5F8FB6B0083; Mon, 4 Nov 2024 06:16:38 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 5A8BB6B0085; Mon, 4 Nov 2024 06:16:38 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 470516B008C; Mon, 4 Nov 2024 06:16:38 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 297646B0083 for ; Mon, 4 Nov 2024 06:16:38 -0500 (EST) Received: from smtpin12.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id B4451406D8 for ; Mon, 4 Nov 2024 11:16:37 +0000 (UTC) X-FDA: 82748158752.12.2B7066D Received: from fanzine2.igalia.com (fanzine.igalia.com [178.60.130.6]) by imf06.hostedemail.com (Postfix) with ESMTP id 097B4180011 for ; Mon, 4 Nov 2024 11:16:12 +0000 (UTC) Authentication-Results: imf06.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=WZDz9AaA; spf=pass (imf06.hostedemail.com: domain of mcanal@igalia.com designates 178.60.130.6 as permitted sender) smtp.mailfrom=mcanal@igalia.com; dmarc=pass (policy=none) header.from=igalia.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1730718813; 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=y9ExVeNjKF0SfMQEmzTjWWX5US8Priue+XLniaZh+Qs=; b=2NUWxDGD8BrM4zAf2svdZawek2kh7sy5kCIlUnZ9QmkxdCJChuBBTk4jmdnF4Lvsi8ptQs IW/eQh2wtFb9AbawnZGuP1lV0Pus6xspXEFBmsdYCuyYgqGuitQz+H4NtwwDKMlbV3G4ff lglaeu/MG7MGGeDw07FkRy7Ok1S39vo= ARC-Authentication-Results: i=1; imf06.hostedemail.com; dkim=pass header.d=igalia.com header.s=20170329 header.b=WZDz9AaA; spf=pass (imf06.hostedemail.com: domain of mcanal@igalia.com designates 178.60.130.6 as permitted sender) smtp.mailfrom=mcanal@igalia.com; dmarc=pass (policy=none) header.from=igalia.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1730718813; a=rsa-sha256; cv=none; b=yKP7YAp+fqXJigfJ/jLA6dUHKvBNnMiVgmk+5zMcmE5Hxy17POsvAxVJQFo7C4zfxZWSsn 4nxoFFOLWh1qx9ClueetVefd2A3sI1SMnooRZsIgetheZWu9NmzwiRqsGXmqgW8ETi6s/5 LLBRRwU07uaBJxfSP+cDskQ/dYVtc0E= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=igalia.com; s=20170329; h=Content-Transfer-Encoding:Content-Type:In-Reply-To:From: References:Cc:To:Subject:MIME-Version:Date:Message-ID:Sender:Reply-To: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=y9ExVeNjKF0SfMQEmzTjWWX5US8Priue+XLniaZh+Qs=; b=WZDz9AaASfdZwB+BONRbuqU3Wv QNpbAAt+sTh/U7VGjUC7WCNg/pCt4bq6dqjddhX15WsAKGPT0XTFc9mggSyeJofShH3kNxIHSMDR7 NyyVndtl8U9meMwJeff94rSB9tqQ0VQw7FYmJiv7TJuVYlqZ7q37NhUNc4VBajprhXAl9An00HU9a PhkS85FDtvp9p4DH1e30Ibe0V3bgPblSlcOh9MZnUD+gefH2t8n9ZXRShMEGxsbYyWCSZDdypIQyM Hj0zggcvqYoBJeAYCdX4uXsmFwFblS1R1iZ/AAFxCL5jvCysNYQPBRZzEQkrCgMcGTfRyLA0GdK4z 90eb4IsA==; Received: from [187.36.213.55] (helo=[192.168.1.103]) by fanzine2.igalia.com with esmtpsa (Cipher TLS1.3:ECDHE_X25519__RSA_PSS_RSAE_SHA256__AES_128_GCM:128) (Exim) id 1t7v46-001WwT-Gt; Mon, 04 Nov 2024 12:16:10 +0100 Message-ID: <06b96a7f-c9bb-4a65-8077-ba10e0ea1e7d@igalia.com> Date: Mon, 4 Nov 2024 08:16:02 -0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v5 3/5] mm: move ``get_order_from_str()`` to internal.h To: Baolin Wang , Jonathan Corbet , Andrew Morton , Hugh Dickins , Barry Song , David Hildenbrand , Ryan Roberts , Lance Yang Cc: linux-mm@kvack.org, dri-devel@lists.freedesktop.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, kernel-dev@igalia.com References: <20241101165719.1074234-2-mcanal@igalia.com> <20241101165719.1074234-5-mcanal@igalia.com> <9d5ce0af-6fca-422f-b1f8-650879f8ff5a@linux.alibaba.com> Content-Language: en-US From: =?UTF-8?Q?Ma=C3=ADra_Canal?= In-Reply-To: <9d5ce0af-6fca-422f-b1f8-650879f8ff5a@linux.alibaba.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 097B4180011 X-Stat-Signature: zrnosyx446ez5u8e6yoi1sgq7epnws5p X-Rspam-User: X-HE-Tag: 1730718972-793869 X-HE-Meta: U2FsdGVkX1/aRhhmutKTTm1QmIcZNJbR6d+ANXfo1b9cD6dOUrt0/r5Cp71tRkGpFTnCA5J9QL0/QUQAc30V7OjRx3abG3k2u96323EvIbaHahQ64TCT/fXqN8C2k+mAHOrlPho3/OdL59PpLGGMCc2DQrW1EMR1zheiwKcTnLcGpfZMo1wMWWLMDaccF/4Ajffi/w9nbHFWeLQ/Rpup0ZWgvG8ruhsZUUXi5tx09T+ifrqCurarcPZMji7uUiQjbC9oNkzWviLH4Mzg2Maq4sDz8CZRtv9XM0YkAH0tonkMfOn4GtVR8ABhpRbJr7G0AvjZpbux+CuJNcU5uvBqEnQRJspc8MQx/Mt4cRmLjeyKSPZ/YyeOVFZzA8Ccl4EjCI+0uZY9t1qK7jS9Ov8JrQdAYqZsmU5HcTipccljZlRgkRZQHj4WNrZVCumexsB8w4YzDuMOcfYzxcKnyBjF9Bu4bpbW0TJbRuW5Acs8qCiVdBlUGDsdBjigW2yCVuzREvwccOi5acxBXnWmSh3QzYxcIJvTmnnJXXlhy6o+oJ+yrUGFKSYzr/Zh5iYpLfQkrvjKQTHxGekN6N3mQ1IqUIvrKSNwhMIFc7soqLMzLUbaZtJcwbXY9JtSRpsTpElYQ37xzpliyKjdWyvXp9xIAt/3l6QWUm4BF2hsxgKh13CH1m0G5VYODbezqaNHhFjt9f757v809dUMTbxC6S0HgB022/xOir/UEFpCtd7TiPnsJVU+L9HlGOdWRdfr0Cks3Eu0AubWEVI4sC3mR7bB3NR4vCAFKnk7gFPLb8hslFUJqrLccnYetAEFZtjM6gz3lTMShONeI4ON08TmW+6hIgJn+9WkFcey/JgKLyTY5F4eWckC7xyNMUBzHxnxQ8xVP9JUQ0ujoQJeQg5iM/mmysWG2NhGyqRAbhZ5hnzNMlt59bohzAl2sQMQHrBOAbKGCeIJIZ9FExLZIE6meod ZhQ8+aUA L1X+g+m8SVFEVtQJStag5Z3QfoFz0aZD+VCljrOo4iWmsPuhkl79BfQbh3al/ua9TWJkGBwhadz8k6OkeuXasA2k5F9phbxPOEjkV3pZeDPuwtZqisS4qSBWmS3gyw9RfPkNcgsd3e4GacYxPPqckOAUgH1Uk5xyjITLf+IRzY6mOjvdcaxzPpx0jsuVQy00e8Gz4P2Er8wmgyhiD+6tHf2VDaeySHPBZquh/bn1s+aYyIMz1OuU91i2idXMfpJA4l4mvDnJ+fGTx8htjON2FJRxN+Q2kDi2wpWAaJkJM21KKUHe3lulOVa1YTZHjhdSNNdicgAkoF6Gv85TumKojwJZK3TYuXcXX/rn/wwsyq/6x+suTaXy+sCrntw+hlmElJQe6v7JJlJSeR6aRLawGLBtaDruDwNA+Szu5hT0yoTweo0m9aW4EhMYLTx6Q9o86LKxR 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: List-Subscribe: List-Unsubscribe: Hi Baolin, On 03/11/24 23:25, Baolin Wang wrote: > > > On 2024/11/2 00:54, Maíra Canal wrote: >> In order to implement a kernel parameter similar to ``thp_anon=`` for >> shmem, we'll need the function ``get_order_from_str()``. >> >> Instead of duplicating the function, move the function to a shared >> header, in which both mm/shmem.c and mm/huge_memory.c will be able to >> use it. >> >> Signed-off-by: Maíra Canal >> --- >>   mm/huge_memory.c | 38 +++++++++++++++----------------------- >>   mm/internal.h    | 22 ++++++++++++++++++++++ >>   2 files changed, 37 insertions(+), 23 deletions(-) >> >> diff --git a/mm/huge_memory.c b/mm/huge_memory.c >> index f92068864469..a6edbd8c4f49 100644 >> --- a/mm/huge_memory.c >> +++ b/mm/huge_memory.c >> @@ -958,26 +958,6 @@ static int __init setup_transparent_hugepage(char >> *str) >>   } >>   __setup("transparent_hugepage=", setup_transparent_hugepage); >> -static inline int get_order_from_str(const char *size_str) >> -{ >> -    unsigned long size; >> -    char *endptr; >> -    int order; >> - >> -    size = memparse(size_str, &endptr); >> - >> -    if (!is_power_of_2(size)) >> -        goto err; >> -    order = get_order(size); >> -    if (BIT(order) & ~THP_ORDERS_ALL_ANON) >> -        goto err; >> - >> -    return order; >> -err: >> -    pr_err("invalid size %s in thp_anon boot parameter\n", size_str); >> -    return -EINVAL; >> -} >> - >>   static char str_dup[PAGE_SIZE] __initdata; >>   static int __init setup_thp_anon(char *str) >>   { >> @@ -1007,10 +987,22 @@ static int __init setup_thp_anon(char *str) >>                   start_size = strsep(&subtoken, "-"); >>                   end_size = subtoken; >> -                start = get_order_from_str(start_size); >> -                end = get_order_from_str(end_size); >> +                start = get_order_from_str(start_size, >> THP_ORDERS_ALL_ANON); >> +                end = get_order_from_str(end_size, THP_ORDERS_ALL_ANON); >>               } else { >> -                start = end = get_order_from_str(subtoken); >> +                start_size = end_size = subtoken; >> +                start = end = get_order_from_str(subtoken, >> +                                 THP_ORDERS_ALL_ANON); >> +            } >> + >> +            if (start == -EINVAL) { >> +                pr_err("invalid size %s in thp_anon boot >> parameter\n", start_size); >> +                goto err; >> +            } >> + >> +            if (end == -EINVAL) { >> +                pr_err("invalid size %s in thp_anon boot >> parameter\n", end_size); >> +                goto err; >>               } > > There are already checks for ‘start’ and ‘end’ below, and will print > error messages if error occurs. So I suspect whether these repeated > checks and error infor are helpful. The idea is to explicitly show to the user which part of the kernel parameter is broke. Instead of saying that something is broken, it is going to return that, for example, "33K" is invalid. Best Regards, - Maíra > > Anyway, I don't have a strong preference. > Reviewed-by: Baolin Wang