Hi Reinette!
On 2024-01-23 at 09:42:07 -0800, Reinette Chatre wrote:
Hi Maciej,
On 1/22/2024 11:58 PM, Maciej Wieczór-Retman wrote:
On 2024-01-22 at 08:32:36 -0800, Reinette Chatre wrote:
Hi Maciej,
On 1/21/2024 11:56 PM, Maciej Wieczór-Retman wrote:
Hi!
On 2024-01-19 at 08:39:31 -0800, Reinette Chatre wrote:
Hi Maciej,
On 1/18/2024 11:37 PM, Maciej Wieczór-Retman wrote:
On 2024-01-18 at 09:15:46 -0800, Reinette Chatre wrote: > On 1/18/2024 4:02 AM, Maciej Wieczór-Retman wrote: >> On 2024-01-17 at 10:49:06 -0800, Reinette Chatre wrote: >>> On 1/17/2024 12:26 AM, Maciej Wieczór-Retman wrote: >>>> On 2024-01-08 at 14:42:11 -0800, Reinette Chatre wrote: >>>>> On 12/12/2023 6:52 AM, Maciej Wieczor-Retman wrote: > >>>>>> + bit_center = count_bits(full_cache_mask) / 2; >>>>>> + cont_mask = full_cache_mask >> bit_center; >>>>>> + >>>>>> + /* Contiguous mask write check. */ >>>>>> + snprintf(schemata, sizeof(schemata), "%lx", cont_mask); >>>>>> + ret = write_schemata("", schemata, uparams->cpu, test->resource); >>>>>> + if (ret) >>>>>> + return ret; >>>>> >>>>> How will user know what failed? I am seeing this single test exercise a few scenarios >>>>> and it is not obvious to me if the issue will be clear if this test, >>>>> noncont_cat_run_test(), fails. >>>> >>>> write_schemata() either succeeds with '0' or errors out with a negative value. If >>>> the contiguous mask write fails, write_schemata should print out what was wrong >>>> and I believe that the test will report an error rather than failure. >>> >>> Right. I am trying to understand whether the user will be able to decipher what failed >>> in case there is an error. Seems like in this case the user is expected to look at the >>> source code of the test to understand what the test was trying to do at the time it >>> encountered the failure. In this case user may be "lucky" that this test only has >>> one write_schemata() call _not_ followed by a ksft_print_msg() so user can use that >>> reasoning to figure out which write_schemata() failed to further dig what test was >>> trying to do. >> >> When a write_schemata() is executed the string that is being written gets >> printed. If there are multiple calls in a single tests and one fails I'd imagine >> it would be easy for the user to figure out which one failed. > > It would be easy for the user the figure out if (a) it is obvious to the user > what schema a particular write_schema() call attempted to write and (b) all the > write_schema() calls attempt to write different schema.
As for (b) depends on what you meant. Other tests that run more than one write_schemata() use different ones every time (CAT, MBM, MBA). Do you suggest that the non-contiguous test should attempt more schematas? For example shift the bit hole from one side to the other? I assumed one CBM with a centered bit hole would be enough to check if non-contiguous CBM feature works properly and more CBMs would be redundant.
Let me try with an example. Scenario 1: The test has the following code: ... write_schemata(..., "0xfff", ...); ... write_schemata(..., "0xf0f", ...); ...
Scenario 2: The test has the following code: ... write_schemata(..., "0xfff", ...); ... write_schemata(..., "0xfff", ...); ...
A failure of either write_schemata() in scenario 1 will be easy to trace since the schemata attempted is different in each case. The schemata printed by the write_schemata() error message can thus easily be connected to the specific write_schemata() call. A failure of either write_schemata() in scenario 2 is not so obvious since they both attempted the same schemata so the error message printed by write_schemata() could belong to either.
I'm sorry to drag this thread out but I want to be sure if I'm right or are you suggesting something and I missed it?
Please just add a ksft_print_msg() to noncont_cat_run_test() when this write_schemata() fails.
My point all along was that if write_schemata() fails it already prints out all the necessary information. I'd like to avoid adding redundant messages so please take a look at how it looks now:
Please consider that there may be different perspectives of "necessary information".
Oh of course. By that I meant the failed schemata which I assumed is what you were looking for in this error handling here.
I injected write_schemata() with an error so it will take a path as if write() failed with 'Permission denied' as a reason. Here is the output for L3 non-contiguous CAT test:
[root@spr1 ~]# ./resctrl_tests -t L3_NONCONT_CAT TAP version 13 # Pass: Check kernel supports resctrl filesystem # Pass: Check resctrl mountpoint "/sys/fs/resctrl" exists # resctrl filesystem not mounted # dmesg: [ 18.579861] resctrl: L3 allocation detected # dmesg: [ 18.590395] resctrl: L2 allocation detected # dmesg: [ 18.595181] resctrl: MB allocation detected # dmesg: [ 18.599963] resctrl: L3 monitoring detected 1..1 # Starting L3_NONCONT_CAT test ... # Mounting resctrl to "/sys/fs/resctrl" # Write schema "L3:0=ff" to resctrl FS # write() failed : Permission denied not ok 1 L3_NONCONT_CAT: test # Totals: pass:0 fail:1 xfail:0 xpass:0 skip:0 error:0
Understood.
Of course if you still think adding a ksft_print_msg() there would be meaningful I'll try to write a sensible message. But I hope you can see what I meant when I wrote that the user could already easily see what failed.
I do still believe that it will be helpful if there is a ksft_print_msg() with something like "Unable to write contiguous CBM" or "Write of contiguous CBM failed" or ... ?
Sure, I can see how that can be helpful, I'll add "Write of contiguous CBM failed", thanks!
Reinette