Message-ID: <34e27c57-fc18-4918-bf44-4f8a53825361@infradead.org>
Date: Tue, 5 Dec 2023 14:10:31 -0800
From: Randy Dunlap <rdunlap@...radead.org>
To: Andy Shevchenko <andy@...nel.org>,
 Nick Desaulniers <ndesaulniers@...gle.com>
Cc: Al Viro <viro@...iv.linux.org.uk>, tanzirh@...gle.com,
 Kees Cook <keescook@...omium.org>, linux-hardening@...r.kernel.org,
 linux-kernel@...r.kernel.org, Nick DeSaulniers <nnn@...gle.com>,
 Andrew Morton <akpm@...ux-foundation.org>, llvm@...ts.linux.dev
Subject: Re: [PATCH] lib/string: shrink lib/string.i via IWYU



On 12/5/23 14:01, Andy Shevchenko wrote:
> On Tue, Dec 05, 2023 at 01:51:10PM -0800, Nick Desaulniers wrote:
>> On Tue, Dec 5, 2023 at 1:38 PM Al Viro <viro@...iv.linux.org.uk> wrote:
>>> On Tue, Dec 05, 2023 at 08:58:53PM +0000, tanzirh@...gle.com wrote:
> 
> ...
> 
>>>> IWYU is implemented using the IWYUScripts github repository which is a tool that is
>>>> currently undergoing development. These changes seek to improve build times.
>>>>
>>>> This change to lib/string.c resulted in a preprocessed size of
>>>> lib/string.i from 26371 lines to 5232 lines (-80%).
>>>
>>> It also breeds includes of asm/*.h, by the look of the output, which is
>>> not a good thing in general ;-/  E.g. #include <asm/uaccess.h> *anywhere*
>>> outside of linux/uaccess.h is a bad idea.
>>
>> It's not clear to me when it's ok to #include <asm/*.h>.  Is there a
>> convention here that I'm missing?
> 
> The mandatory ones can be used, but not all of them.
> In some cases you even must include asm and not linux
> (unaligned.h, byteorder.h, maybe others...).
> 
> As I told, it comes with experience, we lack of the
> respective documentation (or file which is good for
> automation checks, like with IWYU).

I have an unpublished Linux Best Known Practices txt file with
this "rule" and some other info that I have collected over a few
years. I wouldn't say that it's up to date. Anyway, it's copied below
FWIW.

-- 
~Randy

---

=================================
Linux Kernel Best Known Practices
=================================

N.B.: This is an evolving document.


No need to init variables in bss to 0. bss is always cleared to 0.

User interfaces should use explicit types (and hence type sizes)
instead of generic types like "long" or "unsigned int".
E.g.
Also see Documentation/process/botching-up-ioctls.rst

Unused fields of API structures or flags bits should be checked
for value 0 on input and warn if they are not 0.

submit-checklist Rule #1.

use the kmalloc() helpers for variable-size array allocations.

Generic drivers should not (do not) #include header files from
<asm-generic>. Only arch code and arch-specific drivers do that.

Don't use <asm/header.h> in generic driver code. Instead use
some <linux/header.h> that then #includes <asm/header.h>.
asm/ headers are implementation; linux/ headers are API.

Use __maybe_unused to driver suspend/resume functions instead of
surrounding them with #ifdef CONFIG_PM_SLEEP / #endif.

Many API families provide full stub support for when their
Kconfig option is disabled. This means that these APIs can still be used
with COMPILE_TEST. Examples of these API families are:
CONFIG_OF, <more>.

CONFIG_namespace belongs to Kconfig files. A driver or filesystem or
whatever should not have a local macro (or variable!) whose name
begins with "CONFIG_".

see Documentation/process/submit-checklist.rst

run scripts/checkpatch.pl

use Documentation/core-api/printk-formats.rst

Don't introduce any new build errors or warnings
Yes, that means that you need to check for warnings when you build.
Yes, that includes Documentation or kernel-doc warnings or errors
  (now that we have those down to zero).

Do NOT use stack space for DMA.

Don't use 'extern' on function prototypes. (deprecated)

Don't depend on Makefile order to control builtin module/driver init
calls. Use initcall levels instead.

In a Kconfig dependency list,
by convention, please keep "|| COMPILE_TEST" last in the list.

Shell scripts should use /bin/sh instead of another shell
as much as possible.

When making a patch such as a coding-style patch that should have
no effect on the produced code, do a before/after comparison of the
binary files to show that they are the same. If they are not the same,
explain why not.

Most scripts in the kernel source tree try to avoid bashisms.
There may be some exceptions to this, such as bpf-related scripts
and perf-related scripts.

Don't use BIT() macros in UAPI headers. Either open-code bit fields
or use _BITUL().

"sigcontext is accessible to user; do *NOT* assume that sigreturn will find what the kernel had written there" (Viro)

Please just use whole numbers for patch versions, there's no need for .X
for this type of thing as our tools do not know how to catch that, and
it makes no sense.
Just keep bumping the number please. (gkh)

C99 style comments are OK in source code except in UAPI header files.
They may be used with cstd=C90, which does not accept // comments.

TODO:

Should we say something about __delay() not being available in all
arch-es so some other delay function is preferable.
[maybe add something like this to checkpatch?]

#####

Powered by blists - more mailing lists