On Thu, Dec 7, 2023 at 6:25 PM Johan.Erlandsson@sony.com <Johan.Erlandsson@sony.com> wrote:
>>>>> (gdb)  p nr_swapper_spaces
>>>>> $4 = <optimized out>
>>>> Thank you folks, for the various information.
>>>>
>>>> Apparently the data of nr_swapper_spaces exists in the kernel, but its
>>>> symbol does not exist.  I think this means that we cannot calculate the
>>>> number of pages from swapper_spaces[] with such a debuginfo, because we
>>>> cannot get the array size of a swapper_spaces entry.
>>> That's correct, Kazu.
>>>
>>>
>>>> So I'd like to go with Johan's patch.
>>>>
>>>> cc -c -g -DX86_64 -DLZO -DSNAPPY -DGDB_10_2  memory.c -Wall -O2
>>>> -Wstrict-prototypes -Wmissing-prototypes -fstack-protector
>>>> -Wformat-security
>>>> memory.c: In function dump_kmeminfo:
>>>> memory.c:8612:50: warning: pointer targets in passing argument 2 of
>>>> dump_vm_stat differ in signedness [-Wpointer-sign]
>>>>     8612 |                 if (dump_vm_stat("NR_SWAPCACHE",
>>>> &swapper_space_nrpages, 0)) {
>>>>          |
>>>> ^~~~~~~~~~~~~~~~~~~~~~
>>>>          |                                                  |
>>>>          |                                                  ulong * {aka
>>>> long unsigned int *}
>>>> memory.c:298:33: note: expected long int * but argument is of type ulong
>>>> * {aka long unsigned int *}
>>>>      298 | static int dump_vm_stat(char *, long *, ulong);
>>>>          |                                 ^~~~~~
>>>>
>>>> So with the following,
>>>>
>>>> -       ulong swapper_space_nrpages;
>>>> +       long swapper_space_nrpages;
>>> Looks good. Maybe the patch log also needs to be improved a little bit.
>>>
>>> Although the checking of kernel version looks not very good, seems no
>>> better way.
>> hmm, this sounds strange.  My ack is for Johan's patch (attached), which
>
>Ah, I did not see the attached patch. Thank you for pointing out this
>issue, Kazu.
>
>> does not have kernel version check, is this ok?  sorry for confusing.
>
>Sounds good idea if there is no kernel version check.
>
>I still have one question:  the following two commits were introduced
>separately in kernel 5.12 and 4.11.
>
>b6038942480e ("mm: memcg: add swapcache stat for memcg v2")
>
>4b3ef9daa4fc ("mm/swap: split swap cache into 64MB trunks")
>
>It indicates that the attached patch may not work well between kernel
>4.11 and 5.12, is it possible to encounter the similar issue(the
>nr_swapper_spaces may be optimized away)?

I have not noticed any problems with the attached patch on
earlier kernel versions. For instance on GKI kernels based on 5.10
[1] the 'nr_swapper_spaces' variable is not optimized out.


Thank you for the feedback, Johan. It makes sense,  let's leave it there if the above issue does not occur in the actual production environment.

I have no other issues, for Johan's patch: Ack.

Thanks.
Lianbo
 
- [1] https://source.android.com/docs/core/architecture/kernel/gki-android13-5_10-release-builds

Regards
Johan