On Thu, Jun 27, 2024 at 01:20:30PM -0400, Liam R. Howlett wrote:
[snip]
> > +
> > +clean:
> > + $(RM) $(TARGETS) *.o radix-tree.c idr.c generated/map-shift.h generated/bit-length.h
>
> This needs to clean out vma.c to avoid stale testing.
>
> But, none of this is needed.
>
> What we can do instead is add the correct header guards to the
> mm/vma_internal.h file, change the tools/testing/vma/vma_internal.h
> header guards to be the same (ie: remove TESTING_ from the existing
> ones), then we can include vma_internal.h into vma_stub.c prior to
> including "../../../mm/vma.c", and we don't need to copy the file.
>
> Essentially use the #ifdef guards to replace the header by ordering the
> local header for inclusion prior to the c file.
Ack this is a good idea, will do in v2.
>
>
> > diff --git a/tools/testing/vma/errors.txt b/tools/testing/vma/errors.txt
> > new file mode 100644
> > index 000000000000..e69de29bb2d1
> > diff --git a/tools/testing/vma/generated/autoconf.h b/tools/testing/vma/generated/autoconf.h
> > new file mode 100644
> > index 000000000000..92dc474c349b
> > --- /dev/null
> > +++ b/tools/testing/vma/generated/autoconf.h
> > @@ -0,0 +1,2 @@
> > +#include "bit-length.h"
> > +#define CONFIG_XARRAY_MULTI 1
> > diff --git a/tools/testing/vma/linux/atomic.h b/tools/testing/vma/linux/atomic.h
> > new file mode 100644
> > index 000000000000..298b0fb7aab2
> > --- /dev/null
> > +++ b/tools/testing/vma/linux/atomic.h
>
> This should have header guards as well.
Yup, the reason I kept it like this is because existing linux/*.h headers
in shared/linux didn't have header guards and I wanted to keep things in
line with that... will change.
>
> > @@ -0,0 +1,19 @@
> > +#ifndef atomic_t
> > +#define atomic_t int32_t
> > +#endif
> > +
> > +#ifndef atomic_inc
> > +#define atomic_inc(x) uatomic_inc(x)
> > +#endif
> > +
> > +#ifndef atomic_read
> > +#define atomic_read(x) uatomic_read(x)
> > +#endif
> > +
> > +#ifndef atomic_set
> > +#define atomic_set(x, y) do {} while (0)
> > +#endif
> > +
> > +#ifndef U8_MAX
> > +#define U8_MAX UCHAR_MAX
> > +#endif
> > diff --git a/tools/testing/vma/linux/mmzone.h b/tools/testing/vma/linux/mmzone.h
> > new file mode 100644
> > index 000000000000..71546e15bdd3
> > --- /dev/null
> > +++ b/tools/testing/vma/linux/mmzone.h
> > @@ -0,0 +1,37 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +#ifndef _TOOLS_MMZONE_H
> > +#define _TOOLS_MMZONE_H
>
> It might be best to use the same guards here to avoid mmzone.h from
> getting pulled in.
You mean the actual [root]/include/linux/mmzone.h ? Just deploying the same
header guard trick as mentioned above re: vma_internal.h?
[snip]
> > new file mode 100644
> > index 000000000000..b29eeb0daf31
> > --- /dev/null
> > +++ b/tools/testing/vma/main.c
>
> If you employ the use of header guards, we can rename main.c to vma.c
> and produce the executable "vma" instead of "main".
Sure, will do.
>
> > @@ -0,0 +1,161 @@
> > +/* SPDX-License-Identifier: GPL-2.0 */
> > +
> > +#include <assert.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +
> > +#include "maple-shared.h"
> > +#include "vma_internal.h"
> > +#include "vma.h"
>
> You can directly include "../../../mm/vma.h" here and remove the vma.h
> file you have in this directory.
This was, I think, to keep to a convention, but you're right I don't think
there's any reason to do this, will change.
[snip]
> > +int main(void)
> > +{
> > + maple_tree_init();
> > +
> > + test_simple_merge();
> > + test_simple_modify();
> > + test_simple_expand();
> > + test_simple_shrink();
> > +
> > + return EXIT_SUCCESS;
> > +}
>
> It would be nice to have some output stating the number of tests
> passed/failed.
Ack, will add.
>
> > diff --git a/tools/testing/vma/vma.h b/tools/testing/vma/vma.h
> > new file mode 100644
> > index 000000000000..87a6cb222b63
> > --- /dev/null
> > +++ b/tools/testing/vma/vma.h
> > @@ -0,0 +1,3 @@
> > +/* SPDX-License-Identifier: GPL-2.0+ */
> > +
> > +#include "../../../mm/vma.h"
>
> I'd rather just drop this file and have this line in main.c (or vma.c if
> you decide to rename it).
Ack, will do.
[snip]
