Message ID | 7cb5c8b6c6efba7e437595266638be39f23361fc.1701993656.git.jim.cromie@gmail.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:bcd1:0:b0:403:3b70:6f57 with SMTP id r17csp5148912vqy; Thu, 7 Dec 2023 16:17:16 -0800 (PST) X-Google-Smtp-Source: AGHT+IE2MRhDwPiyK+Vq7ZtuXZUEvz5hzUUHUTmDiHr6WBM4T3SYd4w76ZviSSSTwJtZKGuBA74F X-Received: by 2002:a17:902:b708:b0:1d0:748d:eaa4 with SMTP id d8-20020a170902b70800b001d0748deaa4mr2782017pls.91.1701994635868; Thu, 07 Dec 2023 16:17:15 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1701994635; cv=none; d=google.com; s=arc-20160816; b=pLYg+XsY8wYDEdQtM7QiWWdX+cwhJZVUQOk/beukX7v3kdGCoPHVzDZgYI+1b+9UY9 dhHnXoEZ3gswwaAqnLl1w2ujgjDK3CWY66LNmDwO0X69ang6INIBgzyjFwHx4/7k0Ybn EWzy/7vZLBTtjsickZeHCrsGCzrLbo8+NbEnlTuCN8AFxjuE6VsiRlW9kjLlCNzP1Fk5 NWD4qYswiVe/YE5ZpUSMtvlQVw8+J/BmDUqaQlmLrmpPZ0ljtm8ATfGR2iEkp1AkCa6g gjxWKI5HuKtoyzdu22Fy8gbJRagphUNQDkHzG+vxW9QTauYmEGUhRsyOna6JzLN3OZRe i2cA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=mATCtMnSIobn/epfr5Berq9mhTsVI3cfwyVyNaWc5ds=; fh=Zewmf3wThiipMhDjR96yol5QxF5bGjQ0muN6YDM9mM0=; b=ivGwpVm/aTGUZLRiPGzwXs7ZUf5Habb7Ue1UDr5TzzAOrOo2Iea4Bh4Xj4TTuPduf6 HRWjfOFFhs6pJA1a7IfiZmTEltu0/qewdms9pByulR4Oo//ZGxb0das6I3wPdYyVFLCB Kv47PNP2Wjar7ojQ5H5OCZqnZ7DIM9mcn/wQkfdJ23C3E5mcx/E2F2X7SBYvQwHpBJqd 7hefVfc4D91EhrT/ePnrS8GdTEK3Ini0GxkImVFtcoEwemdh8GTrS6qPMJNdhCwF5ISa M+MNs0EogdcWoghn59nGstwPHRzu+oCKDypZhdZaDgMKXg6+SZbrvhWmuPxiC1STjfkz VkKw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=D6ItTSID; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from snail.vger.email (snail.vger.email. [23.128.96.37]) by mx.google.com with ESMTPS id jw17-20020a170903279100b001cfde84f92esi551580plb.82.2023.12.07.16.17.14 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Dec 2023 16:17:15 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37; Authentication-Results: mx.google.com; dkim=pass header.i=@gmail.com header.s=20230601 header.b=D6ItTSID; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=gmail.com Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id EBC48806885C; Thu, 7 Dec 2023 16:15:53 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1444215AbjLHAPh (ORCPT <rfc822;chrisfriedt@gmail.com> + 99 others); Thu, 7 Dec 2023 19:15:37 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:45174 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1444130AbjLHAPX (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Thu, 7 Dec 2023 19:15:23 -0500 Received: from mail-io1-xd35.google.com (mail-io1-xd35.google.com [IPv6:2607:f8b0:4864:20::d35]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A47301713 for <linux-kernel@vger.kernel.org>; Thu, 7 Dec 2023 16:15:29 -0800 (PST) Received: by mail-io1-xd35.google.com with SMTP id ca18e2360f4ac-7b6eb711498so53096639f.3 for <linux-kernel@vger.kernel.org>; Thu, 07 Dec 2023 16:15:29 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1701994529; x=1702599329; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=mATCtMnSIobn/epfr5Berq9mhTsVI3cfwyVyNaWc5ds=; b=D6ItTSIDrBRmTjvVFY5MBzsE69SVWEVtLwOH3lN4BpfsOJQah4aU/PH3HM27JgvE8M iFkEKN2EElYLTBVwGK7UELbzzG8bRKybgVCMppbRWBWLIf5UsEHw8b8rC+eiTsz3OUyV 8AgRVowJUSlOBnTMyBxL7xh6NytXA265WmxxOewTJgEpIPJH2KI7TqLj6PmouNvP5G/g 8d5caIxSxRZF5V1qQKKQtk1WYWaOBiJ38IbEnUA7tHufUBJYM5RVffS6f2gDDq72/y7b JWMo4H3vre6Y8y6SXInBbna86xDSfR5sDzdaji+RpKsKVP/eHR8HaDL/5hlDOIYvTRze lEDg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701994529; x=1702599329; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=mATCtMnSIobn/epfr5Berq9mhTsVI3cfwyVyNaWc5ds=; b=Flh+OW+dI3hWPFC6hhcB4cvCS3nDXq2Co0PmRHaVXNfax0/snON/6yzJFoFaHqp6sW 1+cojVdsUEb38Swv+wzTKYDctfpQBJX67LEhq+GWko1dXG6CBFJSDNoNqH1qd6Mm+Fm3 aAIdRPPqCELrYMa0BhkWTH4xjicWN8gff4r6fJjgkBNk8C6Ml4c+3aGME8rHOgSjJOsp E2F3rhwHrZFYQP8i+FyIaTv/zO93Uc9UJYJHSP5548MvtCTUFQ6ceiEXKaRqhX3rd3sz Mts/RSDkSy5Y3IyUGkfkA4r63mw8A2H6ShukhMyPfoqxrQnUo9jqDr/fkh5H936JzOfQ DMhw== X-Gm-Message-State: AOJu0Yxvu75ThFrY8oajk7CFu97aQx2WDjxRFtBU5z8rnalAGECeOo1s uXD62BcMPSLNVGiP42E6q2w= X-Received: by 2002:a05:6602:400b:b0:7b4:28f8:1e03 with SMTP id bk11-20020a056602400b00b007b428f81e03mr3994734iob.34.1701994529057; Thu, 07 Dec 2023 16:15:29 -0800 (PST) Received: from frodo.. (c-73-78-62-130.hsd1.co.comcast.net. [73.78.62.130]) by smtp.googlemail.com with ESMTPSA id z18-20020a056638241200b004664a0a7f2csm184652jat.177.2023.12.07.16.15.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 07 Dec 2023 16:15:28 -0800 (PST) From: Jim Cromie <jim.cromie@gmail.com> To: lb@semihalf.com, linux-kernel@vger.kernel.org Cc: akpm@linux-foundation.org, bleung@google.com, contact@emersion.fr, daniel@ffwll.ch, dianders@chromium.org, groeck@google.com, jbaron@akamai.com, jim.cromie@gmail.com, john.ogness@linutronix.de, keescook@chromium.org, pmladek@suse.com, ppaalanen@gmail.com, rostedt@goodmis.org, seanpaul@chromium.org, sergey.senozhatsky@gmail.com, upstream@semihalf.com, vincent.whitchurch@axis.com, yanivt@google.com, gregkh@linuxfoundation.org Subject: [re: PATCH v2 00/15 - 05/11] dyndbg: change +T:name_terminator to dot Date: Thu, 7 Dec 2023 17:15:08 -0700 Message-ID: <7cb5c8b6c6efba7e437595266638be39f23361fc.1701993656.git.jim.cromie@gmail.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <cover.1701993656.git.jim.cromie@gmail.com> References: <CAK8ByeK8dGcbxfXghw6=LrhSWLmO0a4XuB8C0nsUc812aoU0Pw@mail.gmail.com> <cover.1701993656.git.jim.cromie@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM, RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Thu, 07 Dec 2023 16:15:54 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1784670727521407805 X-GMAIL-MSGID: 1784670727521407805 |
Commit Message
Jim Cromie
Dec. 8, 2023, 12:15 a.m. UTC
This replaces ',' with '.' as the char that ends the +T:name.
This allows a later patch to treat ',' as a space, which mostly
eliminates the need to quote query/rules. And this in turn avoids
quoting hassles:
modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p
It is particularly good for passing boot-args into test-scripts.
vng -p 4 -v \
-a test_dynamic_debug.dyndbg=class,D2_CORE,+p
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
lib/dynamic_debug.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Comments
On Thu 2023-12-07 17:15:08, Jim Cromie wrote: > This replaces ',' with '.' as the char that ends the +T:name. > > This allows a later patch to treat ',' as a space, which mostly > eliminates the need to quote query/rules. And this in turn avoids > quoting hassles: > > modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p > > It is particularly good for passing boot-args into test-scripts. > > vng -p 4 -v \ > -a test_dynamic_debug.dyndbg=class,D2_CORE,+p Could you please add example how it looked before and after? Is this format documented somewhere? Will the documentation get updated? Could it break existing scripts? [*] The dynamic debug interface is really hard to use for me as an occasional user. I always have to look into Documentation/admin-guide/dynamic-debug-howto.rst Anyway, there should be a good reason to change the interface. And the exaplantion: "Let's use '.' instead of ',' so that we could later treat ',' as space" sounds scarry. It does not explain what is the advantage at all. [*] Some scripts are using the interface even in the mainline, for example: $> git grep "dynamic_debug" tools/testing/ tools/testing/selftests/bpf/test_tunnel.sh: echo 'file ip_gre.c +p' > /sys/kernel/debug/dynamic_debug/control tools/testing/selftests/bpf/test_tunnel.sh: echo 'file ip6_gre.c +p' > /sys/kernel/debug/dynamic_debug/control tools/testing/selftests/bpf/test_tunnel.sh: echo 'file geneve.c +p' > /sys/kernel/debug/dynamic_debug/control tools/testing/selftests/bpf/test_tunnel.sh: echo 'file ipip.c +p' > /sys/kernel/debug/dynamic_debug/control tools/testing/selftests/livepatch/functions.sh: DYNAMIC_DEBUG=$(grep '^kernel/livepatch' /sys/kernel/debug/dynamic_debug/control | \ tools/testing/selftests/livepatch/functions.sh: echo -n "$DYNAMIC_DEBUG" > /sys/kernel/debug/dynamic_debug/control tools/testing/selftests/livepatch/functions.sh:function set_dynamic_debug() { tools/testing/selftests/livepatch/functions.sh: cat <<-EOF > /sys/kernel/debug/dynamic_debug/control tools/testing/selftests/livepatch/functions.sh: set_dynamic_debug Best Regards, Petr
pon., 18 gru 2023 o 18:29 Petr Mladek <pmladek@suse.com> napisał(a): > > On Thu 2023-12-07 17:15:08, Jim Cromie wrote: > > This replaces ',' with '.' as the char that ends the +T:name. > > > > This allows a later patch to treat ',' as a space, which mostly > > eliminates the need to quote query/rules. And this in turn avoids > > quoting hassles: > > > > modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p > > > > It is particularly good for passing boot-args into test-scripts. > > > > vng -p 4 -v \ > > -a test_dynamic_debug.dyndbg=class,D2_CORE,+p > > Could you please add example how it looked before and after? Before a user had to issue a command in the format modprobe test_dynamic_debug dyndbg="class D2_CORE +p" Now a use can use either modprobe test_dynamic_debug dyndbg="class D2_CORE +p" or modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p > Is this format documented somewhere? > Will the documentation get updated? Documentation will be updated. > Could it break existing scripts? [*] > It should not break any scripts as this change does not change the interface but extends it. > The dynamic debug interface is really hard to use for me > as an occasional user. I always have to look into > Documentation/admin-guide/dynamic-debug-howto.rst > > Anyway, there should be a good reason to change the interface. > And the exaplantion: > > "Let's use '.' instead of ',' so that we could later > treat ',' as space" > > sounds scarry. It does not explain what is the advantage at all. > I will clarify in the commit message that this change allows to use two formats either modprobe test_dynamic_debug dyndbg="class D2_CORE +p" or modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p > > [*] Some scripts are using the interface even in the mainline, > for example: > > $> git grep "dynamic_debug" tools/testing/ > tools/testing/selftests/bpf/test_tunnel.sh: echo 'file ip_gre.c +p' > /sys/kernel/debug/dynamic_debug/control > tools/testing/selftests/bpf/test_tunnel.sh: echo 'file ip6_gre.c +p' > /sys/kernel/debug/dynamic_debug/control > tools/testing/selftests/bpf/test_tunnel.sh: echo 'file geneve.c +p' > /sys/kernel/debug/dynamic_debug/control > tools/testing/selftests/bpf/test_tunnel.sh: echo 'file ipip.c +p' > /sys/kernel/debug/dynamic_debug/control > tools/testing/selftests/livepatch/functions.sh: DYNAMIC_DEBUG=$(grep '^kernel/livepatch' /sys/kernel/debug/dynamic_debug/control | \ > tools/testing/selftests/livepatch/functions.sh: echo -n "$DYNAMIC_DEBUG" > /sys/kernel/debug/dynamic_debug/control > tools/testing/selftests/livepatch/functions.sh:function set_dynamic_debug() { > tools/testing/selftests/livepatch/functions.sh: cat <<-EOF > /sys/kernel/debug/dynamic_debug/control > tools/testing/selftests/livepatch/functions.sh: set_dynamic_debug > Good to know. I will use these scripts to make sure the dynamic debug interface is not broken. Thanks, Lukasz > > Best Regards, > Petr
On Thu 2023-12-21 16:21:49, Łukasz Bartosik wrote: > pon., 18 gru 2023 o 18:29 Petr Mladek <pmladek@suse.com> napisał(a): > > > > On Thu 2023-12-07 17:15:08, Jim Cromie wrote: > > > This replaces ',' with '.' as the char that ends the +T:name. > > > > > > This allows a later patch to treat ',' as a space, which mostly > > > eliminates the need to quote query/rules. And this in turn avoids > > > quoting hassles: > > > > > > modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p > > > > > > It is particularly good for passing boot-args into test-scripts. > > > > > > vng -p 4 -v \ > > > -a test_dynamic_debug.dyndbg=class,D2_CORE,+p > > > > Could you please add example how it looked before and after? > > Before a user had to issue a command in the format > modprobe test_dynamic_debug dyndbg="class D2_CORE +p" > > Now a use can use either > modprobe test_dynamic_debug dyndbg="class D2_CORE +p" > or > modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p I see. This was not clear to me. Please, mention it in the commit message. That said, I am not sure if it is worth it and if it is a good idea. Supporting more formats adds complexity and confusion. It is the reason why people hate perl. I agree that quoting in scripts is complicated. Well, a sane approach is to use quotes everywhere where possible. If a script works correctly only with class,D2_CORE,+p and breaks with "class D2_CORE +p" then it is a ticking bomb. People might try to use "class D2_CORE +p" one day because they would cut&paste the string from the internet. > > Is this format documented somewhere? > > Will the documentation get updated? > > Documentation will be updated. > > > Could it break existing scripts? [*] > > It should not break any scripts as this change does not change the > interface but extends it. > > > The dynamic debug interface is really hard to use for me > > as an occasional user. I always have to look into > > Documentation/admin-guide/dynamic-debug-howto.rst > > > > Anyway, there should be a good reason to change the interface. > > And the exaplantion: > > > > "Let's use '.' instead of ',' so that we could later > > treat ',' as space" > > > > sounds scarry. It does not explain what is the advantage at all. > > > I will clarify in the commit message that this change allows to use > two formats either > modprobe test_dynamic_debug dyndbg="class D2_CORE +p" > or > modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p But the patch replaces ',' with '.'. It looks like it modifies some existing syntax. Note that I am not familiar with this code. And I even do not see the patched function in the current Linus' tree. Best Regards, Petr
czw., 4 sty 2024 o 08:54 Petr Mladek <pmladek@suse.com> napisał(a): > > On Thu 2023-12-21 16:21:49, Łukasz Bartosik wrote: > > pon., 18 gru 2023 o 18:29 Petr Mladek <pmladek@suse.com> napisał(a): > > > > > > On Thu 2023-12-07 17:15:08, Jim Cromie wrote: > > > > This replaces ',' with '.' as the char that ends the +T:name. > > > > > > > > This allows a later patch to treat ',' as a space, which mostly > > > > eliminates the need to quote query/rules. And this in turn avoids > > > > quoting hassles: > > > > > > > > modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p > > > > > > > > It is particularly good for passing boot-args into test-scripts. > > > > > > > > vng -p 4 -v \ > > > > -a test_dynamic_debug.dyndbg=class,D2_CORE,+p > > > > > > Could you please add example how it looked before and after? > > > > Before a user had to issue a command in the format > > modprobe test_dynamic_debug dyndbg="class D2_CORE +p" > > > > Now a use can use either > > modprobe test_dynamic_debug dyndbg="class D2_CORE +p" > > or > > modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p > > I see. This was not clear to me. Please, mention it in > the commit message. > In the patchset v3 I squashed "dyndbg: change +T:name_terminator to dot" with "dyndbg: add processing of T(race) flag argument" but I will clarify this topic in the "dyndbg: treat comma as a token separator" > That said, I am not sure if it is worth it and if it > is a good idea. Supporting more formats adds complexity > and confusion. It is the reason why people hate perl. > > I agree that quoting in scripts is complicated. Well, > a sane approach is to use quotes everywhere where possible. > > If a script works correctly only with class,D2_CORE,+p > and breaks with "class D2_CORE +p" then it is a ticking > bomb. People might try to use "class D2_CORE +p" > one day because they would cut&paste the string > from the internet. > Addition of new format of dynamic debug queries does not obsolete the existing format. > > > Is this format documented somewhere? > > > Will the documentation get updated? > > > > Documentation will be updated. > > > > > Could it break existing scripts? [*] > > > > It should not break any scripts as this change does not change the > > interface but extends it. > > > > > The dynamic debug interface is really hard to use for me > > > as an occasional user. I always have to look into > > > Documentation/admin-guide/dynamic-debug-howto.rst > > > > > > Anyway, there should be a good reason to change the interface. > > > And the exaplantion: > > > > > > "Let's use '.' instead of ',' so that we could later > > > treat ',' as space" > > > > > > sounds scarry. It does not explain what is the advantage at all. > > > > > I will clarify in the commit message that this change allows to use > > two formats either > > modprobe test_dynamic_debug dyndbg="class D2_CORE +p" > > or > > modprobe test_dynamic_debug dyndbg=class,D2_CORE,+p > > But the patch replaces ',' with '.'. It looks like it modifies > some existing syntax. > > Note that I am not familiar with this code. And I even do not see > the patched function in the current Linus' tree. > Next patchset will include updated dynamic debug documentation. Hopefully this will help to resolve doubts. Regards, Lukasz > Best Regards, > Petr >
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 2ac1bd7f105f..a5fc80edd24c 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -172,7 +172,7 @@ char *read_T_args(const char *str, struct flag_settings *modifiers) } str += 2; - end = strchr(str, ','); + end = strchr(str, '.'); if (end && *(end + 1) == '\0') return NULL; @@ -264,7 +264,7 @@ static char *ddebug_describe_ctrl(struct dd_ctrl *ctrl, struct ctrlbuf *cb) for (i = 0; i < ARRAY_SIZE(opt_array); ++i) if (ctrl->flags & opt_array[i].flag) { if (show_args) - *p++ = ','; + *p++ = '.'; *p++ = opt_array[i].opt_char; show_args = opt_array[i].show_args; if (show_args)