Re: [ELISA Development Process WG] [linux-safety] [PATCH] mm: vmscan: provide a change to the development-process group

On Thu, 17 Sep 2020, Sudip Mukherjee wrote:

> 
> 
> On 17/09/2020 16:29, Lukas Bulwahn wrote:
> > 
> > 
> > On Thu, 17 Sep 2020, Sudip Mukherjee wrote:
> > 
> >>
> >>
> >> On 17/09/2020 09:44, Lukas Bulwahn wrote:
> >>> I think this change is needed for safety, whatever that might mean to you.
> >>>
> >>> I am unqualified to really make a change here, as I have no clue what this
> >>> code does, nor what my change does, but sure, the testing and verification
> >>> reference process can now point out the required next steps in the
> >>> reference process to test this code and code change.
> >>>
> >>> Good luck :)
> >>>
> >>> Not intended for distribution to the general kernel mailing lists.
> >>>
> >>> Signed-off-by: Lukas Bulwahn <[email protected]>
> >>> ---
> >>> I would like to submit such a patch, what do I need to do according to
> >>> the expected testing and verification recommendations for safety-related
> >>> systems?
> >>>
> >>> Please help me. What do I need to compile, what test do I need to run,
> >>> which verification tool do I need to employ for this change?
> >>
> >> The change looks valid, 'reclaim_order' has not been used anywhere after
> >> READ_ONCE(), and its  So, it looks like a harmless change, you will only
> >> need a good commit message detailing why its harmless.
> >>
> > 
> > Thanks, Sudip. Yes, I also conclude it is harmless but I really cannot say 
> > as I did not even compile it :) and I guess you did not either :)
> > 
> > I would actually want to argue that I compiled it for all available (and 
> > relevant) kernel configurations and the binary is identical before and 
> > after the change.
> 
> That, I dont think is possible. The maintainers will receive thousands
> of patch in a day. If they have to compile each for all available
> configuration (and arch), then they might spend the full day just
> checking patches. Also, many upstream contributors contribute in their
> personal spare time, so if building in all possible configuration
> becomes a requirement then I think that is going to discourage many
> contributors from contributing.
>

I am not asking the maintainer, I would only waste my own energy bill on 
that :) if I would what to do...

But even finding out which configurations actually make a difference is an 
unsolved challenge, right? The experts know, but how would I find out?

> > 
> > It is a Dead Store, so I expect the compiler to detect that and just 
> > optimize that away...
> 
> Which is also something I always think, we are relying on the compiler
> to produce the code that is actually executed on the hardware. There are
> different compilers and each compiler has different versions, so that
> means the generated code is going to be different. Even though we say
> Linus Kernel meets the safety requirement, can we say that the kernel
> that is executing on the hardware meets the safety requirement? This, I
> think is completely off-topic for this WG, but just a thought.
>

Yes, let us keep it simple for now; but you are right. This whole story of 
'test and verification' fully independently quickly breaks... but let the
group figure that out.
  
> > 
> >> So, from a safety pov, is it a requirement that every submitted patch
> >> will need to be tested based on the safety tests and all the other
> >> defined tests?
> >>
> > 
> > Well, I do not know what Roberto thinks his reference process is good for, 
> > but I would like to know if Roberto thinks it can guide anyone on such a 
> > question or not?
> > 
> > It is really just some fun for the discussion in this group... there are 
> > thousands of commits travelling into the kernel... if we cannot provide 
> > an answer for a single one, how to do it for thousands?
> 
> Lets have another example of a change.
> 
> diff --git a/drivers/staging/rtl8712/rtl871x_ioctl_rtl.c
> b/drivers/staging/rtl8712/rtl871x_ioctl_rtl.c
> index b78101afc93d..2b539335206a 100644
> --- a/drivers/staging/rtl8712/rtl871x_ioctl_rtl.c
> +++ b/drivers/staging/rtl8712/rtl871x_ioctl_rtl.c
> @@ -367,7 +367,6 @@ uint oid_rt_get_scan_in_progress_hdl(struct
> oid_par_priv *poid_par_priv)
>         return RNDIS_STATUS_SUCCESS;
>  }
> 
> -
>  uint oid_rt_forced_data_rate_hdl(struct oid_par_priv *poid_par_priv)
>  {
>         return RNDIS_STATUS_SUCCESS;
> 
> 
> 
> Not a formal patch, just pasted the git diff. This fixes the checkpatch
> warning of "Please don't use multiple blank lines". What tests are
> needed on this patch to say that the kernel development meets the safety
> requirement?
> 
>

Nice :) Sudip, You are making the kernel safer, Yeah! ;)

Reference process, where are thou?

Lukas