All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sergey Ryazanov <ryazanov.s.a@gmail.com>
To: Johannes Berg <johannes@sipsolutions.net>
Cc: "David S . Miller" <davem@davemloft.net>,
	Jakub Kicinski <kuba@kernel.org>,
	netdev@vger.kernel.org, M Chetan Kumar <m.chetan.kumar@intel.com>,
	Intel Corporation <linuxwwan@intel.com>,
	Loic Poulain <loic.poulain@linaro.org>,
	Leon Romanovsky <leon@kernel.org>
Subject: Re: [PATCH RESEND net-next 5/5] net: wwan: core: make debugfs optional
Date: Mon, 29 Nov 2021 02:45:16 +0300	[thread overview]
Message-ID: <CAHNKnsSgc0bEwJbS01f26JRLpnzky9mcSJ6sWy2vFDuNOHz-Xw@mail.gmail.com> (raw)
In-Reply-To: <ac532d400cd61a0f86ad5b1931df87a83582323c.camel@sipsolutions.net>

Add Leon to CC to merge both conversations.

On Sun, Nov 28, 2021 at 8:01 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> On Sun, 2021-11-28 at 15:55 +0300, Sergey Ryazanov wrote:
>>
>> +config WWAN_DEBUGFS
>> +     bool "WWAN subsystem common debugfs interface"
>> +     depends on DEBUG_FS
>> +     help
>> +       Enables common debugfs infrastructure for WWAN devices.
>> +
>> +       If unsure, say N.
>>
>
> I wonder if that really should even say "If unsure, say N." because
> really, once you have DEBUG_FS enabled, you can expect things to show up
> there?
>
> And I'd probably even argue that it should be
>
>         bool "..." if EXPERT
>         default y
>         depends on DEBUG_FS
>
> so most people aren't even bothered by the question?
>
>
>>  config WWAN_HWSIM
>>       tristate "Simulated WWAN device"
>>       help
>> @@ -83,6 +91,7 @@ config IOSM
>>  config IOSM_DEBUGFS
>>       bool "IOSM Debugfs support"
>>       depends on IOSM && DEBUG_FS
>> +     select WWAN_DEBUGFS
>>
> I guess it's kind of a philosophical question, but perhaps it would make
> more sense for that to be "depends on" (and then you can remove &&
> DEBUG_FS"), since that way it becomes trivial to disable all of WWAN
> debugfs and not have to worry about individual driver settings?
>
>
> And after that change, I'd probably just make this one "def_bool y"
> instead of asking the user.

When I was preparing this series, my primary considered use case was
embedded firmwares. For example, in OpenWrt, you can not completely
disable debugfs, as a lot of wireless stuff can only be configured and
monitored with the debugfs knobs. At the same time, reducing the size
of a kernel and modules is an essential task in the world of embedded
software. Disabling the WWAN and IOSM debugfs interfaces allows us to
save 50K (x86-64 build) of space for module storage. Not much, but
already considerable when you only have 16MB of storage.

I personally like Johannes' suggestion to enable these symbols by
default to avoid bothering PC users with such negligible things for
them. One thing that makes me doubtful is whether we should hide the
debugfs disabling option under the EXPERT. Or it would be an EXPERT
option misuse, since the debugfs knobs existence themself does not
affect regular WWAN device use.

Leon, would it be Ok with you to add these options to the kernel
configuration and enable them by default?

-- 
Sergey

  reply	other threads:[~2021-11-28 23:47 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-28 12:55 [PATCH RESEND net-next 0/5] WWAN debugfs tweaks Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 1/5] net: wwan: iosm: consolidate trace port init code Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 2/5] net: wwan: iosm: allow trace port be uninitialized Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 3/5] net: wwan: iosm: move debugfs knobs into a subdir Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 4/5] net: wwan: iosm: make debugfs optional Sergey Ryazanov
2021-11-28 12:55 ` [PATCH RESEND net-next 5/5] net: wwan: core: " Sergey Ryazanov
2021-11-28 17:01   ` Johannes Berg
2021-11-28 23:45     ` Sergey Ryazanov [this message]
2021-11-29  6:40       ` Leon Romanovsky
2021-11-29 23:44         ` Sergey Ryazanov
2021-11-30 10:05           ` Leon Romanovsky
2021-12-01 22:03             ` Sergey Ryazanov
2021-12-02 12:03               ` Leon Romanovsky
2021-12-02 22:42                 ` Sergey Ryazanov
2021-11-28 17:04   ` kernel test robot
2021-11-28 17:04     ` kernel test robot
2021-11-28 17:05   ` Johannes Berg
2021-11-28 22:57     ` Sergey Ryazanov
2021-11-28 18:27 ` [PATCH RESEND net-next 0/5] WWAN debugfs tweaks Leon Romanovsky
2021-11-30  8:03   ` Johannes Berg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAHNKnsSgc0bEwJbS01f26JRLpnzky9mcSJ6sWy2vFDuNOHz-Xw@mail.gmail.com \
    --to=ryazanov.s.a@gmail.com \
    --cc=davem@davemloft.net \
    --cc=johannes@sipsolutions.net \
    --cc=kuba@kernel.org \
    --cc=leon@kernel.org \
    --cc=linuxwwan@intel.com \
    --cc=loic.poulain@linaro.org \
    --cc=m.chetan.kumar@intel.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.