Re: git: b827afb9e3a7 - releng/14.1 - Revert "intrng: switch from MAXCPU to mp_ncpus"

From: Zhenlei Huang <zlei_at_FreeBSD.org>
Date: Thu, 16 May 2024 01:56:41 UTC

> On May 16, 2024, at 9:23 AM, Mike Karels <karels@freebsd.org> wrote:
> 
> On 15 May 2024, at 19:58, Zhenlei Huang wrote:
> 
>>> On May 16, 2024, at 12:35 AM, Mike Karels <karels@FreeBSD.org> wrote:
>>> 
>>> The branch releng/14.1 has been updated by karels:
>>> 
>>> URL: https://cgit.FreeBSD.org/src/commit/?id=b827afb9e3a7aaaa2da7d101c46881c646d4df2f
>>> 
>>> commit b827afb9e3a7aaaa2da7d101c46881c646d4df2f
>>> Author:     Mike Karels <karels@FreeBSD.org>
>>> AuthorDate: 2024-05-14 22:44:58 +0000
>>> Commit:     Mike Karels <karels@FreeBSD.org>
>>> CommitDate: 2024-05-15 16:35:38 +0000
>>> 
>>>   Revert "intrng: switch from MAXCPU to mp_ncpus"
>>> 
>>>   This reverts commit b4d11915c73f199501672b278be86e1f63790036.
>>>   This is a direct commit to stable/14.  The change breaks booting
>>>   on older Raspberry Pi 4's, although that works on main.  The cause
>> 
>> Emm, I think this revert affects other arch also. Does this have large impact ? If yes, and only
>> older Paspberry Pi 4 is to be fixed, why not add conditional compile #if directive for Paspberry Pi 4
>> instead ?
> 
> That won't help with installations.  We use one GENERIC config for arm64.  On
> the other hand, arm64 has 32K counters for interrupts and only 1K for IPIs
> (with this reverted and MAXCPU at 1024), so this isn't that big an increment.
> arm has MAXCPU of 4; riscv has 16.  This reversion makes the outcome the same
> as on 14.0.

Thanks for the explanation.

> 
>> For amd64 the MAXCPU has been bumped from 256 to 1024 [1]. That is large IMO.
> 
> This change does not affect amd64 or i386, just systems with INTRNG (arm,
> arm64, and riscv).  So the change is only significant on arm64.

Sorry I was not aware that INTRNG is arm, arm64 and riscv only.

> 
> 		Mike
> 
>> 1. 9051987e40c5 amd64: Bump MAXCPU to 1024 (from 256)
>> 
>>>   is unknown.  The original commit should be redone on stable/14
>>>   if/when it catches up with main.
>>> 
>>>   (cherry picked from commit 3e627553bbd791a4f73eaeea2c2d795fd4e0ee70)
>>> 
>>>   Approved-by:    re (cperciva)
>>> ---
>>> sys/kern/subr_intr.c | 22 +++++++++++-----------
>>> 1 file changed, 11 insertions(+), 11 deletions(-)
>>> 
>>> diff --git a/sys/kern/subr_intr.c b/sys/kern/subr_intr.c
>>> index 49fe20cdc890..6535c42f2404 100644
>>> --- a/sys/kern/subr_intr.c
>>> +++ b/sys/kern/subr_intr.c
>>> @@ -175,11 +175,11 @@ intr_irq_init(void *dummy __unused)
>>> 
>>> 	/*
>>> 	 * - 2 counters for each I/O interrupt.
>>> -	 * - mp_maxid + 1 counters for each IPI counters for SMP.
>>> +	 * - MAXCPU counters for each IPI counters for SMP.
>>> 	 */
>>> 	nintrcnt = intr_nirq * 2;
>>> #ifdef SMP
>>> -	nintrcnt += INTR_IPI_COUNT * (mp_maxid + 1);
>>> +	nintrcnt += INTR_IPI_COUNT * MAXCPU;
>>> #endif
>>> 
>>> 	intrcnt = mallocarray(nintrcnt, sizeof(u_long), M_INTRNG,
>>> @@ -312,18 +312,18 @@ intr_ipi_setup_counters(const char *name)
>>> 	mtx_lock(&isrc_table_lock);
>>> 
>>> 	/*
>>> -	 * We should never have a problem finding mp_maxid + 1 contiguous
>>> -	 * counters, in practice. Interrupts will be allocated sequentially
>>> -	 * during boot, so the array should fill from low to high index. Once
>>> -	 * reserved, the IPI counters will never be released. Similarly, we
>>> -	 * will not need to allocate more IPIs once the system is running.
>>> +	 * We should never have a problem finding MAXCPU contiguous counters,
>>> +	 * in practice. Interrupts will be allocated sequentially during boot,
>>> +	 * so the array should fill from low to high index. Once reserved, the
>>> +	 * IPI counters will never be released. Similarly, we will not need to
>>> +	 * allocate more IPIs once the system is running.
>>> 	 */
>>> -	bit_ffc_area(intrcnt_bitmap, nintrcnt, mp_maxid + 1, &index);
>>> +	bit_ffc_area(intrcnt_bitmap, nintrcnt, MAXCPU, &index);
>>> 	if (index == -1)
>>> 		panic("Failed to allocate %d counters. Array exhausted?",
>>> -		    mp_maxid + 1);
>>> -	bit_nset(intrcnt_bitmap, index, index + mp_maxid);
>>> -	for (i = 0; i < mp_maxid + 1; i++) {
>>> +		    MAXCPU);
>>> +	bit_nset(intrcnt_bitmap, index, index + MAXCPU - 1);
>>> +	for (i = 0; i < MAXCPU; i++) {
>>> 		snprintf(str, INTRNAME_LEN, "cpu%d:%s", i, name);
>>> 		intrcnt_setname(str, index + i);
>>> 	}
>> 
>> Best regards,
>> Zhenlei