Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1741768 > unrolled thread

[PATCH 0/4] x86/platform/UV: Update TSC support

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/4] x86/platform/UV: Update TSC support <mike.travis@hpe.com> - 2017-09-28 20:20 +0200
    Re: [PATCH 0/4] x86/platform/UV: Update TSC support Peter Zijlstra <peterz@infradead.org> - 2017-09-29 10:50 +0200
      Re: [PATCH 0/4] x86/platform/UV: Update TSC support Mike Travis <mike.travis@hpe.com> - 2017-09-29 17:30 +0200
        Re: [PATCH 0/4] x86/platform/UV: Update TSC support Peter Zijlstra <peterz@infradead.org> - 2017-09-29 18:30 +0200
          Re: [PATCH 0/4] x86/platform/UV: Update TSC support Mike Travis <mike.travis@hpe.com> - 2017-09-29 19:40 +0200
            Re: [PATCH 0/4] x86/platform/UV: Update TSC support Peter Zijlstra <peterz@infradead.org> - 2017-09-29 20:50 +0200

#1741768 — [PATCH 0/4] x86/platform/UV: Update TSC support

The UV BIOS goes to considerable effort to get the TSC synchronization
accurate across the entire system.  Included in that are multiple chassis
that can have 32+ sockets.  The architecture does support an external
high resolution clock to aid in maintaining this synchronization.

The resulting TSC accuracy set by the UV system BIOS is much better
than the generic kernel TSC ADJUST functions.  This is important for
applications that read the TSC values directly for accessing data bases.

*   These patches disable an assumption made by the kernel tsc sync
    functions that Socket 0 in the system should have a TSC ADJUST
    value of zero.  This is not correct when the chassis are reset
    asynchronously to each other so which TSC's should be zero is
    not predictable.

*   When the system BIOS determines that the TSC is not stable, it then
    sets a flag so the UV kernel setup can set the "tsc is unstable"
    flag.  A patch now prevents the kernel from attempting to fix the
    TSC causing a slew of warning messages.

*   It also eliminates another avalanche of warning messages from older
    BIOS that did not have the TSC ADJUST MSR (ex. >3000 msgs in a 32
    socket Skylake system).  It now notes this with a single warning
    message and then moves on with fixing them.

-- 

[toc] | [next] | [standalone]

#1741993

On Thu, Sep 28, 2017 at 01:03:39PM -0500, mike.travis@hpe.com wrote:
> 
> The UV BIOS goes to considerable effort to get the TSC synchronization
> accurate across the entire system.  Included in that are multiple chassis
> that can have 32+ sockets.  The architecture does support an external
> high resolution clock to aid in maintaining this synchronization.
> 
> The resulting TSC accuracy set by the UV system BIOS is much better
> than the generic kernel TSC ADJUST functions.  This is important for
> applications that read the TSC values directly for accessing data bases.
> 
> *   These patches disable an assumption made by the kernel tsc sync
>     functions that Socket 0 in the system should have a TSC ADJUST
>     value of zero.  This is not correct when the chassis are reset
>     asynchronously to each other so which TSC's should be zero is
>     not predictable.
> 
> *   When the system BIOS determines that the TSC is not stable, it then
>     sets a flag so the UV kernel setup can set the "tsc is unstable"
>     flag.  A patch now prevents the kernel from attempting to fix the
>     TSC causing a slew of warning messages.
> 
> *   It also eliminates another avalanche of warning messages from older
>     BIOS that did not have the TSC ADJUST MSR (ex. >3000 msgs in a 32
>     socket Skylake system).  It now notes this with a single warning
>     message and then moves on with fixing them.

So I would still like to get clarification on how ART works (or likely
doesn't) on your systems. I think for now its fairly prudent to kill
detect_art() on UV.

Also, while indeed not strictly required, that TSC_ADJUST==0 test on
bootcpu is nice for consumer systems, BIOS did something 'weird' if that
is not true. Is something like is_uv_system() available early enough?

Other than that, the patches look good to me.

[toc] | [prev] | [next] | [standalone]

#1742204

On 9/29/2017 1:46 AM, Peter Zijlstra wrote:
> On Thu, Sep 28, 2017 at 01:03:39PM -0500, mike.travis@hpe.com wrote:
>>
>> The UV BIOS goes to considerable effort to get the TSC synchronization
>> accurate across the entire system.  Included in that are multiple chassis
>> that can have 32+ sockets.  The architecture does support an external
>> high resolution clock to aid in maintaining this synchronization.
>>
>> The resulting TSC accuracy set by the UV system BIOS is much better
>> than the generic kernel TSC ADJUST functions.  This is important for
>> applications that read the TSC values directly for accessing data bases.
>>
>> *   These patches disable an assumption made by the kernel tsc sync
>>      functions that Socket 0 in the system should have a TSC ADJUST
>>      value of zero.  This is not correct when the chassis are reset
>>      asynchronously to each other so which TSC's should be zero is
>>      not predictable.
>>
>> *   When the system BIOS determines that the TSC is not stable, it then
>>      sets a flag so the UV kernel setup can set the "tsc is unstable"
>>      flag.  A patch now prevents the kernel from attempting to fix the
>>      TSC causing a slew of warning messages.
>>
>> *   It also eliminates another avalanche of warning messages from older
>>      BIOS that did not have the TSC ADJUST MSR (ex. >3000 msgs in a 32
>>      socket Skylake system).  It now notes this with a single warning
>>      message and then moves on with fixing them.
> 
> So I would still like to get clarification on how ART works (or likely
> doesn't) on your systems. I think for now its fairly prudent to kill
> detect_art() on UV.

I tested with both detect_art enabled and disabled and didn't notice a 
difference though I wasn't sure what test to run to verify whether it 
was being used or not.  (I'd be glad to run some specific test if one 
can be suggested?)  The num/denom setting for a 2100Mhz CPU was 168/2 if 
that information helps?

> Also, while indeed not strictly required, that TSC_ADJUST==0 test on
> bootcpu is nice for consumer systems, BIOS did something 'weird' if that
> is not true. Is something like is_uv_system() available early enough?

My previous version of the patches had me setting a flag that could be 
checked by the tsc_sanitize_first_cpu() function and disable the 
requirement of "TSC == 0 on socket 0" for any arch that specified it.
(And UV did set that flag.)

But Thomas said it was "hackery" and that TSC being 0 on socket 0 was no 
longer a requirement.  So I took it out for this version and made the 
"TSC == 0 on socket 0" no longer the default for any arch.
> 
> Other than that, the patches look good to me.
> 

[toc] | [prev] | [next] | [standalone]

#1742234

On Fri, Sep 29, 2017 at 08:19:22AM -0700, Mike Travis wrote:
> > So I would still like to get clarification on how ART works (or likely
> > doesn't) on your systems. I think for now its fairly prudent to kill
> > detect_art() on UV.
> 
> I tested with both detect_art enabled and disabled and didn't notice a
> difference though I wasn't sure what test to run to verify whether it was
> being used or not.  (I'd be glad to run some specific test if one can be
> suggested?)  The num/denom setting for a 2100Mhz CPU was 168/2 if that
> information helps?

While ART has a ratio to TSC, it too has an absolute relation to it.
Given an ART time stamp we can compute a TSC value and vice versa, this
allows correlating device timestamps (Network, Audio/Video etc..) with
CPU time stamps.

Per detect_art() we have a single system wide offset, namely:

  rdmsrl(MSR_IA32_TSC_ADJUST, art_to_tsc_offset);

But you use TSC_ADJUST to sync between your cabinets, this cannot ever
be right. The ART clock of the other cabinets (those that did not run
detect_art) will have a different offset.

Currently there are only two device drivers that use ART:

  drivers/net/ethernet/intel/e1000e/ptp.c:        *system = convert_art_to_tsc(sys_cycles);
  sound/pci/hda/hda_controller.c: *system = convert_art_to_tsc(tsc_counter);

Outside of that nobody cares, _for_now_.

I'm not sure if there's a means for the CPU to read ART in order to test
this correlation.

Intel SDM Vol 3B 17.17.4 speaks of 'K' with a footnote about TSC_ADJUST
and the VMCS TSC fields. But basically both TSC and ART start at 0 on
power on and given the frequency ratio 'K' is a known for native system
agents.

Again, I would suggest killing detect_art() (and the setting of
X86_FEATURE_ART) on UV systems until things are worked out. Also, given
you have your own distributed clock, I'm thinking you use that on your
own devices, obviating the immediate need for ART.

> > Also, while indeed not strictly required, that TSC_ADJUST==0 test on
> > bootcpu is nice for consumer systems, BIOS did something 'weird' if that
> > is not true. Is something like is_uv_system() available early enough?
> 
> My previous version of the patches had me setting a flag that could be
> checked by the tsc_sanitize_first_cpu() function and disable the requirement
> of "TSC == 0 on socket 0" for any arch that specified it.
> (And UV did set that flag.)
> 
> But Thomas said it was "hackery" and that TSC being 0 on socket 0 was no
> longer a requirement.  So I took it out for this version and made the "TSC
> == 0 on socket 0" no longer the default for any arch.

That's where it comes from. But normal systems really _should_ have it
at 0 and its a useful sanity check IMO. We really want to know when the
BIOS does a funny behind our backs.

[toc] | [prev] | [next] | [standalone]

#1742255

On 9/29/2017 9:23 AM, Peter Zijlstra wrote:
> On Fri, Sep 29, 2017 at 08:19:22AM -0700, Mike Travis wrote:
>>> So I would still like to get clarification on how ART works (or likely
>>> doesn't) on your systems. I think for now its fairly prudent to kill
>>> detect_art() on UV.
>>
>> I tested with both detect_art enabled and disabled and didn't notice a
>> difference though I wasn't sure what test to run to verify whether it was
>> being used or not.  (I'd be glad to run some specific test if one can be
>> suggested?)  The num/denom setting for a 2100Mhz CPU was 168/2 if that
>> information helps?
> 
> While ART has a ratio to TSC, it too has an absolute relation to it.
> Given an ART time stamp we can compute a TSC value and vice versa, this
> allows correlating device timestamps (Network, Audio/Video etc..) with
> CPU time stamps.
> 
> Per detect_art() we have a single system wide offset, namely:
> 
>    rdmsrl(MSR_IA32_TSC_ADJUST, art_to_tsc_offset);
> 
> But you use TSC_ADJUST to sync between your cabinets, this cannot ever
> be right. The ART clock of the other cabinets (those that did not run
> detect_art) will have a different offset.
> 
> Currently there are only two device drivers that use ART:
> 
>    drivers/net/ethernet/intel/e1000e/ptp.c:        *system = convert_art_to_tsc(sys_cycles);
>    sound/pci/hda/hda_controller.c: *system = convert_art_to_tsc(tsc_counter);
> 
> Outside of that nobody cares, _for_now_.

I'm checking with the hardware/firmware designers but your mention of 
e1000e reminded me that I did see this but didn't quite connect the 
meaning.  If it's really a system wide constant, then yes we cannot 
provide a single value that would apply to all CPU's.

> 
> I'm not sure if there's a means for the CPU to read ART in order to test
> this correlation.
> 
> Intel SDM Vol 3B 17.17.4 speaks of 'K' with a footnote about TSC_ADJUST
> and the VMCS TSC fields. But basically both TSC and ART start at 0 on
> power on and given the frequency ratio 'K' is a known for native system
> agents.
> 
> Again, I would suggest killing detect_art() (and the setting of
> X86_FEATURE_ART) on UV systems until things are worked out. Also, given
> you have your own distributed clock, I'm thinking you use that on your
> own devices, obviating the immediate need for ART.
> 
>>> Also, while indeed not strictly required, that TSC_ADJUST==0 test on
>>> bootcpu is nice for consumer systems, BIOS did something 'weird' if that
>>> is not true. Is something like is_uv_system() available early enough?
>>
>> My previous version of the patches had me setting a flag that could be
>> checked by the tsc_sanitize_first_cpu() function and disable the requirement
>> of "TSC == 0 on socket 0" for any arch that specified it.
>> (And UV did set that flag.)
>>
>> But Thomas said it was "hackery" and that TSC being 0 on socket 0 was no
>> longer a requirement.  So I took it out for this version and made the "TSC
>> == 0 on socket 0" no longer the default for any arch.
> 
> That's where it comes from. But normal systems really _should_ have it
> at 0 and its a useful sanity check IMO. We really want to know when the
> BIOS does a funny behind our backs.
> 

How about a more generic flag, such as "multi_tsc_sync_sources"?  That 
could trigger both disabling the "TSC == 0 on socket 0" check as well as 
disabling X86_FEATURE_ART where appropriate?  Or I could clear the 
feature ART cap separately in the UV system init code if they are not 
really related?

[toc] | [prev] | [next] | [standalone]

#1742281

On Fri, Sep 29, 2017 at 10:39:28AM -0700, Mike Travis wrote:
> >That's where it comes from. But normal systems really _should_ have it
> >at 0 and its a useful sanity check IMO. We really want to know when the
> >BIOS does a funny behind our backs.
> >
> 
> How about a more generic flag, such as "multi_tsc_sync_sources"?  That could
> trigger both disabling the "TSC == 0 on socket 0" check as well as disabling
> X86_FEATURE_ART where appropriate?  Or I could clear the feature ART cap
> separately in the UV system init code if they are not really related?

I _think_ the X86_FEATURE_ART is an artificial flag. We key off of
cpuid_level here.

So that multi_tsc_sync_sources or a more explicit is_uv_system() would
be required.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web