ACK/Cmnt: ACK+Cmnt: [SRU][F][PATCH 0/1] Fix build issues in selftest/memfd

[Luke Nowakowski-Krijger]

On Tue, Aug 31, 2021 at 09:32:35AM +0200, Stefan Bader wrote:
> On 31.08.21 01:56, Thadeu Lima de Souza Cascardo wrote:
> > On Mon, Aug 30, 2021 at 04:11:56PM -0700, Luke Nowakowski-Krijger wrote:
> > > [Impact]
> > > BugLink: https://bugs.launchpad.net/bugs/1926142
> > > 
> > > There are build issues in selftest/memfd that are leading to regression
> > > and adt test failures across all B/5.4 and possibly hwe kernels due to
> > > selftests being built against headers that do not have the newer
> > > definitions used by the tests.
> > > 
> > > [Fix]
> > > * selftests/memfd: fix build when F_SEAL_FUTURE_WRITE is not defined
> > > 
> > > [Test Plan]
> > > Recompile tests to make sure they build and tests pass.
> > > 
> > > [Where problems could occur]
> > > Unlikely to be any problems because userspace apis should be backwards
> > > compatiable and the definition we are trying to copy should not be
> > > changing.
> > > 
> > > [Other Info]
> > > Should not go upstream.
> > 
> > Honestly, I don't see why not. If you look at
> > tools/testing/selftests/seccomp/seccomp_bpf.c, you will notice a lot of these
> > definitions.
> > 
> > These tests are supposed to be used with the kernels they are shipped with, and
> > these kernels do support these features. And they should build on systems with
> > not as recent system headers as the kernels.

That makes sense to me. As Stefan mentioned below there does seem to be sort
of a grey area on whether to respect the API to test things as user
space sees it, but since its a kernel selftest I would say that testing
the feature is more important. 

I can submit a patch for this upstream with this justification and see
what they think about it. 

> 
> First for Luke, you also have to add the SRU justification to the bug report
> (preferably extend the description). That is the place which must have the
> justification. The cover email may repeat this but can be more geared to
> give reviewers technical info. While the SRU justification in the bug report
> is for a different (at least in theory) audience (SRU team which do not have
> to be kernel devs and testers). And thus is more high level.
> 

Okay that makes sense. Ill make sure to include the SRU justification in
future patches in the bug report. Thank you for informing me.

> For Thadeu: Generally I wonder whether the selftests could change to prefer
> the kernel specific headers over the global uapi ones. On the other hand I
> suppose it is more of interest to see whether things which a normal
> user-space application would do is working. And since those only use the
> global headers they would not be able to make use of new functionality.
> Probably a general issue what to provide and how.
>
> Acked-by: Stefan Bader <stefan.bader at canonical.com>
> > 
> > Cascardo.
> > 
> > > 
> > > Luke Nowakowski-Krijger (1):
> > >    selftests/memfd: fix build when F_SEAL_FUTURE_WRITE is not defined
> > > 
> > >   tools/testing/selftests/memfd/memfd_test.c | 4 ++++
> > >   1 file changed, 4 insertions(+)
> > > 
> > > -- 
> > > 2.30.2
> > > 
> > > 
> > > -- 
> > > kernel-team mailing list
> > > kernel-team at lists.ubuntu.com
> > > https://lists.ubuntu.com/mailman/listinfo/kernel-team
> > 
> 
> 

- Luke