Re: [LTP] [PATCH 09/13] testcases/kernel/mem: Move KSM bits to ksm tests

Cyril Hrubis
2025-02-10T11:17:47Z

Hi!
> > new file mode 100644
> > index 000000000..ebce8197c
> > --- /dev/null
> > +++ b/testcases/kernel/mem/ksm/ksm_test.h
> ...
> > +static inline void ksm_child_memset(int child_num, int size, int total_unit,
> > + struct ksm_merge_data ksm_merge_data, char **memory)
> > +{
> > + int i = 0, j;
> > + int unit = size / total_unit;
> > +
> > + tst_res(TINFO, "child %d continues...", child_num);
> > +
> > + if (ksm_merge_data.mergeable_size == size * TST_MB) {
> This introduces new warnings, because the original code used MB which is long.
>
> #define MB (1UL<<20)
>
> Now we use TST_MB, which is plain int.
>
> In file included from ksm_common.h:16,
> from ksm04.c:42:
> ksm_test.h: In function ‘ksm_child_memset’:
> ksm_test.h:108:43: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Wsign-compare]
> 108 | if (ksm_merge_data.mergeable_size == size * TST_MB) {
> | ^~
> ksm_test.h:119:45: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Wsign-compare]
> 119 | for (i = 0; (unsigned int)i < unit * TST_MB; i++)
> | ^
> ksm_test.h:126:43: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Wsign-compare]
> 126 | if (ksm_merge_data.mergeable_size < size * TST_MB)
> | ^
> ksm_test.h: In function ‘create_ksm_child’:
> ksm_test.h:166:54: warning: comparison of integer expressions of different signedness: ‘unsigned int’ and ‘int’ [-Wsign-compare]
> 166 | if (ksm_merge_data[j].mergeable_size < size * TST_MB) {
> | ^
> It would be nice to add cast before merge.

So the size is signed but it was implicitly casted to unsigned by the
TST_MB being unsigned. I guess that the best fix here is to make the
size and unit unsigned to begin with since these are numbers that should
not be negative at all.

> > + tst_res(TINFO, "child %d allocates %d TST_MB filled with '%c'",
>
> This should use MB (replace to whole file obviously should exclude strings):
>
> tst_res(TINFO, "child %d allocates %d MB filled with '%c'",
>
> > + child_num, size, ksm_merge_data.data);
> > +
> > + } else {
> > + tst_res(TINFO, "child %d allocates %d TST_MB filled with '%c'"
>
> And here as well.
> tst_res(TINFO, "child %d allocates %d MB filled with '%c'"
> > + " except one page with 'e'",
> > + child_num, size, ksm_merge_data.data);
> > + }

Ah, sorry, that is a result from blindly doing sed s/...

--
Cyril Hrubis
chrubis@suse.cz