Re: git: e81e77c5a055 - main - Enable PPS_SYNC on amd64, arm64 and armv7

On 10/11/21 3:05 PM, Konstantin Belousov wrote:
> On Mon, Oct 11, 2021 at 02:09:58PM -0700, John Baldwin wrote:
>> On 10/11/21 1:46 PM, Konstantin Belousov wrote:
>>> On Mon, Oct 11, 2021 at 01:37:21PM -0700, John Baldwin wrote:
>>>> On 10/10/21 12:34 PM, Konstantin Belousov wrote:
>>>>> The branch main has been updated by kib:
>>>>>
>>>>> URL: https://cgit.FreeBSD.org/src/commit/?id=e81e77c5a055d1cbf6d6a6f0acbaf443267aa84f
>>>>>
>>>>> commit e81e77c5a055d1cbf6d6a6f0acbaf443267aa84f
>>>>> Author:     Konstantin Belousov <kib@FreeBSD.org>
>>>>> AuthorDate: 2021-10-10 12:20:45 +0000
>>>>> Commit:     Konstantin Belousov <kib@FreeBSD.org>
>>>>> CommitDate: 2021-10-10 19:34:40 +0000
>>>>>
>>>>>        Enable PPS_SYNC on amd64, arm64 and armv7
>>>>>        Remove the option from NOTES/LINT, and add to NOTES for powerpc and
>>>>>        riscv.
>>>>>        PR:     259036
>>>>>        Requested by:   John Hay <john@sanren.ac.za>
>>>>>        Discussed with: ian, imp
>>>>>        Sponsored by:   The FreeBSD Foundation
>>>>>        MFC after:      1 week
>>>>
>>>> Hmm, if the option is MI, why move it out of the MI NOTES?  We don't
>>>> generally remove items from NOTES just because they are enabled by default
>>>> in GENERIC.  That would break the functionality of NOTES where it documents
>>>> options (and sometimes provides more detail than the 1-liner comments we
>>>> use in GENERIC).
>>>>
>>>> In this case I would have left NOTES as-is and instead added PPS_SYNC to
>>>> the relevant GENERIC files with a one-line comment leaving the more
>>>> detailed comment in NOTES.
>>>
>>> To have both PPS_SYNC and !PPS_SYNC covered during tinderbox.
>>> If an option is present in both NOTES and GENERIC, there is a chance
>>> that its absence is not compiled.> Bruce' opinion was that LINT should
>>> be mostly complementary to GENERIC.
>>
>> That doesn't match what we actually do.  For example, INET and INET6 are in
>> both, SMP, SOFTUPDATES, etc.  I think you can go through amd64 GENERIC and
>> every option in it is in NOTES except now for PPS_SYNC.
>>
>> I ran this before your commit:
>>
>> % awk '/^(device|options)/ { print $2 }' GENERIC | sort > one
>> % awk '/^(device|options)/ { print $2 }' ../../conf/NOTES ../../x86/conf/NOTES NOTES | sort > two
>>
>> Items only in GENERIC (I would say most of these are probably bugs in the form of
>> missing entries in NOTES, ULE is 4BSD in NOTES instead, and VERBOSE_SYSINIT
>> has the default value of 1 in NOTES):
>>
>> % > comm -23 one two
>> AH_AR5416_INTERRUPT_MITIGATION
>> ATH_ENABLE_11N
>> BUF_TRACKING
>> DDB_CTF
>> FIB_ALGO
>> FULL_BUF_TRACKING
>> IOMMU
>> KDTRACE_FRAME
>> PRINTF_BUFR_SIZE=128
>> RACCT_DEFAULT_TO_DISABLED
>> SCHED_ULE
>> VERBOSE_SYSINIT=0
>> ath_pci
>> ocs_fc
>> pvscsi
>> tws
>> vge
>>
>> The number of options duplicated in both NOTES and GENERIC:
>>
>> % comm -12 one two | wc -l
>>       259
> 
> Ok, are you fine with the following?
> 
> commit 902360bb75ed2bc5a9c33b790eded627cac57483
> Author: Konstantin Belousov <kib@FreeBSD.org>
> Date:   Tue Oct 12 01:02:35 2021 +0300
> 
>      Restore PPS_SYNC in NOTES
>      
>      This partially reverts e81e77c5a055, leaving the option both in
>      GENERICs on amd64/arm64/arm, and in global NOTES file.  Apparently
>      this better matches existing practice, where we do not try to hard
>      to make LINT and GENERIC complimentary.
>      
>      Requested by:   jhb
>      Sponsored by:   The FreeBSD Foundation
>      MFC after:      1 week

Yes, thanks.

-- 
John Baldwin