Re: [PATCH] mm/memtest: Add ARCH_USE_MEMTEST

2021-02-07 Thread Anshuman Khandual



On 2/5/21 2:50 PM, Vladimir Murzin wrote:
> Hi Anshuman,
> 
> On 2/5/21 4:10 AM, Anshuman Khandual wrote:
>> early_memtest() does not get called from all architectures. Hence enabling
>> CONFIG_MEMTEST and providing a valid memtest=[1..N] kernel command line
>> option might not trigger the memory pattern tests as would be expected in
>> normal circumstances. This situation is misleading.
> 
> Documentation already mentions which architectures support that:
> 
> memtest=[KNL,X86,ARM,PPC] Enable memtest
> 
> yet I admit that not all reflected there

But there is nothing that prevents CONFIG_MEMTEST from being set on
other platforms that do not have an affect, which is not optimal.

> 
>>
>> The change here prevents the above mentioned problem after introducing a
>> new config option ARCH_USE_MEMTEST that should be subscribed on platforms
>> that call early_memtest(), in order to enable the config CONFIG_MEMTEST.
>> Conversely CONFIG_MEMTEST cannot be enabled on platforms where it would
>> not be tested anyway.
>>
> 
> Is that generic pattern? What about other cross arch parameters? Do they 
> already
> use similar subscription or they rely on documentation?

Depending solely on the documentation should not be sufficient.

> 
> I'm not against the patch just want to check if things are consistent...
Not sure about other similar situations but those if present should
get fixed as well.


Re: [PATCH] mm/memtest: Add ARCH_USE_MEMTEST

2021-02-07 Thread Anshuman Khandual



On 2/5/21 1:05 PM, Max Filippov wrote:
> On Thu, Feb 4, 2021 at 8:10 PM Anshuman Khandual
>  wrote:
>>
>> early_memtest() does not get called from all architectures. Hence enabling
>> CONFIG_MEMTEST and providing a valid memtest=[1..N] kernel command line
>> option might not trigger the memory pattern tests as would be expected in
>> normal circumstances. This situation is misleading.
>>
>> The change here prevents the above mentioned problem after introducing a
>> new config option ARCH_USE_MEMTEST that should be subscribed on platforms
>> that call early_memtest(), in order to enable the config CONFIG_MEMTEST.
>> Conversely CONFIG_MEMTEST cannot be enabled on platforms where it would
>> not be tested anyway.
>>
>> Cc: Russell King 
>> Cc: Catalin Marinas 
>> Cc: Will Deacon 
>> Cc: Thomas Bogendoerfer 
>> Cc: Michael Ellerman 
>> Cc: Benjamin Herrenschmidt 
>> Cc: Paul Mackerras 
>> Cc: Thomas Gleixner 
>> Cc: Ingo Molnar 
>> Cc: Chris Zankel 
>> Cc: Max Filippov 
>> Cc: linux-arm-ker...@lists.infradead.org
>> Cc: linux-m...@vger.kernel.org
>> Cc: linuxppc-dev@lists.ozlabs.org
>> Cc: linux-xte...@linux-xtensa.org
>> Cc: linux...@kvack.org
>> Cc: linux-ker...@vger.kernel.org
>> Signed-off-by: Anshuman Khandual 
>> ---
>> This patch applies on v5.11-rc6 and has been tested on arm64 platform. But
>> it has been just build tested on all other platforms.
>>
>>  arch/arm/Kconfig | 1 +
>>  arch/arm64/Kconfig   | 1 +
>>  arch/mips/Kconfig| 1 +
>>  arch/powerpc/Kconfig | 1 +
>>  arch/x86/Kconfig | 1 +
>>  arch/xtensa/Kconfig  | 1 +
>>  lib/Kconfig.debug| 9 -
>>  7 files changed, 14 insertions(+), 1 deletion(-)
> 
> Anshuman, entries in arch/*/Konfig files are sorted in alphabetical order,
> please keep them that way.

Sure, will fix up and resend.

> 
> Reviewed-by: Max Filippov 
> 


Re: [PATCH] mm/memtest: Add ARCH_USE_MEMTEST

2021-02-05 Thread Vladimir Murzin
Hi Anshuman,

On 2/5/21 4:10 AM, Anshuman Khandual wrote:
> early_memtest() does not get called from all architectures. Hence enabling
> CONFIG_MEMTEST and providing a valid memtest=[1..N] kernel command line
> option might not trigger the memory pattern tests as would be expected in
> normal circumstances. This situation is misleading.

Documentation already mentions which architectures support that:

memtest=[KNL,X86,ARM,PPC] Enable memtest

yet I admit that not all reflected there

> 
> The change here prevents the above mentioned problem after introducing a
> new config option ARCH_USE_MEMTEST that should be subscribed on platforms
> that call early_memtest(), in order to enable the config CONFIG_MEMTEST.
> Conversely CONFIG_MEMTEST cannot be enabled on platforms where it would
> not be tested anyway.
> 

Is that generic pattern? What about other cross arch parameters? Do they already
use similar subscription or they rely on documentation?

I'm not against the patch just want to check if things are consistent...

Cheers
Vladimir


Re: [PATCH] mm/memtest: Add ARCH_USE_MEMTEST

2021-02-04 Thread Max Filippov
On Thu, Feb 4, 2021 at 8:10 PM Anshuman Khandual
 wrote:
>
> early_memtest() does not get called from all architectures. Hence enabling
> CONFIG_MEMTEST and providing a valid memtest=[1..N] kernel command line
> option might not trigger the memory pattern tests as would be expected in
> normal circumstances. This situation is misleading.
>
> The change here prevents the above mentioned problem after introducing a
> new config option ARCH_USE_MEMTEST that should be subscribed on platforms
> that call early_memtest(), in order to enable the config CONFIG_MEMTEST.
> Conversely CONFIG_MEMTEST cannot be enabled on platforms where it would
> not be tested anyway.
>
> Cc: Russell King 
> Cc: Catalin Marinas 
> Cc: Will Deacon 
> Cc: Thomas Bogendoerfer 
> Cc: Michael Ellerman 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Chris Zankel 
> Cc: Max Filippov 
> Cc: linux-arm-ker...@lists.infradead.org
> Cc: linux-m...@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: linux-xte...@linux-xtensa.org
> Cc: linux...@kvack.org
> Cc: linux-ker...@vger.kernel.org
> Signed-off-by: Anshuman Khandual 
> ---
> This patch applies on v5.11-rc6 and has been tested on arm64 platform. But
> it has been just build tested on all other platforms.
>
>  arch/arm/Kconfig | 1 +
>  arch/arm64/Kconfig   | 1 +
>  arch/mips/Kconfig| 1 +
>  arch/powerpc/Kconfig | 1 +
>  arch/x86/Kconfig | 1 +
>  arch/xtensa/Kconfig  | 1 +
>  lib/Kconfig.debug| 9 -
>  7 files changed, 14 insertions(+), 1 deletion(-)

Anshuman, entries in arch/*/Konfig files are sorted in alphabetical order,
please keep them that way.

Reviewed-by: Max Filippov 

-- 
Thanks.
-- Max