From patchwork Sun Feb 18 11:07:02 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Eugeniu Rosca X-Patchwork-Id: 10226727 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 08D21602DC for ; Sun, 18 Feb 2018 11:07:17 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id D4D7E28961 for ; Sun, 18 Feb 2018 11:07:16 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id C7BFB28A0F; Sun, 18 Feb 2018 11:07:16 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, RCVD_IN_DNSWL_HI, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9567E28961 for ; Sun, 18 Feb 2018 11:07:14 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751114AbeBRLHN (ORCPT ); Sun, 18 Feb 2018 06:07:13 -0500 Received: from mail-wm0-f66.google.com ([74.125.82.66]:34921 "EHLO mail-wm0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751001AbeBRLHM (ORCPT ); Sun, 18 Feb 2018 06:07:12 -0500 Received: by mail-wm0-f66.google.com with SMTP id x21so10210805wmh.0 for ; Sun, 18 Feb 2018 03:07:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:date:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=c+5fkFxVsgUamynBJMpPSpQYQ5etu4hvx5ebE9PFQj8=; b=su1SNgJ2UGybjPB12v72hu/IqsLA8OmUK03y4U+XrTXmG3iGD7kAJRyK5a7ubkiwMu Qd7PoxT/PDu9v6jCMzOWCZ2Ga3xVRWzRhUs+GF/ONCseu6qpWK8zkdftFM5LmbrVd5lT OjSHh+GnTLl046vO3DsqcFxst8ItCUAdCFkpSrEj7410uTBDdlVWIMpR1cxk0azt9avj UorBmrGg2U5xCSl6MnuaVX433emjddDkHmRUdlkMySvnVX9BvTJSJ0slOMWUAfG1B74l hiSzxTd0i+pDd5GP5na8kdByHLJX9p77uNzWC5awM79HtzcwcJu8/RQGRbVWZ0W8QFvv RMuQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:date:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=c+5fkFxVsgUamynBJMpPSpQYQ5etu4hvx5ebE9PFQj8=; b=K2T8dX/E8vdTkAVJCy/JNcG1BkctxxmccCsHd6U3bEGzCaXqu+Xm4K0ZLoYkiqQZGD O+gKf3xhQnGeWw7NOzgfROfkG2GXcq/rAL/47fQIuGzRg65ASOJZXW0Q1i/pmQ2wYs73 w/YUF0xuFpE3vKt2RsjUkqnMSLl6ONpilUKFy2F//QDIbjIoZTRCBTO733FTbnUUcz0e p1Ux/OGax7bULaKDuL6CH/st24MecG+7Y/sI5IKR7PVvkQsE7BOpfIs3r4kvXL7PhrA/ un5Q9jvAsOaDREa/WvGoxy0zivMJebBmKBSrZjbTNLnYt2PHlxOno6qR9Cz/YmCDP6O3 zyzA== X-Gm-Message-State: APf1xPC1U6KGFZuoUSMgXrzvkZENbEhkndDrHMQ2wN/TQAJQ7C1Unxdj e1rIk4638dSxY2TcuMZuoDo= X-Google-Smtp-Source: AH8x224DXPqmyZVF0WfA0SWx2mQD42SdRUpyVkKrSDFQhN2f5WUzxv0eb/x4k5eynTP+5fZM0x3cIQ== X-Received: by 10.80.201.73 with SMTP id p9mr14941146edh.161.1518952031157; Sun, 18 Feb 2018 03:07:11 -0800 (PST) Received: from example.com ([2a02:8108:91c0:4bcc:98e2:b49f:4e58:283e]) by smtp.gmail.com with ESMTPSA id l23sm17134269edc.20.2018.02.18.03.07.07 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Sun, 18 Feb 2018 03:07:10 -0800 (PST) From: Eugeniu Rosca X-Google-Original-From: Eugeniu Rosca Date: Sun, 18 Feb 2018 12:07:02 +0100 To: Ulf Magnusson Cc: Eugeniu Rosca , Masahiro Yamada , Petr Vorel , Nicolas Pitre , Randy Dunlap , Paul Bolle , Eugeniu Rosca , Linux Kbuild mailing list Subject: Re: [PATCH v3 3/3] kconfig: Print reverse dependencies in groups Message-ID: <20180218110702.GA26185@example.com> References: <0ed37972989a9ba7a8b50777bd60ea8f46495a2a.1518826148.git.erosca@de.adit-jv.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kbuild-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kbuild@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Sat, Feb 17, 2018 at 05:55:01PM +0100, Ulf Magnusson wrote: > On Sat, Feb 17, 2018 at 3:05 AM, Eugeniu Rosca wrote: > > From: Eugeniu Rosca > > > > Assuming commit 617aebe6a97e ("Merge tag 'usercopy-v4.16-rc1' of > > git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux"), ARCH=arm64 > > and vanilla arm64 defconfig, here is the top 10 CONFIG options with > > the highest amount of top level "||" sub-expressions/tokens that make > > up the final "{Selected,Implied} by" reverse dependency expression. > > > > | Config | Revdep all | Revdep ![=n] | > > |-------------------------------|------------|--------------| > > | REGMAP_I2C | 212 | 9 | > > | CRC32 | 167 | 25 | > > | FW_LOADER | 128 | 5 | > > | MFD_CORE | 124 | 9 | > > | FB_CFB_IMAGEBLIT | 114 | 2 | > > | FB_CFB_COPYAREA | 111 | 2 | > > | FB_CFB_FILLRECT | 110 | 2 | > > | SND_PCM | 103 | 2 | > > | CRYPTO_HASH | 87 | 19 | > > | WATCHDOG_CORE | 86 | 6 | > > > > The story behind the above is that users need to visually > > review/evaluate 212 expressions which *potentially* select REGMAP_I2C > > in order to identify the expressions which *actually* select REGMAP_I2C, > > for a particular ARCH and for a particular defconfig used. > > > > To make this experience smoother, change the way reverse dependencies > > are displayed to the user from [1] to [2]. > > > > [1] Old representation of reverse dependencies for DMA_ENGINE_RAID: > > Selected by: > > - AMCC_PPC440SPE_ADMA [=n] && DMADEVICES [=y] && (440SPe || 440SP) > > - BCM_SBA_RAID [=m] && DMADEVICES [=y] && (ARM64 [=y] || ... > > - FSL_RAID [=n] && DMADEVICES [=y] && FSL_SOC && ... > > - INTEL_IOATDMA [=n] && DMADEVICES [=y] && PCI [=y] && X86_64 > > - MV_XOR [=n] && DMADEVICES [=y] && (PLAT_ORION || ARCH_MVEBU [=y] ... > > - MV_XOR_V2 [=y] && DMADEVICES [=y] && ARM64 [=y] > > - XGENE_DMA [=n] && DMADEVICES [=y] && (ARCH_XGENE [=y] || ... > > - DMATEST [=n] && DMADEVICES [=y] && DMA_ENGINE [=y] > > > > [2] New representation of reverse dependencies for DMA_ENGINE_RAID: > > Selected by [y]: > > - MV_XOR_V2 [=y] && DMADEVICES [=y] && ARM64 [=y] > > Selected by [m]: > > - BCM_SBA_RAID [=m] && DMADEVICES [=y] && (ARM64 [=y] || ... > > Selected by [n]: > > - AMCC_PPC440SPE_ADMA [=n] && DMADEVICES [=y] && (440SPe || ... > > - FSL_RAID [=n] && DMADEVICES [=y] && FSL_SOC && ... > > - INTEL_IOATDMA [=n] && DMADEVICES [=y] && PCI [=y] && X86_64 > > - MV_XOR [=n] && DMADEVICES [=y] && (PLAT_ORION || ARCH_MVEBU [=y] ... > > - XGENE_DMA [=n] && DMADEVICES [=y] && (ARCH_XGENE [=y] || ... > > - DMATEST [=n] && DMADEVICES [=y] && DMA_ENGINE [=y] > > > > Signed-off-by: Eugeniu Rosca > > --- > > scripts/kconfig/expr.c | 59 ++++++++++++++++++++++++++++++++++++++++++++- > > scripts/kconfig/expr.h | 4 +++ > > scripts/kconfig/lkc_proto.h | 1 + > > scripts/kconfig/menu.c | 37 ++++++++++++++++++++-------- > > 4 files changed, 90 insertions(+), 11 deletions(-) > > > > diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c > > index c610cb14284f..48b99371d276 100644 > > --- a/scripts/kconfig/expr.c > > +++ b/scripts/kconfig/expr.c > > @@ -1213,6 +1213,18 @@ __expr_print(struct expr *e, > > case PRINT_REVDEP_ALL: > > expr_print_newline(e, fn, data, E_OR); > > break; > > + case PRINT_REVDEP_YES: > > + if (expr_calc_value(e) == yes) > > + expr_print_newline(e, fn, data, E_OR); > > + break; > > + case PRINT_REVDEP_MOD: > > + if (expr_calc_value(e) == mod) > > + expr_print_newline(e, fn, data, E_OR); > > + break; > > + case PRINT_REVDEP_NO: > > + if (expr_calc_value(e) == no) > > + expr_print_newline(e, fn, data, E_OR); > > + break; > > Perhaps this logic could be moved into expr_print_newline() (which > could be renamed to something like expr_print_select() in that case). > Could have it take 'enum print_type' as an argument. If expr_print_newline is renamed to expr_print_{select,revdep}, then I still face the need of expr_print_newline. So, I kept the *newline() function in place and came up with below solution to aggregate duplicated code. Please, let me know if it looks OK to you (btw, it creates a slightly higher --stat compared to current solution). > > > + PRINT_REVDEP_YES, > > + PRINT_REVDEP_MOD, > > + PRINT_REVDEP_NO, > > }; > > > > union expr_data { > > @@ -316,6 +319,7 @@ void expr_fprint(struct expr *e, FILE *out); > > struct gstr; /* forward */ > > void expr_gstr_print(struct expr *e, struct gstr *gs); > > void expr_gstr_print_revdep(struct expr *e, struct gstr *gs, enum print_type t); > > +bool expr_revdep_contains(struct expr *e, tristate val); > > > > static inline int expr_is_yes(struct expr *e) > > { > > diff --git a/scripts/kconfig/lkc_proto.h b/scripts/kconfig/lkc_proto.h > > index 9dc8abfb1dc3..69ed1477e4ef 100644 > > --- a/scripts/kconfig/lkc_proto.h > > +++ b/scripts/kconfig/lkc_proto.h > > @@ -25,6 +25,7 @@ bool menu_has_help(struct menu *menu); > > const char * menu_get_help(struct menu *menu); > > struct gstr get_relations_str(struct symbol **sym_arr, struct list_head *head); > > void menu_get_ext_help(struct menu *menu, struct gstr *help); > > +void get_revdep_by_type(struct expr *e, char *s, struct gstr *r); > > > > /* symbol.c */ > > extern struct symbol * symbol_hash[SYMBOL_HASHSIZE]; > > diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c > > index 5b8edba105f2..d13ffa69d65b 100644 > > --- a/scripts/kconfig/menu.c > > +++ b/scripts/kconfig/menu.c > > @@ -790,6 +790,31 @@ static void get_symbol_props_str(struct gstr *r, struct symbol *sym, > > str_append(r, "\n"); > > } > > > > +void get_revdep_by_type(struct expr *e, char *s, struct gstr *r) > > +{ > > + if (!e) > > + return; > > + > > + if (expr_revdep_contains(e, yes)) { > > + str_append(r, s); > > + str_append(r, _(" [y]:")); > > + expr_gstr_print_revdep(e, r, PRINT_REVDEP_YES); > > + str_append(r, "\n"); > > + } > > + if (expr_revdep_contains(e, mod)) { > > + str_append(r, s); > > + str_append(r, _(" [m]:")); > > + expr_gstr_print_revdep(e, r, PRINT_REVDEP_MOD); > > + str_append(r, "\n"); > > + } > > + if (expr_revdep_contains(e, no)) { > > + str_append(r, s); > > + str_append(r, _(" [n]:")); > > + expr_gstr_print_revdep(e, r, PRINT_REVDEP_NO); > > + str_append(r, "\n"); > > + } > > +} > > Those _() are for gettext btw. Not sure those strings need > translations (or if anyone is translating Kconfig), so I think they > could be removed here. No problem. I can remove them :) > > > + > > /* > > * head is optional and may be NULL > > */ > > @@ -826,18 +851,10 @@ static void get_symbol_str(struct gstr *r, struct symbol *sym, > > } > > > > get_symbol_props_str(r, sym, P_SELECT, _(" Selects: ")); > > - if (sym->rev_dep.expr) { > > - str_append(r, _(" Selected by: ")); > > - expr_gstr_print_revdep(sym->rev_dep.expr, r, PRINT_REVDEP_ALL); > > - str_append(r, "\n"); > > - } > > + get_revdep_by_type(sym->rev_dep.expr, " Selected by", r); > > > > get_symbol_props_str(r, sym, P_IMPLY, _(" Implies: ")); > > - if (sym->implied.expr) { > > - str_append(r, _(" Implied by: ")); > > - expr_gstr_print_revdep(sym->implied.expr, r, PRINT_REVDEP_ALL); > > - str_append(r, "\n"); > > - } > > + get_revdep_by_type(sym->implied.expr, " Implied by", r); > > > > str_append(r, "\n\n"); > > } > > -- > > 2.16.1 > > > > Looks like a very nice patchset in general to me. After getting your feedback/preference on the above, I will hopefully be able to push v4. Thanks for the reviewing effort! > Cheers, > Ulf Regards, Eugeniu. --- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c index 48b99371d276..a6316e5681d9 100644 --- a/scripts/kconfig/expr.c +++ b/scripts/kconfig/expr.c @@ -1189,6 +1189,34 @@ expr_print_newline(struct expr *e, expr_print(e, fn, data, prevtoken); } +static void +expr_print_revdep(struct expr *e, + void (*fn)(void *, struct symbol *, const char *), + void *data, + int prevtoken, + enum print_type type) +{ + switch (type) { + case PRINT_REVDEP_ALL: + expr_print_newline(e, fn, data, prevtoken); + break; + case PRINT_REVDEP_YES: + if (expr_calc_value(e) == yes) + expr_print_newline(e, fn, data, prevtoken); + break; + case PRINT_REVDEP_MOD: + if (expr_calc_value(e) == mod) + expr_print_newline(e, fn, data, prevtoken); + break; + case PRINT_REVDEP_NO: + if (expr_calc_value(e) == no) + expr_print_newline(e, fn, data, prevtoken); + break; + default: + break; + } +} + static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, const char *), @@ -1211,21 +1239,10 @@ __expr_print(struct expr *e, fn(data, e->left.sym, e->left.sym->name); break; case PRINT_REVDEP_ALL: - expr_print_newline(e, fn, data, E_OR); - break; case PRINT_REVDEP_YES: - if (expr_calc_value(e) == yes) - expr_print_newline(e, fn, data, E_OR); - break; case PRINT_REVDEP_MOD: - if (expr_calc_value(e) == mod) - expr_print_newline(e, fn, data, E_OR); - break; case PRINT_REVDEP_NO: - if (expr_calc_value(e) == no) - expr_print_newline(e, fn, data, E_OR); - break; - default: + expr_print_revdep(e, fn, data, E_OR, type); break; } else @@ -1283,21 +1300,10 @@ __expr_print(struct expr *e, expr_print(e->right.expr, fn, data, E_AND); break; case PRINT_REVDEP_ALL: - expr_print_newline(e, fn, data, E_OR); - break; case PRINT_REVDEP_YES: - if (expr_calc_value(e) == yes) - expr_print_newline(e, fn, data, E_OR); - break; case PRINT_REVDEP_MOD: - if (expr_calc_value(e) == mod) - expr_print_newline(e, fn, data, E_OR); - break; case PRINT_REVDEP_NO: - if (expr_calc_value(e) == no) - expr_print_newline(e, fn, data, E_OR); - break; - default: + expr_print_revdep(e, fn, data, E_OR, type); break; } break; > > > default: > > break; > > } > > @@ -1273,6 +1285,18 @@ __expr_print(struct expr *e, > > case PRINT_REVDEP_ALL: > > expr_print_newline(e, fn, data, E_OR); > > break; > > + case PRINT_REVDEP_YES: > > + if (expr_calc_value(e) == yes) > > + expr_print_newline(e, fn, data, E_OR); > > + break; > > + case PRINT_REVDEP_MOD: > > + if (expr_calc_value(e) == mod) > > + expr_print_newline(e, fn, data, E_OR); > > + break; > > + case PRINT_REVDEP_NO: > > + if (expr_calc_value(e) == no) > > + expr_print_newline(e, fn, data, E_OR); > > + break; > > That would simplify this part as well, since they're equal. > > > default: > > break; > > } > > @@ -1353,10 +1377,43 @@ void expr_gstr_print(struct expr *e, struct gstr *gs) > > expr_print(e, expr_print_gstr_helper, gs, E_NONE); > > } > > > > +/* > > + * Allow front ends to check if a specific reverse dependency expression > > + * has at least one top level "||" member which evaluates to "val". This, > > + * will allow front ends to, as example, avoid printing "Selected by [y]:" > > + * line when there are actually no top level "||" sub-expressions which > > + * evaluate to =y. > > + */ > > +bool expr_revdep_contains(struct expr *e, tristate val) > > +{ > > + bool ret = false; > > + > > + if (!e) > > + return ret; > > + > > + switch (e->type) { > > + case E_SYMBOL: > > + case E_AND: > > + if (expr_calc_value(e) == val) > > + ret = true; > > + break; > > + case E_OR: > > + if (expr_revdep_contains(e->left.expr, val)) > > + ret = true; > > + else if (expr_revdep_contains(e->right.expr, val)) > > + ret = true; > > + break; > > + default: > > + break; > > + } > > + return ret; > > +} > > + > > /* > > * Transform the top level "||" tokens into newlines and prepend each > > * line with a minus. This makes expressions much easier to read. > > - * Suitable for reverse dependency expressions. > > + * Suitable for reverse dependency expressions. In addition, allow > > + * selective printing of tokens/sub-expressions by their tristate value. > > */ > > void expr_gstr_print_revdep(struct expr *e, struct gstr *gs, enum print_type t) > > { > > diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h > > index 21cb67c15091..d5b096725ca8 100644 > > --- a/scripts/kconfig/expr.h > > +++ b/scripts/kconfig/expr.h > > @@ -37,6 +37,9 @@ enum expr_type { > > enum print_type { > > PRINT_NORMAL, > > PRINT_REVDEP_ALL, > > PRINT_REVDEP_ALL is unused now, right? > > Guess it doesn't hurt that much to have it there, though I'm more of a > "it won't be needed" person. Correct. It remains unused if this patchset is applied. I would avoid removing PRINT_REVDEP_ALL in the same context with adding support for PRINT_REVDEP_{YES|MOD|NO}. I think this should be done in a standalone commit for better visibility (to be reverted easily if ever needed). Here is how it would look like on top of v3 patchset. Please, provide your preference if the 6 lines may stay or should be gone. diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c index b91cbc1e20c0..2313a157ebaf 100644 --- a/scripts/kconfig/expr.c +++ b/scripts/kconfig/expr.c @@ -1199,9 +1199,6 @@ expr_print_revdep(struct expr *e, switch (type) { case PRINT_NORMAL: break; - case PRINT_REVDEP_ALL: - expr_print_newline(e, fn, data, prevtoken); - break; case PRINT_REVDEP_YES: if (expr_calc_value(e) == yes) expr_print_newline(e, fn, data, prevtoken); @@ -1238,7 +1235,6 @@ __expr_print(struct expr *e, case PRINT_NORMAL: fn(data, e->left.sym, e->left.sym->name); break; - case PRINT_REVDEP_ALL: case PRINT_REVDEP_YES: case PRINT_REVDEP_MOD: case PRINT_REVDEP_NO: @@ -1299,7 +1295,6 @@ __expr_print(struct expr *e, fn(data, NULL, " && "); expr_print(e->right.expr, fn, data, E_AND); break; - case PRINT_REVDEP_ALL: case PRINT_REVDEP_YES: case PRINT_REVDEP_MOD: case PRINT_REVDEP_NO: diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h index d5b096725ca8..e5687b430c17 100644 --- a/scripts/kconfig/expr.h +++ b/scripts/kconfig/expr.h @@ -36,7 +36,6 @@ enum expr_type { enum print_type { PRINT_NORMAL, - PRINT_REVDEP_ALL, PRINT_REVDEP_YES, PRINT_REVDEP_MOD, PRINT_REVDEP_NO,