linux-kernel - Re: [PATCH net-next v12 01/14] mm: page_frag: add a test module for page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c9cc66e0-195a-4db4-98b8-cdbb986e0619@huawei.com>
Date: Fri, 2 Aug 2024 18:02:35 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Alexander Duyck <alexander.duyck@...il.com>
CC: <davem@...emloft.net>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<netdev@...r.kernel.org>, <linux-kernel@...r.kernel.org>, Andrew Morton
	<akpm@...ux-foundation.org>, <linux-mm@...ck.org>
Subject: Re: [PATCH net-next v12 01/14] mm: page_frag: add a test module for
 page_frag

On 2024/8/1 22:50, Alexander Duyck wrote:

>>
>> The above was my initial thinking too, I went to the ptrpool thing using
>> at least two CPUs as the below reason:
>> 1. Test the concurrent calling between allocing and freeing more throughly,
>>    for example, page->_refcount concurrent handling, cache draining and
>>    cache reusing code path will be tested more throughly.
>> 2. Test the performance impact of cache bouncing between different CPUs.
>>
>> I am not sure if there is a more lightweight implementation than ptrpool
>> to do the above testing more throughly.
> 
> You can still do that with a single producer single consumer ring
> buffer/array and not have to introduce a ton of extra overhead for
> some push/pop approach. There are a number of different
> implementations for such things throughout the kernel.

if we limit that to single producer single consumer, it seems we can
use ptr_ring to replace ptrpool.

> 
>>
>>>
>>> Lastly something that is a module only tester that always fails to
>>> probe doesn't sound like it really makes sense as a standard kernel
>>
>> I had the same feeling as you, but when doing testing, it seems
>> convenient enough to do a 'insmod xxx.ko' for testing without a
>> 'rmmod xxx.ko'
> 
> It means this isn't a viable module though. If it supports insmod to
> trigger your tests you should let it succeed, and then do a rmmod to
> remove it afterwards. Otherwise it is a test module and belongs in the
> selftest block.
> 
>>> module. I still think it would make more sense to move it to the
>>> selftests tree and just have it build there as a module instead of
>>
>> I failed to find one example of test kernel module that is in the
>> selftests tree yet. If it does make sense, please provide an example
>> here, and I am willing to follow the pattern if there is one.
> 
> You must not have been looking very hard. A quick grep for
> "module_init" in the selftest folder comes up with
> "tools/testing/selftests/bpf/bpf_testmod/" containing an example of a
> module built in the selftests folder.

After close look, it seems it will be treated as third party module when
adding a kernel module in tools/testing/selftests as there seems to be no
config for it in Kconfig file and can only be compiled as a module not as
built-in.

> 
>>> trying to force it into the mm tree. The example of dmapool_test makes
>>> sense as it could be run at early boot to run the test and then it
>>
>> I suppose you meant dmapool is built-in to the kernel and run at early
>> boot? I am not sure what is the point of built-in for dmapool, as it
>> only do one-time testing, and built-in for dmapool only waste some
>> memory when testing is done.
> 
> There are cases where one might want to test on a system w/o console
> access such as an embedded system, or in the case of an environment
> where people run without an initrd at all.

I think moving it to tools/testing/selftests may defeat the above purpose.

> 
>>> just goes quiet. This module won't load and will always just return
>>> -EAGAIN which doesn't sound like a valid kernel module to me.
>>
>> As above, it seems convenient enough to do a 'insmod xxx.ko' for testing
>> without a 'rmmod xxx.ko'.
> 
> It is, but it isn't. The problem is it creates a bunch of ugliness in

Yes, it seems a bit ugly, but it supports the below perf cmd, I really
would like to support the below case as it is very convenient.

perf stat -r 200 -- insmod ./page_frag_test.ko test_push_cpu=16 test_pop_cpu=17

> the build as you are a tristate that isn't a tristate as you are only
> building it if it is set to "m". There isn't anything like that
> currently in the mm tree.

After moving page_frag_test to selftest, it is only bulit as module, I guess
it is ok to return -EAGAIN?