From patchwork Tue Feb 20 15:00:18 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ulf Magnusson X-Patchwork-Id: 10230207 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 AF3F5602B1 for ; Tue, 20 Feb 2018 15:00:41 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 8A5F0286DF for ; Tue, 20 Feb 2018 15:00:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 87E3528710; Tue, 20 Feb 2018 15:00:41 +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 5F349287DE for ; Tue, 20 Feb 2018 15:00:34 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751039AbeBTPAe (ORCPT ); Tue, 20 Feb 2018 10:00:34 -0500 Received: from mail-lf0-f66.google.com ([209.85.215.66]:37334 "EHLO mail-lf0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751000AbeBTPAd (ORCPT ); Tue, 20 Feb 2018 10:00:33 -0500 Received: by mail-lf0-f66.google.com with SMTP id y19so4616251lfd.4 for ; Tue, 20 Feb 2018 07:00:32 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=r032PKNlPQ60J3AuiIXmG6avwH2exiG+tbJqMz7e7QU=; b=FIe33C6AWNHG84kkX73gNNP3TZkdGhUhH1fD+yQAWqH6mTeZ35k6abB/X+K/7fBOVb HD7FGTEVXKaapBgI2V94XbIje7zVt/wzXsy6Wmgl28N0aslV/ukYejf5rb0GDhcif3YO XhOjI3AdyjcPHhae2O/GY0WtyYNLUC4MT9EvLnmlSN9z/p/l4MY/GTNoRYOtR+HMQbqX R7X2P+006Egj3MpG0ELoQ1kioZkmQASgqpTFUGfImBuVGVxDOP3IlA7PbkE/7gF8BHEw XmwfpSu+xyEywjl27N3Oy16whTNOgHXZT639E1Hvh+h/U4jPBqnXIQrDJ7gMbcpfdYsa 4fuQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=r032PKNlPQ60J3AuiIXmG6avwH2exiG+tbJqMz7e7QU=; b=EWoWwdSIkSXwI9XWMZGZ5Io4xLXoamRjelG7JfMDdW2EajcVwNC7C36GjCOer7AIqR uzv2ZSEMZcRwWLocm4bHeD5CVEnkx4pXKAHIZB0JKiUvLTZ/sL50IJr2mq9TQyguqlXp BNyv0DXmQ4FVhKL9bPLGlnJTjhTVk3h1d86gRw/+soHAa1HRi/KYpCN0NRGv180xa9TN LQBfjDnXmYV+gQSJa5BVCz93VHp7ocz2U7PWxLbrJwnb4PuTzZKQOObF2sDAcHPnPvs/ ucDC5EwPLEmtiKdP+ysIOykILqncGPu0ZCqeqcv2eN24+0PFEhdFKbQubrDtMPgD2BV5 2yAA== X-Gm-Message-State: APf1xPBqbGwJYTXdifq0tEqma0YzKB2SvpZ/7LSpMYIgKqUa1uAEWlOV Iys8pIqpqE6tTSntIrJGA4A= X-Google-Smtp-Source: AH8x225Jr4WCTHWiabVcqyZLpnvzHAWsnmBxoeHm+RvsQHj2fvq3kFlVwj8x72q4ae3EWgmikMYCCg== X-Received: by 10.25.152.21 with SMTP id a21mr3879350lfe.93.1519138831371; Tue, 20 Feb 2018 07:00:31 -0800 (PST) Received: from huvuddator (ua-213-113-106-221.cust.bredbandsbolaget.se. [213.113.106.221]) by smtp.gmail.com with ESMTPSA id q62sm5415204ljq.17.2018.02.20.07.00.30 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 20 Feb 2018 07:00:30 -0800 (PST) Date: Tue, 20 Feb 2018 16:00:18 +0100 From: Ulf Magnusson To: Masahiro Yamada Cc: Eugeniu Rosca , Petr Vorel , Nicolas Pitre , Randy Dunlap , Paul Bolle , Eugeniu Rosca , Eugeniu Rosca , Linux Kbuild mailing list Subject: Re: [PATCH v4 0/3] Kconfig: Print reverse dependencies in groups Message-ID: <20180220150018.mjyjmdmao7xqyukr@huvuddator> References: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170609 (1.8.3) 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 Tue, Feb 20, 2018 at 05:24:01PM +0900, Masahiro Yamada wrote: > 2018-02-20 17:13 GMT+09:00 Masahiro Yamada : > > 2018-02-19 5:47 GMT+09:00 Eugeniu Rosca : > >> From: Eugeniu Rosca > >> > >> Hello Masahiro, Ulf, Petr and all, > >> > >> Here are a few words about the motivation behind this patch series. > >> > >> First, the reason I got in touch with Kconfig is to optimize the > >> configuration of automotive kernels, as well as to align the kernel > >> configuration across a number of platforms. In the context of kernel > >> optimization, one of the primary goals is to filter out any features > >> that are not mentioned in the platform requirements. > >> > >> Surprisingly or not, disabling a CONFIG option (which is assumed to > >> be unneeded) may be not so trivial. Especially it is not trivial, when > >> this CONFIG option is selected by a dozen of other configs. Before the > >> moment commit 1ccb27143360 ("kconfig: make "Selected by:" and > >> "Implied by:" readable") was submitted by Petr and eventually popped > >> up in v4.16-rc1, it was an absolute pain to break down the "Selected by" > >> reverse dependency expression in order to identify all those configs > >> which select (IOW *do not allow disabling*) a certain feature (assumed > >> to be not needed). > >> > >> This patch series tries to make one step further and puts at users' > >> fingertips the revdep top level OR sub-expressions grouped/clustered by > >> the tristate value they evaluate to. This should allow the users to > >> directly concentrate on and tackle the active reverse dependencies, > >> which imho are the only ones that matter for a given ARCH and for a > >> given defconfig (nevertheless we still print all of them). > >> > >> Changes v3->v4 (fixed review findings from Ulf): > >> - Remove redundant default cases in switch constructs. > >> - Remove gettext _() tokens in str_append() calls. > >> - Aggregate code repetitions in expr_print_revdep(). > >> > >> Changes v2->v3: > >> - Switch from reverse dependencies prefixed by their tristate value to > >> reverse dependencies grouped by the tristate value they evaluate to. > >> - Skip printing "{Selected,Implied} by [y|m|n]:" if there are no top > >> level OR tokens/sub-expressions that evaluate to y|m|n (suggested > >> by Petr). > >> - Use [1] as template for updating the interface/prototype of > >> __expr_print() (suggested by Ulf). > >> > >> Changes v1->v2: > >> - Don't skip the =n reverse dependency OR tokens, since some users might > >> still need this information (suggested by Ulf). > >> - Instead of using "Selected by" for active tokens only, use it for all > >> OR tokens, but specify the tristate value of each token as prefix > >> (suggested by Masahiro). > >> > >> [1] https://marc.info/?l=linux-kbuild&m=151777006005199&w=4 > >> > >> Eugeniu Rosca (3): > >> kconfig: Print reverse dependencies on new line consistently > >> kconfig: Prepare for printing reverse dependencies in groups > >> kconfig: Print reverse dependencies in groups > >> > >> scripts/kconfig/expr.c | 102 +++++++++++++++++++++++++++++++++++++------- > >> scripts/kconfig/expr.h | 11 ++++- > >> scripts/kconfig/lkc_proto.h | 1 + > >> scripts/kconfig/menu.c | 37 +++++++++++----- > >> 4 files changed, 124 insertions(+), 27 deletions(-) > >> > > > > > > I do not like this implementation. > > The code is super ugly, the diff-stat is too much than needed. > > > > Please rewrite the code within 20 lines. > > > > For a hint, I cleaned up the code base. > https://patchwork.kernel.org/patch/10229545/ > > which should be equivalent to yours: > https://patchwork.kernel.org/patch/10226951/ > > > No 'enum print_type', please. The reason I prefer them on separate lines consistently is to avoid stuff like the following: Selected by [y]: MV_XOR_V2 [=y] && 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 That looks confusing and unbalanced to me. There are some simple ways to trim down the size of this patchset though. Eugeniu: What do you think about the following refactoring of your 3/3 patch? Maybe there's a way to have expr_print_revdep() take just a tristate too, though it's not worth it if it just grows the code elsewhere. (By the way, I noticed that expr_print_revdep() previously generated a warning suggesting parentheses, which was my fault. If you saw that, don't copy my mistakes. The build should be warning-free. :) --- 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 95dc058a236f..db9a89b9bede 100644 --- a/scripts/kconfig/expr.c +++ b/scripts/kconfig/expr.c @@ -1186,10 +1186,9 @@ expr_print_revdep(struct expr *e, int prevtoken, enum print_type type) { - if (type == PRINT_REVDEP_ALL || - type == PRINT_REVDEP_YES && expr_calc_value(e) == yes || - type == PRINT_REVDEP_MOD && expr_calc_value(e) == mod || - type == PRINT_REVDEP_NO && expr_calc_value(e) == no) { + if ((type == PRINT_REVDEP_YES && expr_calc_value(e) == yes) || + (type == PRINT_REVDEP_MOD && expr_calc_value(e) == mod) || + (type == PRINT_REVDEP_NO && expr_calc_value(e) == no)) { fn(data, NULL, "\n - "); expr_print(e, fn, data, prevtoken); } @@ -1212,17 +1211,10 @@ __expr_print(struct expr *e, switch (e->type) { case E_SYMBOL: if (e->left.sym->name) - switch (type) { - case PRINT_NORMAL: + if (type == 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: + else expr_print_revdep(e, fn, data, E_OR, type); - break; - } else fn(data, NULL, ""); break; @@ -1271,18 +1263,12 @@ __expr_print(struct expr *e, __expr_print(e->right.expr, fn, data, E_OR, type); break; case E_AND: - switch (type) { - case PRINT_NORMAL: + if (type == PRINT_NORMAL) { expr_print(e->left.expr, fn, data, E_AND); 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: + } else { expr_print_revdep(e, fn, data, E_OR, type); - break; } break; case E_LIST: @@ -1370,27 +1356,14 @@ void expr_gstr_print(struct expr *e, struct gstr *gs) */ bool expr_revdep_contains(struct expr *e, tristate val) { - bool ret = false; - if (!e) - return ret; + return false; - 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; + if (e->type == E_OR) + return expr_revdep_contains(e->left.expr, val) || + expr_revdep_contains(e->right.expr, val); + + return expr_calc_value(e) == val; } /* 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,