[PATCH v5] kconfig: Print reverse dependencies in groups

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Eugeniu Rosca <erosca@xxxxxxxxxxxxxx>

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") 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 tries to make one step further by putting 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.

To give some numbers and quantify the complexity of certain reverse
dependencies, 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 by" reverse dependency expression.

| Config            | All revdep | Active revdep |
|-------------------|------------|---------------|
| 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 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 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]

Suggested-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
Signed-off-by: Eugeniu Rosca <erosca@xxxxxxxxxxxxxx>
---
v4->v5:
* 99% based on Masahiro's https://patchwork.kernel.org/patch/10231295/
* Requires Masahiro's https://patchwork.kernel.org/patch/10229545/
* Fixes review findings from Petr Vorel:
  * Adds a colon symbol to the end of every "Selected by [y|m|n]"
  * Fixes copy-paste error (s/sym->rev_dep.expr/sym->implied.expr/)
    leading to segfault when searching for PTP_1588_CLOCK in menuconfig.
* Ignores checkpatch findings:
  * "WARNING: line over 80 characters", for the same of readability.
  * "ERROR: Please use git commit description style", which seems to
    be a checkpatch issue.

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().

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).

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).

 scripts/kconfig/expr.c | 18 ++++++++++++------
 scripts/kconfig/expr.h |  3 ++-
 scripts/kconfig/menu.c | 10 ++++++----
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
index cd3a8f501f38..49376e12fa30 100644
--- a/scripts/kconfig/expr.c
+++ b/scripts/kconfig/expr.c
@@ -1323,19 +1323,25 @@ void expr_gstr_print(struct expr *e, struct gstr *gs)
  */
 static void expr_print_revdep(struct expr *e,
 			      void (*fn)(void *, struct symbol *, const char *),
-			      void *data)
+			      void *data, tristate pr_type, const char **title)
 {
 	if (e->type == E_OR) {
-		expr_print_revdep(e->left.expr, fn, data);
-		expr_print_revdep(e->right.expr, fn, data);
-	} else {
+		expr_print_revdep(e->left.expr, fn, data, pr_type, title);
+		expr_print_revdep(e->right.expr, fn, data, pr_type, title);
+	} else if (expr_calc_value(e) == pr_type) {
+		if (*title) {
+			fn(data, NULL, *title);
+			*title = NULL;
+		}
+
 		fn(data, NULL, "  - ");
 		expr_print(e, fn, data, E_NONE);
 		fn(data, NULL, "\n");
 	}
 }
 
-void expr_gstr_print_revdep(struct expr *e, struct gstr *gs)
+void expr_gstr_print_revdep(struct expr *e, struct gstr *gs,
+			    tristate pr_type, const char *title)
 {
-	expr_print_revdep(e, expr_print_gstr_helper, gs);
+	expr_print_revdep(e, expr_print_gstr_helper, gs, pr_type, &title);
 }
diff --git a/scripts/kconfig/expr.h b/scripts/kconfig/expr.h
index c16e82e302a2..8dbf2a4cdae1 100644
--- a/scripts/kconfig/expr.h
+++ b/scripts/kconfig/expr.h
@@ -310,7 +310,8 @@ struct expr *expr_simplify_unmet_dep(struct expr *e1, struct expr *e2);
 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);
+void expr_gstr_print_revdep(struct expr *e, struct gstr *gs,
+			    tristate pr_type, const char *title);
 
 static inline int expr_is_yes(struct expr *e)
 {
diff --git a/scripts/kconfig/menu.c b/scripts/kconfig/menu.c
index 7e70be34aa6c..3415fe0bbad7 100644
--- a/scripts/kconfig/menu.c
+++ b/scripts/kconfig/menu.c
@@ -827,14 +827,16 @@ 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: \n"));
-		expr_gstr_print_revdep(sym->rev_dep.expr, r);
+		expr_gstr_print_revdep(sym->rev_dep.expr, r, yes, "  Selected by [y]:\n");
+		expr_gstr_print_revdep(sym->rev_dep.expr, r, mod, "  Selected by [m]:\n");
+		expr_gstr_print_revdep(sym->rev_dep.expr, r, no, "  Selected by [n]:\n");
 	}
 
 	get_symbol_props_str(r, sym, P_IMPLY, _("  Implies: "));
 	if (sym->implied.expr) {
-		str_append(r, _("  Implied by: \n"));
-		expr_gstr_print_revdep(sym->implied.expr, r);
+		expr_gstr_print_revdep(sym->implied.expr, r, yes, "  Implied by [y]:\n");
+		expr_gstr_print_revdep(sym->implied.expr, r, mod, "  Implied by [m]:\n");
+		expr_gstr_print_revdep(sym->implied.expr, r, no, "  Implied by [n]:\n");
 	}
 
 	str_append(r, "\n\n");
-- 
2.16.2

--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux&nblp;USB Development]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite Secrets]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux