Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <38f38f66-0fd5-4e6c-4839-8272cef77046@ni.com>
Date:   Fri, 9 Jun 2023 16:00:55 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.11.0
Subject: Re: [RFC PATCH] pps: Increase PPS_MAX_SOURCES value.
To:     Rodolfo Giometti <giometti@enneenne.com>
Cc:     linux-kernel@vger.kernel.org, brenda.streiff@ni.com
References: <20230605203147.694716-1-charlie.johnston@ni.com>
 <fe435e68-8e05-9078-0fe6-63ef7cce2fc9@enneenne.com>
 <b794bccc-0233-4d78-df6f-bf7c688a7d7d@ni.com>
 <70ce864c-ca13-4fc4-fcb5-9b7f91579a90@enneenne.com>
Content-Language: en-US
From:   Charlie Johnston <charlie.johnston@ni.com>
In-Reply-To: <70ce864c-ca13-4fc4-fcb5-9b7f91579a90@enneenne.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?RW1iTGhGTUhERTVzZW9RTStRNkgzMkdMZEtLSVlid2RkTGxaRUQwbmlPR01M?=
 =?utf-8?B?R1RXMmNiVWE0TGtXYjJJUjc0WUlxT2YrM0R3K2lQcUZKUG5RYkd2SWVEcDNs?=
 =?utf-8?B?SXNnNjBMenZGcU9vQjNQdWNuU3FzeW9jNlpZVzVqTEpHMmRNYnllWk0zeWcv?=
 =?utf-8?B?Q3IzZWNSWk9SUkZTTHI4N3BKK1BQbGdqZUtpU1dEOVhDUDRHNjFOeThNaDYv?=
 =?utf-8?B?RXhTRHVTRVpxMmpMMmkxWnFMRjYwN3VwelZOOGxGL0ROTGtpczRJWjF2bFNS?=
 =?utf-8?B?c3hoUkRucmRlcG8zbXQ1Y25OVm1JNHpGQTJhcU5uaEczV3hOSE1SMmhpYU5u?=
 =?utf-8?B?bkFId2dxbnJRT0kzakswaXExT2RXUVZMTkhjWEtjTHFiUWhXeUxPVVFHNUdQ?=
 =?utf-8?B?ZGZyYUlxVzZFS3ZsNUZQVTJ0ZUEvNHZFZHR1bmhZTkVPVEdRU1owWHV1SXJt?=
 =?utf-8?B?OG5MbER5b0daNGo5dGQ0elhzSXpKdEM1TkkwR09sRlF2c2NzMmo3dmUzdjc4?=
 =?utf-8?B?Wmw5Rnd0R0o5WkRvakF6ZlN6ZGF6b25leVpRMmdidUJiaE1JUTNwSitIRFow?=
 =?utf-8?B?ZHh1dmd4Z3RadmtPVWhPd2s3Q1JiVnZmK3hjMENVaXpaR2haclBPN1JmZ1p1?=
 =?utf-8?B?UklGeDlNTTNJbm5qaFJvVnNSNEE4aW1PTVcrV1A5WUF3Vy9WeUZNYk5GQUJp?=
 =?utf-8?B?Zkh2WDBhdWJOVERETVdBdWNJb0ovTjh2SStVQWtURm9kYlVDNHNPcjl2bWJC?=
 =?utf-8?B?ZW5nNzI5VzhBVHUwTGYwVzl1RWI2eE8zNFBqWVhNWGh4dGtOYTBnZ0FmOWVu?=
 =?utf-8?B?eVo0WFhSL0JnL2JIL2h0WDBranYrSWIwem9aMEhlb0dkWnE0Zk9rRVE5QTZh?=
 =?utf-8?B?ajdkRXVaM3hOc2FwcW9TcXZ0UWllOVJoN0dpM2ZzUFVaWExKd2dtYkNNaWRG?=
 =?utf-8?B?aVFkM0JIQjdpY1U1aGJzdXJMNXFGRmpWbTAzN0VnOFN4YWFuZkMzZ0FLbk4v?=
 =?utf-8?B?YXpUUVpuQWoxcnpTelN3Q2ZMUEw4ZHBPT2YzNGhpdHJGK09zSk9wWS8raVdS?=
 =?utf-8?B?YW1Ta2FBbFB5U25tdXhqMmZVS3JMTEtaZTh1RjBNQWVWcjNVRXdtRm80emFO?=
 =?utf-8?B?YkI2My9lMGQ1MmxNSVduYnh1VXAzVm9zMCs4QnRzeERpVTFOVndIOUNXRCtV?=
 =?utf-8?B?OFg2WnVwT1RGUXpSNjBNN1ZFeldOZHc4VXdSQnBlWmwwVDExTTZjVktWdUp2?=
 =?utf-8?B?MkFVMHJGVEpKbU1oZ0J2M29XTFZWT2U3S3lLbVllS1dRNzZFa3VVNmxjWkp6?=
 =?utf-8?B?Qnc4N2NLRFczM3prZll4NTkzRVNTUnAxZGFyQk4rY3Uyd3hFUm9FTFRZUkxC?=
 =?utf-8?B?cU9Rd09KcUNRSkFYYUJKdkdKUzlvWnF3K0J4dzlxUnFWTVp1ZktheDhqOUxq?=
 =?utf-8?B?UzF0SGtaSk90VTZZbjhCTWZPTUdkN2ZNWkUwUk5ZQjNFdVY3NWRqcWlMeEFi?=
 =?utf-8?B?cUhoeDJQeGRLaUFldGVDNFF3OWVZaUNlS3Q0WlM1L1lnRjVqRmFjY1dZT3FN?=
 =?utf-8?B?QldQQjljQjhZMEFkY25NZ1NidlAvenptbWFsYjNzVEJlSnVuU2RkYVZBQVhN?=
 =?utf-8?B?TUNlTnJsalNnZ3Y4TXluSFRIQ3pzcWM5WnAwODBFNlEvMXBxb01yZm9MZmVo?=
 =?utf-8?B?UzZnQ0JTUW9JQ095WTFsMGZEdmlPVXB4Ui83ajF3cUxPL2FMOGtvVk1nZzJp?=
 =?utf-8?B?NlV1cEl4R1pFdXc1QkZ0dHpuODQzMllvOWQwQVJVUDV5U2l6aU1qbkROeWtP?=
 =?utf-8?B?aXk4dTRyY01BcWZMcXdmdDhRUnBPMlpSWG1GOFZmTnJaaE5RQ2MwR25Vam1m?=
 =?utf-8?B?aDBtemtIMDlWcFk2RFJzVWlLQS80cUYzbG16QXZ2Wk5CNzdxd25jTXZwM1h5?=
 =?utf-8?B?RkFPdmFqSTVZOWo5TWE3aXpURkdXeFMybmdmOWEzWGUwRkU1TGlOazllVFla?=
 =?utf-8?B?TVFqSUhMWmo2TWw4VEdMWGJPaC9yTzUyVlJnbE0wRStVOHgycHVVYUlyc1Rt?=
 =?utf-8?B?OCs3MXhOWlc3SnZNd2hjZkdpK2R2OGNpeEZiRnFSY3VuaXpwdEV5T3hZM2Fm?=
 =?utf-8?B?TGlQMXpwYkt3RWw3TDdlak0ybHVrNDhWaHNycDdvbFBVeUQ4ZkI5WkZkd29Y?=
 =?utf-8?B?bVE9PQ==?=
X-OriginatorOrg: ni.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 8b660a3b-76df-4738-8a14-08db692c9efd
X-MS-Exchange-CrossTenant-AuthSource: SN6PR04MB4879.namprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 09 Jun 2023 21:00:57.1641
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 87ba1f9a-44cd-43a6-b008-6fdb45a5204e
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: qdBjhMjn+ABFsjWr46I3U5gGIpIT1aOQh15TnuGfj+e4fAa/y5tIsZZiO5elpHu5/0fXW5+6rkNAcsfC3RVVjg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR04MB7225
X-Proofpoint-ORIG-GUID: Ac5X0eHmF1yR_hEmo9YES5paikNx8nk4
X-Proofpoint-GUID: Ac5X0eHmF1yR_hEmo9YES5paikNx8nk4
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.254,Aquarius:18.0.957,Hydra:6.0.573,FMLib:17.11.176.26
 definitions=2023-06-09_15,2023-06-09_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=outbound_policy_notspam policy=outbound_policy score=30
 priorityscore=1501 phishscore=0 impostorscore=0 bulkscore=0
 mlxlogscore=877 spamscore=0 malwarescore=0 clxscore=1015 adultscore=0
 lowpriorityscore=0 mlxscore=0 suspectscore=0 classifier=spam adjust=30
 reason=mlx scancount=1 engine=8.12.0-2305260000
 definitions=main-2306090177
X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_LOW,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 6/9/23 02:30, Rodolfo Giometti wrote:
> On 08/06/23 00:07, Charlie Johnston wrote:
>> On 6/7/23 02:33, Rodolfo Giometti wrote:
>>> On 05/06/23 22:31, Charlie Johnston wrote:
>>>> For consistency with what ptp uses for minors, this
>>>> change sets PPS_MAX_SOURCES to MINORMASK + 1.
>>>>
>>>> The PPS_MAX_SOURCES value is currently set to 16. In
>>>> some cases this was not sufficient for a system. For
>>>> example, a system with multiple (4+) PCIe cards each
>>>> with 4 PTP-capable ethernet interfaces could run out
>>>> of the available PPS major:minors if each interface
>>>> registers a PPS source.
>>>>
>>>> Signed-off-by: Charlie Johnston <charlie.johnston@ni.com>
>>>> ---
>>>>    include/uapi/linux/pps.h | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/include/uapi/linux/pps.h b/include/uapi/linux/pps.h
>>>> index 009ebcd8ced5..85f472330da8 100644
>>>> --- a/include/uapi/linux/pps.h
>>>> +++ b/include/uapi/linux/pps.h
>>>> @@ -26,7 +26,7 @@
>>>>    #include <linux/types.h>
>>>>      #define PPS_VERSION        "5.3.6"
>>>> -#define PPS_MAX_SOURCES        16        /* should be enough... */
>>>> +#define PPS_MAX_SOURCES        (MINORMASK + 1)
>>>>      /* Implementation note: the logical states ``assert'' and ``clear''
>>>>     * are implemented in terms of the chip register, i.e. ``assert''
>>>
>>> I have just one question: are you sure that it's safe to call idr_alloc(..., 0, (MINORMASK + 1), ...)?
>>>
>>> Ciao,
>>>
>>> Rodolfo
>>>
>>
>> Thanks for taking a look!
>>
>> My understanding is that idr_alloc(..., start, end, ...) can take any end value up to INT_MAX. It also handles any values <= 0 by treating them as equal to INT_MAX + 1 since the end value is non-inclusive. I can't think of any reason using MINORMASK + 1 here would be an issue since it's much less than the maximum value idr_alloc() allows.
>>
>> A number of drivers (e.g. ptp) just explicitly use a start and end value of 0, but I don't think that change would fit here.
> 
> I see and maybe I should replace the usage of idr_*() with ida_*() as PTP does...
> 
> However the right-thing(TM) to do here should be dropping PPS_MAX_SOURCES at all!
> 
> Let me go deeper in this issue. I'm going to produce a patch set in next days. Have you any chances to test it?
> 
> Ciao,
> 
> Rodolfo
> 

I'll have to check when the system we used for testing is available again (not easy to find a system with 20+ Ethernet ports) but I'd be happy to test a patch!

I know an increase to PPS_MAX_SOURCES was tested on that system.

Thanks,
Charlie