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 X-Spam-Level: X-Spam-Status: No, score=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B375CC63793 for ; Thu, 15 Jul 2021 19:13:04 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 95B1161405 for ; Thu, 15 Jul 2021 19:13:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S241942AbhGOTPw (ORCPT ); Thu, 15 Jul 2021 15:15:52 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59350 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S242284AbhGOTBv (ORCPT ); Thu, 15 Jul 2021 15:01:51 -0400 Received: from mail-ej1-x632.google.com (mail-ej1-x632.google.com [IPv6:2a00:1450:4864:20::632]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 81FF8C0617A6 for ; Thu, 15 Jul 2021 11:58:14 -0700 (PDT) Received: by mail-ej1-x632.google.com with SMTP id c17so10941058ejk.13 for ; Thu, 15 Jul 2021 11:58:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=soleen.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=fYKgvYLgvtSQW10Pkkm7+cl/2v+kKNbnHVNtsf/foy4=; b=I1P8lxvHTr2xXP69nPZ/+7IW0uj06lsFuq2VivGUARKQjT/5ggKxzGM/n1ztJmatLe TivTuUjTr0xe94W6KYF0CnsoTjtYoNL3fhEORVO+D+sr1IcKbUeKmdOXZtn7JjWlq2lC ndCldBAAjucaHQaVBIzY8bnHI15b61gOPyrQBBhncKqmMeCwI1E+z6z35j/8Ch6o1yHr TQpeoAET9IMB86aBuFqGpt6K+m2BMNet9rvWt2Vzpk6dvEfeFR98AJ2l2qlRE7kmFb2v 1oIBGWdwQpP0w8BVr1FVuGhlDBxr52MIK+pj93nepBC0y4FXPK4MWKHfeKBZpIlbgx2l zpsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=fYKgvYLgvtSQW10Pkkm7+cl/2v+kKNbnHVNtsf/foy4=; b=J7eDwomrJiFWwlbl+dMdFX34nU3kdoggUVJ4z12M3f8sCOHqAHDW6nNLPQOfPelYVS BKgAstNYTibfG3SdF8pKNYFZ0s8C8IlaclW6+WkuH5f11gFaJwZb2D+lt25uljgcK8nj FoNEzos/2oKAeKnQRA8SWZGTJP0qQs3nRNlqdj6jE3tnNEt2ppVecdN9ZYiN/IGKWcPz DI0S5bkINDdgdDD74kC8ZDOyVujaa9r7Yp7ag0DSPdgDYXsAwMQedWUTDNGwKk5NeMI7 3kyvUOsJ76HlbnwIjH+lWTCSVS3gv2EOQVdJBXnMFwZ/uyX9+TteUdrYBJkKzm0pYdBp yAGg== X-Gm-Message-State: AOAM530BuaBoEbEu5CFfVjHhI9l26+OIkf2aX5GnFJJsX3Ra/ZwrFE/E YsmCXBkF9wYyUvtU7HY00xAXdXKLlTO3VCoLJSsOKw== X-Google-Smtp-Source: ABdhPJyJb8MbkcAOF0uAQweRXiBLqmMU0tzuj8g4DHd1+KDxsnz3qffsAdA3JfM/2GRyO1QcVkQx647Qmhs6PVl7pvs= X-Received: by 2002:a17:906:8558:: with SMTP id h24mr7138820ejy.519.1626375493122; Thu, 15 Jul 2021 11:58:13 -0700 (PDT) MIME-Version: 1.0 References: <343394260f599d940cacc37f1dcc0309239ae220.1626371112.git.zhansayabagdaulet@gmail.com> In-Reply-To: From: Pavel Tatashin Date: Thu, 15 Jul 2021 14:57:37 -0400 Message-ID: Subject: Re: [PATCH 1/2] mm: KSM: fix ksm_run data type To: Matthew Wilcox Cc: Zhansaya Bagdauletkyzy , Andrew Morton , Tyler Hicks , linux-mm , LKML Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Jul 15, 2021 at 2:30 PM Matthew Wilcox wrote: > > On Thu, Jul 15, 2021 at 02:21:21PM -0400, Pavel Tatashin wrote: > > On Thu, Jul 15, 2021 at 2:18 PM Matthew Wilcox wrote: > > > > > > On Fri, Jul 16, 2021 at 12:01:01AM +0600, Zhansaya Bagdauletkyzy wrote: > > > > +++ b/mm/ksm.c > > > > @@ -289,7 +289,7 @@ static int ksm_nr_node_ids = 1; > > > > #define KSM_RUN_MERGE 1 > > > > #define KSM_RUN_UNMERGE 2 > > > > #define KSM_RUN_OFFLINE 4 > > > > -static unsigned long ksm_run = KSM_RUN_STOP; > > > > +static unsigned int ksm_run = KSM_RUN_STOP; > > > > > > Should this be an enum instead? > > > > I think "unsigned int" is OK here, as it is exposed as uint to users: > > Documentation/ABI/testing/sysfs-kernel-mm-ksm > > > > /sys/kernel/mm/ksm/run > > > > run: write 0 to disable ksm, read 0 while ksm is disabled. > > > > - write 1 to run ksm, read 1 while ksm is running. > > - write 2 to disable ksm and unmerge all its pages. > > The document is out of date then as it does not mention 'offline'. The offline mode cannot be set externally. In run_store() if (flags > KSM_RUN_UNMERGE) return -EINVAL; > > Also, why does the call to kstrtouint() specify base 10? If it is a > bitmap, then permitting 0x [1] is more natural. I would expect to see > base 0 there. Users can only write 0, 1, or 2, it is not a bitmap from the user's perspective as the user cannot write: '3' . But, I think it is somewhat weird that ksm_run is used as a bitmap internally with KSM_RUN_OFFLINE = 4. Imo, KSM_RUN_OFFLINE should be placed in a separate boolean from "ksm_run". > > [1] or even 0b, although I see that _parse_integer_fixup_radix does not > support the 0b notation. 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 X-Spam-Level: X-Spam-Status: No, score=-10.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5F07FC636C9 for ; Thu, 15 Jul 2021 18:58:16 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id C5A88613DF for ; Thu, 15 Jul 2021 18:58:15 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org C5A88613DF Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=soleen.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 2544A8D00F4; Thu, 15 Jul 2021 14:58:16 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 22A718D00EC; Thu, 15 Jul 2021 14:58:16 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 0CBB78D00F4; Thu, 15 Jul 2021 14:58:16 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0105.hostedemail.com [216.40.44.105]) by kanga.kvack.org (Postfix) with ESMTP id DF46C8D00EC for ; Thu, 15 Jul 2021 14:58:15 -0400 (EDT) Received: from smtpin04.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id BC9EE18027A73 for ; Thu, 15 Jul 2021 18:58:14 +0000 (UTC) X-FDA: 78365732508.04.ED4C889 Received: from mail-ej1-f44.google.com (mail-ej1-f44.google.com [209.85.218.44]) by imf16.hostedemail.com (Postfix) with ESMTP id 72DA5F000091 for ; Thu, 15 Jul 2021 18:58:14 +0000 (UTC) Received: by mail-ej1-f44.google.com with SMTP id oz7so7185265ejc.2 for ; Thu, 15 Jul 2021 11:58:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=soleen.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=fYKgvYLgvtSQW10Pkkm7+cl/2v+kKNbnHVNtsf/foy4=; b=I1P8lxvHTr2xXP69nPZ/+7IW0uj06lsFuq2VivGUARKQjT/5ggKxzGM/n1ztJmatLe TivTuUjTr0xe94W6KYF0CnsoTjtYoNL3fhEORVO+D+sr1IcKbUeKmdOXZtn7JjWlq2lC ndCldBAAjucaHQaVBIzY8bnHI15b61gOPyrQBBhncKqmMeCwI1E+z6z35j/8Ch6o1yHr TQpeoAET9IMB86aBuFqGpt6K+m2BMNet9rvWt2Vzpk6dvEfeFR98AJ2l2qlRE7kmFb2v 1oIBGWdwQpP0w8BVr1FVuGhlDBxr52MIK+pj93nepBC0y4FXPK4MWKHfeKBZpIlbgx2l zpsQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=fYKgvYLgvtSQW10Pkkm7+cl/2v+kKNbnHVNtsf/foy4=; b=Iw7w/EK/I2lJOr0yqvEaTe7LLUBUtRCe79XdHlPPZKCvVKbrSkiKDuRzakCyfBdQ1C Z2Q609rtGcq70YYR1y/dM629DyvcHNSFWMqbq72TJyE5SZ8vEn4pBIHRa74yZzZGHeBh p3bUmeqoIXfVdIE3ga8MeRdoMLtO9DInqxgD8N0f/X2Gt9Ff9UAacMr2x0k1GzxdprTF Eqc/Y7qfKFmLsXgSXzKQhuxxYD0eiHgLdoNRdJm7h9WTQlE9w1m44gx/51FnIY6/WmL7 CLUH/pLQ826VeBxp0/mdgeclpUkpOq5Tmu/1SKSadGpdDhlhRk8DoMO3Oo1cF2OE/e1g Eolg== X-Gm-Message-State: AOAM533SqPsHfdHoCde82Qk6ljh8VCQ/wOuUQAK9fr9Mju5V0D6Gl/WI /u+awse3W+/GJzwDUFcyPE9V7xCcWwMkqAlYqbcMyQ== X-Google-Smtp-Source: ABdhPJyJb8MbkcAOF0uAQweRXiBLqmMU0tzuj8g4DHd1+KDxsnz3qffsAdA3JfM/2GRyO1QcVkQx647Qmhs6PVl7pvs= X-Received: by 2002:a17:906:8558:: with SMTP id h24mr7138820ejy.519.1626375493122; Thu, 15 Jul 2021 11:58:13 -0700 (PDT) MIME-Version: 1.0 References: <343394260f599d940cacc37f1dcc0309239ae220.1626371112.git.zhansayabagdaulet@gmail.com> In-Reply-To: From: Pavel Tatashin Date: Thu, 15 Jul 2021 14:57:37 -0400 Message-ID: Subject: Re: [PATCH 1/2] mm: KSM: fix ksm_run data type To: Matthew Wilcox Cc: Zhansaya Bagdauletkyzy , Andrew Morton , Tyler Hicks , linux-mm , LKML Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=soleen.com header.s=google header.b=I1P8lxvH; spf=pass (imf16.hostedemail.com: domain of pasha.tatashin@soleen.com designates 209.85.218.44 as permitted sender) smtp.mailfrom=pasha.tatashin@soleen.com; dmarc=none X-Rspamd-Server: rspam02 X-Stat-Signature: kydiq5h6a64fyk58898j7iaqmcypexr4 X-Rspamd-Queue-Id: 72DA5F000091 X-HE-Tag: 1626375494-810362 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 Thu, Jul 15, 2021 at 2:30 PM Matthew Wilcox wrote: > > On Thu, Jul 15, 2021 at 02:21:21PM -0400, Pavel Tatashin wrote: > > On Thu, Jul 15, 2021 at 2:18 PM Matthew Wilcox wrote: > > > > > > On Fri, Jul 16, 2021 at 12:01:01AM +0600, Zhansaya Bagdauletkyzy wrote: > > > > +++ b/mm/ksm.c > > > > @@ -289,7 +289,7 @@ static int ksm_nr_node_ids = 1; > > > > #define KSM_RUN_MERGE 1 > > > > #define KSM_RUN_UNMERGE 2 > > > > #define KSM_RUN_OFFLINE 4 > > > > -static unsigned long ksm_run = KSM_RUN_STOP; > > > > +static unsigned int ksm_run = KSM_RUN_STOP; > > > > > > Should this be an enum instead? > > > > I think "unsigned int" is OK here, as it is exposed as uint to users: > > Documentation/ABI/testing/sysfs-kernel-mm-ksm > > > > /sys/kernel/mm/ksm/run > > > > run: write 0 to disable ksm, read 0 while ksm is disabled. > > > > - write 1 to run ksm, read 1 while ksm is running. > > - write 2 to disable ksm and unmerge all its pages. > > The document is out of date then as it does not mention 'offline'. The offline mode cannot be set externally. In run_store() if (flags > KSM_RUN_UNMERGE) return -EINVAL; > > Also, why does the call to kstrtouint() specify base 10? If it is a > bitmap, then permitting 0x [1] is more natural. I would expect to see > base 0 there. Users can only write 0, 1, or 2, it is not a bitmap from the user's perspective as the user cannot write: '3' . But, I think it is somewhat weird that ksm_run is used as a bitmap internally with KSM_RUN_OFFLINE = 4. Imo, KSM_RUN_OFFLINE should be placed in a separate boolean from "ksm_run". > > [1] or even 0b, although I see that _parse_integer_fixup_radix does not > support the 0b notation.