[v6] x86/resctrl: Track llc_occupancy of RMIDs in limbo list

[PATCH v6 0/2] x86/resctrl: Track llc_occupancy of RMIDs in limbo list

Posted by Haifeng Xu 1 year, 10 months ago

After removing a monitor group, its RMID may not be freed immediately
unless its llc_occupancy is less than the re-allocation threshold. If
turning up the threshold, the RMID can be reused. In order to know how
much the threshold should be, it's necessary to acquire the llc_occupancy.

The patch series provides a new tracepoint to track the llc_occupancy.

Changes since v1:
- Rename pseudo_lock_event.h instead of creating a new header file.
- Modify names used in the tracepoint.
- Update changelog.

Changes since v2:
- Fix typo and use the x86/resctrl subject prefix in the cover letter.
- Track both CLOSID and RMID in the tracepoint.
- Remove the details on how perf can be used in patch2's changelog.

Changes since v3:
- Put the tracepoint in the 'else' section of the if/else after
  resctrl_arch_rmid_read().
- Modify names used in the tracepoint.
- Document the properties of the tracepoint.

Changes since v4:
- Add Reviewed-by tags.
- Include more maintainers in the submission.

Changes since v5:
- Update the documentation and comments of the tracepoint.
- Code cleanup.

Haifeng Xu (2):
  x86/resctrl: Rename pseudo_lock_event.h to trace.h
  x86/resctrl: Add tracepoint for llc_occupancy tracking

 Documentation/arch/x86/resctrl.rst            |  6 +++++
 arch/x86/kernel/cpu/resctrl/monitor.c         | 11 +++++++++
 arch/x86/kernel/cpu/resctrl/pseudo_lock.c     |  2 +-
 .../resctrl/{pseudo_lock_event.h => trace.h}  | 24 +++++++++++++++----
 4 files changed, 38 insertions(+), 5 deletions(-)
 rename arch/x86/kernel/cpu/resctrl/{pseudo_lock_event.h => trace.h} (56%)

-- 
2.25.1

Re: [PATCH v6 0/2] x86/resctrl: Track llc_occupancy of RMIDs in limbo list

Posted by Reinette Chatre 1 year, 10 months ago

Hi Haifeng,

On 3/19/2024 1:30 AM, Haifeng Xu wrote:
> After removing a monitor group, its RMID may not be freed immediately
> unless its llc_occupancy is less than the re-allocation threshold. If
> turning up the threshold, the RMID can be reused. In order to know how
> much the threshold should be, it's necessary to acquire the llc_occupancy.
> 
> The patch series provides a new tracepoint to track the llc_occupancy.

There seems to be a problem with the DKIM attestation. Here is what I see
when I download this series:

    $ b4 am -Q 20240319083039.223088-1-haifeng.xu@shopee.com
    Grabbing thread from lore.kernel.org/all/20240319083039.223088-1-haifeng.xu@shopee.com/t.mbox.gz
    Analyzing 3 messages in the thread
    Looking for additional code-review trailers on lore.kernel.org
    Checking attestation on all messages, may take a moment...
    ---
      ✗ [PATCH v6 1/2] x86/resctrl: Rename pseudo_lock_event.h to trace.h
      ✗ [PATCH v6 2/2] x86/resctrl: Add tracepoint for llc_occupancy tracking
    ---
      ✗ BADSIG: DKIM/shopee.com
    ---
    Total patches: 2
    ---

The patches look good to me. Thank you very much for adding this.
Please resubmit with the DKIM attestation fixed and then you can add:

| Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette

Re: [PATCH v6 0/2] x86/resctrl: Track llc_occupancy of RMIDs in limbo list

Posted by Haifeng Xu 1 year, 10 months ago


On 2024/3/30 07:06, Reinette Chatre wrote:
> Hi Haifeng,
> 
> On 3/19/2024 1:30 AM, Haifeng Xu wrote:
>> After removing a monitor group, its RMID may not be freed immediately
>> unless its llc_occupancy is less than the re-allocation threshold. If
>> turning up the threshold, the RMID can be reused. In order to know how
>> much the threshold should be, it's necessary to acquire the llc_occupancy.
>>
>> The patch series provides a new tracepoint to track the llc_occupancy.
> 
> There seems to be a problem with the DKIM attestation. Here is what I see
> when I download this series:
> 
>     $ b4 am -Q 20240319083039.223088-1-haifeng.xu@shopee.com
>     Grabbing thread from lore.kernel.org/all/20240319083039.223088-1-haifeng.xu@shopee.com/t.mbox.gz
>     Analyzing 3 messages in the thread
>     Looking for additional code-review trailers on lore.kernel.org
>     Checking attestation on all messages, may take a moment...
>     ---
>       ✗ [PATCH v6 1/2] x86/resctrl: Rename pseudo_lock_event.h to trace.h
>       ✗ [PATCH v6 2/2] x86/resctrl: Add tracepoint for llc_occupancy tracking
>     ---
>       ✗ BADSIG: DKIM/shopee.com
>     ---
>     Total patches: 2
>     ---
> 

Hi, Reinette,

I can also reproduced it. After digging into it, I found that our DKIM signature header
has 't' and 'x' flags. They are recommended as a means to help identify spam.

t= is the DKIM signature timestamp.
x= is the DKIM signature expiration time.

The source code of DKIM Validation can be seen in dkim/__init__.py(line 351), I paste it
here.

 343     if b'x' in sig:
 344         if re.match(br"\d+$", sig[b'x']) is None:
 345             raise ValidationError(
 346               "x= value is not a decimal integer (%s)" % sig[b'x'])
 347         x_sign = int(sig[b'x'])
 348         now = int(time.time())
 349         slop = 36000 # 10H leeway for mailers with inaccurate clocks
 350         if x_sign < now - slop:
 351             raise ValidationError(
 352                 "x= value is past (%s)" % sig[b'x'])
 353             if x_sign < t_sign:
 354                 raise ValidationError(
 355                     "x= value is less than t= value (x=%s t=%s)" %
 356                     (sig[b'x'], sig[b't']))

The expiry time is less than the time point you download the patch, so the validation
fails. If I comment out these lines, this series can be successfully downloaded.

The signature is only valid for a week. So if you use b4 to download the patch series,
It's best to do it in a week after the patch is sent.

Thanks.


> The patches look good to me. Thank you very much for adding this.
> Please resubmit with the DKIM attestation fixed and then you can add:
> 
> | Reviewed-by: Reinette Chatre <reinette.chatre@intel.com>
> 
> Reinette

Thanks for your review.

Re: [PATCH v6 0/2] x86/resctrl: Track llc_occupancy of RMIDs in limbo list

Posted by Konstantin Ryabitsev 1 year, 10 months ago

On Mon, Apr 08, 2024 at 03:44:00PM +0800, Haifeng Xu wrote:
> I can also reproduced it. After digging into it, I found that our DKIM signature header
> has 't' and 'x' flags. They are recommended as a means to help identify spam.
> 
> t= is the DKIM signature timestamp.
> x= is the DKIM signature expiration time.
> 
> The source code of DKIM Validation can be seen in dkim/__init__.py(line 351), I paste it
> here.
> 
>  343     if b'x' in sig:
>  344         if re.match(br"\d+$", sig[b'x']) is None:
>  345             raise ValidationError(
>  346               "x= value is not a decimal integer (%s)" % sig[b'x'])
>  347         x_sign = int(sig[b'x'])
>  348         now = int(time.time())
>  349         slop = 36000 # 10H leeway for mailers with inaccurate clocks
>  350         if x_sign < now - slop:
>  351             raise ValidationError(
>  352                 "x= value is past (%s)" % sig[b'x'])
>  353             if x_sign < t_sign:
>  354                 raise ValidationError(
>  355                     "x= value is less than t= value (x=%s t=%s)" %
>  356                     (sig[b'x'], sig[b't']))
> 
> The expiry time is less than the time point you download the patch, so the validation
> fails. If I comment out these lines, this series can be successfully downloaded.

FWIW, I've requested ability to ignore the x= flag when validating signatures:
https://bugs.launchpad.net/dkimpy/+bug/2047054

-K

Re: [PATCH v6 0/2] x86/resctrl: Track llc_occupancy of RMIDs in limbo list

Posted by Reinette Chatre 1 year, 10 months ago


On 4/8/2024 8:29 AM, Konstantin Ryabitsev wrote:
> On Mon, Apr 08, 2024 at 03:44:00PM +0800, Haifeng Xu wrote:
>> I can also reproduced it. After digging into it, I found that our DKIM signature header
>> has 't' and 'x' flags. They are recommended as a means to help identify spam.
>>
>> t= is the DKIM signature timestamp.
>> x= is the DKIM signature expiration time.
>>
>> The source code of DKIM Validation can be seen in dkim/__init__.py(line 351), I paste it
>> here.
>>
>>  343     if b'x' in sig:
>>  344         if re.match(br"\d+$", sig[b'x']) is None:
>>  345             raise ValidationError(
>>  346               "x= value is not a decimal integer (%s)" % sig[b'x'])
>>  347         x_sign = int(sig[b'x'])
>>  348         now = int(time.time())
>>  349         slop = 36000 # 10H leeway for mailers with inaccurate clocks
>>  350         if x_sign < now - slop:
>>  351             raise ValidationError(
>>  352                 "x= value is past (%s)" % sig[b'x'])
>>  353             if x_sign < t_sign:
>>  354                 raise ValidationError(
>>  355                     "x= value is less than t= value (x=%s t=%s)" %
>>  356                     (sig[b'x'], sig[b't']))
>>
>> The expiry time is less than the time point you download the patch, so the validation
>> fails. If I comment out these lines, this series can be successfully downloaded.
> 
> FWIW, I've requested ability to ignore the x= flag when validating signatures:
> https://bugs.launchpad.net/dkimpy/+bug/2047054

Thank you very much Haifeng and Konstantin.

Reinette