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".

> 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 ... ? 

Reinette


Reply via email to