linux-kernel - Re: [PATCH v2] macintosh/via-pmu-led: make disk activity usage a parameter.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87fssox7ah.fsf@linux.ibm.com>
Date:   Tue, 26 Oct 2021 08:08:38 -0500
From:   Nathan Lynch <nathanl@...ux.ibm.com>
To:     Hill Ma <maahiuzeon@...il.com>
Cc:     linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org,
        benh@...nel.crashing.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v2] macintosh/via-pmu-led: make disk activity usage a
 parameter.

Hello,

Hill Ma <maahiuzeon@...il.com> writes:
> Whether to use the LED as a disk activity is a user preference.
> Some like this usage while others find the LED too bright. So it
> might be a good idea to make this choice a runtime parameter rather
> than compile-time config.

Users already have the ability to change the LED behavior at runtime
already, correct? I.e. they can do:

  echo none > /sys/class/leds/pmu-led::front/trigger

in their boot scripts. Granted, a kernel built with ADB_PMU_LED_DISK=y
will blink the LED on disk activity until user space is running. Is this
unsatisfactory?

> The default is set to disabled as OS X does not use the LED as a
> disk activity indicator.

This is long-standing behavior in Linux and OS X has been EOL on this
architecture for a decade, so this isn't much of a consideration at this
point. Seems more important to avoid surprising existing users and
distributions with a behavior change that makes additional work for
them. See below.

> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 43dc35fe5bc0..a656a51ba0a8 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -250,6 +250,12 @@
>  			Use timer override. For some broken Nvidia NF5 boards
>  			that require a timer override, but don't have HPET
>  
> +	adb_pmu_led_disk [PPC]
> +			Use front LED as disk LED by default. Only applies to
> +			PowerBook, iBook, PowerMac 7,2/7,3.
> +			Format: <bool>  (1/Y/y=enable, 0/N/n=disable)
> +			Default: disabled
> +
>  	add_efi_memmap	[EFI; X86] Include EFI memory map in
>  			kernel's map of available physical RAM.
>  
> diff --git a/drivers/macintosh/Kconfig b/drivers/macintosh/Kconfig
> index 5cdc361da37c..243215de563c 100644
> --- a/drivers/macintosh/Kconfig
> +++ b/drivers/macintosh/Kconfig
> @@ -78,16 +78,6 @@ config ADB_PMU_LED
>  	  behaviour of the old CONFIG_BLK_DEV_IDE_PMAC_BLINK, select this
>  	  and the disk LED trigger and configure appropriately through sysfs.
>  
> -config ADB_PMU_LED_DISK
> -	bool "Use front LED as DISK LED by default"
> -	depends on ADB_PMU_LED
> -	depends on LEDS_CLASS
> -	select LEDS_TRIGGERS
> -	select LEDS_TRIGGER_DISK
> -	help
> -	  This option makes the front LED default to the disk trigger
> -	  so that it blinks on disk activity.
> -

So, if I've been relying on CONFIG_ADB_PMU_LED_DISK=y and I upgrade to a
newer kernel with the proposed change, from my point of view the disk
activity LED has stopped working and I need to alter the bootloader
config or init scripts to restore the expected behavior. That seems
undesirable to me.

I don't think we rigidly enforce Kconfig backward compatibility, but
when it comes to a user-visible function on a legacy platform where
users and distros likely have their configurations figured out already,
it's probably best to avoid such changes.