Re: [PATCH] kconfig: Minimize 'Selected by' and 'Implied by'

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

 



On Sun, Feb 4, 2018 at 3:46 PM, Ulf Magnusson <ulfalizer@xxxxxxxxx> wrote:
> On Sun, Feb 4, 2018 at 12:37 PM, Eugeniu Rosca <roscaeugeniu@xxxxxxxxx> wrote:
>> From: Eugeniu Rosca <erosca@xxxxxxxxxxxxxx>
>>
>> Commit 1ccb27143360 ("kconfig: make "Selected by:" and "Implied by:"
>> readable") made an incredible improvement in how reverse dependencies
>> are perceived by the user, by breaking down the single (often
>> interminable) expression string into small readable chunks.
>>
>> Even so, what happens in practice when reading the reverse
>> dependencies is that 80-90% of the OR sub-expressions simply don't
>> matter, since they evaluate to [=n].
>>
>> 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 30 of CONFIG options with
>> the highest amount of OR sub-expressions 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            |
>> | IRQ_DOMAIN                    | 75         | 19           |
>> | SERIAL_CORE                   | 75         | 9            |
>> | PHYLIB                        | 74         | 16           |
>> | REGMAP_MMIO                   | 72         | 15           |
>> | GENERIC_PHY                   | 67         | 20           |
>> | DMA_ENGINE                    | 66         | 11           |
>> | SERIAL_CORE_CONSOLE           | 64         | 9            |
>> | CRYPTO_BLKCIPHER              | 64         | 13           |
>> | PINMUX                        | 60         | 17           |
>> | CRYPTO                        | 59         | 10           |
>> | MII                           | 58         | 8            |
>> | GPIOLIB_IRQCHIP               | 58         | 9            |
>> | MFD_SYSCON                    | 58         | 15           |
>> | VIDEOBUF2_DMA_CONTIG          | 46         | 4            |
>> | REGMAP_IRQ                    | 45         | 6            |
>> | REGMAP_SPI                    | 44         | 2            |
>> | CLKSRC_MMIO                   | 42         | 5            |
>> | SND_SOC_GENERIC_DMAENGINE_PCM | 41         | 3            |
>> | CRYPTO_SHA1                   | 37         | 2            |
>> | REGMAP                        | 36         | 4            |
>>
>> The story behind the above is that we still 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.
>>
>> This patch attempts to bring at user's fingertips those reverse
>> dependencies that actually participate in selection of given symbol
>> filtering out the rest of them.
>>
>> Signed-off-by: Eugeniu Rosca <erosca@xxxxxxxxxxxxxx>
>> ---
>>  scripts/kconfig/expr.c | 24 +++++++++++++++++-------
>>  1 file changed, 17 insertions(+), 7 deletions(-)
>>
>> diff --git a/scripts/kconfig/expr.c b/scripts/kconfig/expr.c
>> index 2ba332b3fed7..147b2d8a8f3e 100644
>> --- a/scripts/kconfig/expr.c
>> +++ b/scripts/kconfig/expr.c
>> @@ -1234,14 +1234,24 @@ static void __expr_print(struct expr *e, void (*fn)(void *, struct symbol *, con
>>                 fn(data, e->right.sym, e->right.sym->name);
>>                 break;
>>         case E_OR:
>> -               if (revdep && e->left.expr->type != E_OR)
>> -                       fn(data, NULL, "\n  - ");
>> -               __expr_print(e->left.expr, fn, data, E_OR, revdep);
>> -               if (revdep)
>> -                       fn(data, NULL, "\n  - ");
>> -               else
>> +               if (revdep) {
>> +                       struct expr *left = e->left.expr;
>> +                       struct expr *right = e->right.expr;
>> +
>> +                       if (expr_calc_value(left) != no) {
>> +                               if (left->type != E_OR)
>> +                                       fn(data, NULL, "\n  - ");
>> +                               __expr_print(left, fn, data, E_OR, revdep);
>> +                       }
>> +                       if (expr_calc_value(right) != no) {
>> +                               fn(data, NULL, "\n  - ");
>> +                               __expr_print(right, fn, data, E_OR, revdep);
>> +                       }
>> +               } else {
>> +                       __expr_print(e->left.expr, fn, data, E_OR, revdep);
>>                         fn(data, NULL, " || ");
>> -               __expr_print(e->right.expr, fn, data, E_OR, revdep);
>> +                       __expr_print(e->right.expr, fn, data, E_OR, revdep);
>> +               }
>>                 break;
>>         case E_AND:
>>                 expr_print(e->left.expr, fn, data, E_AND);
>> --
>> 2.16.1
>>
>
> Hello,
>
> One downside to this is that people might expect e.g. the '?'
> menuconfig screen to list all the selecting symbols and use it as a
> reference.
>
> The best solution IMO would be to have a separate "Currently selected
> by:" section on that screen, listing just the non-n selects. The
> simpler next best thing would be to just replace the "Selected by:"
> heading with "Currently selected by:", to make it clear that it
> includes just the active selects.
>
> For the most-selected symbols you listed, most of them end up as "m"
> on my system by the way, because they come from drivers compiled in as
> modules.

Most of the selecting symbols that is.

> "n" is the minority. Might want to check that most of the
> ones with a million selects aren't like that, because it might not be
> that hard to see what's going on for those anyway.
>
> I used a similar approach in
> https://github.com/ulfalizer/Kconfiglib/blob/master/kconfiglib.py#L3022
> by the way. I was always a bit worried that all the expression
> simplification shenanigans going on in the C implementation might mess
> with an approach like that, but it seems fine in practice. :)
>
> Cheers,
> Ulf
--
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