Re: [PATCH -next v5 2/4] selftests/memory-hotplug: Restore memory before exit

Re: [PATCH -next v5 2/4] selftests/memory-hotplug: Restore memory before exit

[zhaogongyi]

Hi Shuah, David,

I am sorry for my slow response, I have submit a new patch to fix it. Please see: https://patchwork.kernel.org/project/linux-kselftest/patch/20221011013926.200774-1-zhaogongyi@huawei.com/

Thanks,
Gongyi

> 
> On 10/10/22 00:54, David Hildenbrand wrote:
> > On 08.10.22 03:40, zhaogongyi wrote:
> 
> >>
> >> Yes, online_memory_expect_success() already prints a warning,
> remove
> >> the warning in online_all_offline_memory() seems ok,
> >>
> >> My previous consideration was that one more log information would
> make it easier to locate the wrong location.
> >
> > Let's keep it simple unless there is real reason to warn twice.
> >
> 
> zhaogongyi,
> 
> Please note that I already applied the patches to linux-kselftest next for my
> second pull request before the merge window. Please send the change
> David requested in a separate patch on top of next as a fix.
> 
> thanks,
> -- Shuah

Re: [PATCH -next v5 2/4] selftests/memory-hotplug: Restore memory before exit

[zhaogongyi]

Hi!

> 
> On 30.09.22 10:52, zhaogongyi wrote:
> > Hi!
> >
> >>
> >> On 30.09.22 08:35, Zhao Gongyi wrote:
> >>> Some momory will be left in offline state when calling
> >>> offline_memory_expect_fail() failed. Restore it before exit.
> >>>
> >>> Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
> >>> ---
> >>>    .../memory-hotplug/mem-on-off-test.sh         | 21
> >> ++++++++++++++-----
> >>>    1 file changed, 16 insertions(+), 5 deletions(-)
> >>>
> >>> diff --git a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
> >> b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
> >>> index 1d87611a7d52..91a7457616bb 100755
> >>> --- a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
> >>> +++ b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
> >>> @@ -134,6 +134,16 @@ offline_memory_expect_fail()
> >>>    	return 0
> >>>    }
> >>>
> >>> +online_all_offline_memory()
> >>> +{
> >>> +	for memory in `hotpluggable_offline_memory`; do
> >>> +		if ! online_memory_expect_success $memory; then
> >>> +			echo "$FUNCNAME $memory: unexpected fail" >&2
> >>
> >> Do we need that output?
> >
> > In my opinion, if online a memory node failed ,it should be a kernel bug
> catched, so, I think the output here is needed.
> 
> But online_memory_expect_success() already prints a warning, no?

Yes, online_memory_expect_success() already prints a warning, remove the warning in online_all_offline_memory() seems ok,

My previous consideration was that one more log information would make it easier to locate the wrong location.

Best Regards,
Gongyi

Re: [PATCH -next v5 2/4] selftests/memory-hotplug: Restore memory before exit

[David Hildenbrand]

On 08.10.22 03:40, zhaogongyi wrote:
> Hi!
> 
>>
>> On 30.09.22 10:52, zhaogongyi wrote:
>>> Hi!
>>>
>>>>
>>>> On 30.09.22 08:35, Zhao Gongyi wrote:
>>>>> Some momory will be left in offline state when calling
>>>>> offline_memory_expect_fail() failed. Restore it before exit.
>>>>>
>>>>> Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
>>>>> ---
>>>>>     .../memory-hotplug/mem-on-off-test.sh         | 21
>>>> ++++++++++++++-----
>>>>>     1 file changed, 16 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
>>>> b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
>>>>> index 1d87611a7d52..91a7457616bb 100755
>>>>> --- a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
>>>>> +++ b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
>>>>> @@ -134,6 +134,16 @@ offline_memory_expect_fail()
>>>>>     	return 0
>>>>>     }
>>>>>
>>>>> +online_all_offline_memory()
>>>>> +{
>>>>> +	for memory in `hotpluggable_offline_memory`; do
>>>>> +		if ! online_memory_expect_success $memory; then
>>>>> +			echo "$FUNCNAME $memory: unexpected fail" >&2
>>>>
>>>> Do we need that output?
>>>
>>> In my opinion, if online a memory node failed ,it should be a kernel bug
>> catched, so, I think the output here is needed.
>>
>> But online_memory_expect_success() already prints a warning, no?
> 
> Yes, online_memory_expect_success() already prints a warning, remove the warning in online_all_offline_memory() seems ok,
> 
> My previous consideration was that one more log information would make it easier to locate the wrong location.

Let's keep it simple unless there is real reason to warn twice.

-- 
Thanks,

David / dhildenb

Re: [PATCH -next v5 2/4] selftests/memory-hotplug: Restore memory before exit

[Shuah Khan]

On 10/10/22 00:54, David Hildenbrand wrote:
> On 08.10.22 03:40, zhaogongyi wrote:

>>
>> Yes, online_memory_expect_success() already prints a warning, remove the warning in online_all_offline_memory() seems ok,
>>
>> My previous consideration was that one more log information would make it easier to locate the wrong location.
> 
> Let's keep it simple unless there is real reason to warn twice.
> 

zhaogongyi,

Please note that I already applied the patches to linux-kselftest
next for my second pull request before the merge window. Please
send the change David requested in a separate patch on top of
next as a fix.

thanks,
-- Shuah

Re: [PATCH -next v5 2/4] selftests/memory-hotplug: Restore memory before exit

[zhaogongyi]

Hi!

> 
> On 30.09.22 08:35, Zhao Gongyi wrote:
> > Some momory will be left in offline state when calling
> > offline_memory_expect_fail() failed. Restore it before exit.
> >
> > Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
> > ---
> >   .../memory-hotplug/mem-on-off-test.sh         | 21
> ++++++++++++++-----
> >   1 file changed, 16 insertions(+), 5 deletions(-)
> >
> > diff --git a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
> b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
> > index 1d87611a7d52..91a7457616bb 100755
> > --- a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
> > +++ b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
> > @@ -134,6 +134,16 @@ offline_memory_expect_fail()
> >   	return 0
> >   }
> >
> > +online_all_offline_memory()
> > +{
> > +	for memory in `hotpluggable_offline_memory`; do
> > +		if ! online_memory_expect_success $memory; then
> > +			echo "$FUNCNAME $memory: unexpected fail" >&2
> 
> Do we need that output?

In my opinion, if online a memory node failed ,it should be a kernel bug catched, so, I think the output here is needed.

Thanks!

Gongyi

Re: [PATCH -next v5 2/4] selftests/memory-hotplug: Restore memory before exit

[David Hildenbrand]

On 30.09.22 10:52, zhaogongyi wrote:
> Hi!
> 
>>
>> On 30.09.22 08:35, Zhao Gongyi wrote:
>>> Some momory will be left in offline state when calling
>>> offline_memory_expect_fail() failed. Restore it before exit.
>>>
>>> Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
>>> ---
>>>    .../memory-hotplug/mem-on-off-test.sh         | 21
>> ++++++++++++++-----
>>>    1 file changed, 16 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
>> b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
>>> index 1d87611a7d52..91a7457616bb 100755
>>> --- a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
>>> +++ b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
>>> @@ -134,6 +134,16 @@ offline_memory_expect_fail()
>>>    	return 0
>>>    }
>>>
>>> +online_all_offline_memory()
>>> +{
>>> +	for memory in `hotpluggable_offline_memory`; do
>>> +		if ! online_memory_expect_success $memory; then
>>> +			echo "$FUNCNAME $memory: unexpected fail" >&2
>>
>> Do we need that output?
> 
> In my opinion, if online a memory node failed ,it should be a kernel bug catched, so, I think the output here is needed.

But online_memory_expect_success() already prints a warning, no?

-- 
Thanks,

David / dhildenb

[PATCH -next v5 0/4] Optimize and bugfix for memory-hotplug

[Zhao Gongyi]

1. Add checking after online or offline
2. Restore memory before exit
3. Adjust log info for maintainability
4. Correct test's name

Changes in v5:
  - Adjust log info for maintainability

Changes in v4:
  - Remove redundant log information

Changes in v3:
  - Remove 2 obselute patches

Zhao Gongyi (4):
  selftests/memory-hotplug: Add checking after online or offline
  selftests/memory-hotplug: Restore memory before exit
  selftests/memory-hotplug: Adjust log info for maintainability
  docs: notifier-error-inject: Correct test's name

 .../fault-injection/notifier-error-inject.rst |  4 +--
 .../memory-hotplug/mem-on-off-test.sh         | 34 +++++++++++++++----
 2 files changed, 29 insertions(+), 9 deletions(-)

--
2.17.1

[PATCH -next v5 2/4] selftests/memory-hotplug: Restore memory before exit

[Zhao Gongyi]

Some momory will be left in offline state when calling
offline_memory_expect_fail() failed. Restore it before exit.

Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
---
 .../memory-hotplug/mem-on-off-test.sh         | 21 ++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
index 1d87611a7d52..91a7457616bb 100755
--- a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
+++ b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
@@ -134,6 +134,16 @@ offline_memory_expect_fail()
 	return 0
 }

+online_all_offline_memory()
+{
+	for memory in `hotpluggable_offline_memory`; do
+		if ! online_memory_expect_success $memory; then
+			echo "$FUNCNAME $memory: unexpected fail" >&2
+			retval=1
+		fi
+	done
+}
+
 error=-12
 priority=0
 # Run with default of ratio=2 for Kselftest run
@@ -275,11 +285,7 @@ done
 # Online all hot-pluggable memory
 #
 echo 0 > $NOTIFIER_ERR_INJECT_DIR/actions/MEM_GOING_ONLINE/error
-for memory in `hotpluggable_offline_memory`; do
-	if ! online_memory_expect_success $memory; then
-		retval=1
-	fi
-done
+online_all_offline_memory

 #
 # Test memory hot-remove error handling (online => offline)
@@ -296,4 +302,9 @@ done
 echo 0 > $NOTIFIER_ERR_INJECT_DIR/actions/MEM_GOING_OFFLINE/error
 /sbin/modprobe -q -r memory-notifier-error-inject

+#
+# Restore memory before exit
+#
+online_all_offline_memory
+
 exit $retval
--
2.17.1

Re: [PATCH -next v5 2/4] selftests/memory-hotplug: Restore memory before exit

[David Hildenbrand]

On 30.09.22 08:35, Zhao Gongyi wrote:
> Some momory will be left in offline state when calling
> offline_memory_expect_fail() failed. Restore it before exit.
> 
> Signed-off-by: Zhao Gongyi <zhaogongyi@huawei.com>
> ---
>   .../memory-hotplug/mem-on-off-test.sh         | 21 ++++++++++++++-----
>   1 file changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
> index 1d87611a7d52..91a7457616bb 100755
> --- a/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
> +++ b/tools/testing/selftests/memory-hotplug/mem-on-off-test.sh
> @@ -134,6 +134,16 @@ offline_memory_expect_fail()
>   	return 0
>   }
> 
> +online_all_offline_memory()
> +{
> +	for memory in `hotpluggable_offline_memory`; do
> +		if ! online_memory_expect_success $memory; then
> +			echo "$FUNCNAME $memory: unexpected fail" >&2

Do we need that output?


-- 
Thanks,

David / dhildenb