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

[Petr Vorel <pvorel@suse.cz>]

Hi Cyril,

> 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.

> +		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);
> +	}

Otherwise LGTM.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp