Re: git: 234683726708 - main - devclass: make devclass_alloc_unit use M_NOWAIT

> On Mar 6, 2025, at 7:32 PM, Zhenlei Huang <zlei@FreeBSD.org> wrote:
> 
> 
> 
>> On Mar 6, 2025, at 7:03 PM, Mateusz Guzik <mjg@FreeBSD.org <mailto:mjg@FreeBSD.org>> wrote:
>> 
>> The branch main has been updated by mjg:
>> 
>> URL: https://cgit.FreeBSD.org/src/commit/?id=234683726708cf5212d672d676d30056d4133859 <https://cgit.freebsd.org/src/commit/?id=234683726708cf5212d672d676d30056d4133859>
>> 
>> commit 234683726708cf5212d672d676d30056d4133859
>> Author:     Mateusz Guzik <mjg@FreeBSD.org <mailto:mjg@FreeBSD.org>>
>> AuthorDate: 2025-03-06 11:01:49 +0000
>> Commit:     Mateusz Guzik <mjg@FreeBSD.org <mailto:mjg@FreeBSD.org>>
>> CommitDate: 2025-03-06 11:01:49 +0000
>> 
>>    devclass: make devclass_alloc_unit use M_NOWAIT
>> 
>>    The only caller already does this.
>> 
>>    The routine can be called with a mutex held making M_WAITOK illegal.
>> 
>>    Sponsored by:   Rubicon Communications, LLC ("Netgate")
>> ---
>> sys/kern/subr_bus.c | 8 ++++++--
>> 1 file changed, 6 insertions(+), 2 deletions(-)
>> 
>> diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c
>> index 9506e471705c..0422352bba51 100644
>> --- a/sys/kern/subr_bus.c
>> +++ b/sys/kern/subr_bus.c
>> @@ -1208,6 +1208,7 @@ devclass_get_sysctl_tree(devclass_t dc)
>> static int
>> devclass_alloc_unit(devclass_t dc, device_t dev, int *unitp)
>> {
>> +	device_t *devices;
>> 	const char *s;
>> 	int unit = *unitp;
>> 
>> @@ -1264,8 +1265,11 @@ devclass_alloc_unit(devclass_t dc, device_t dev, int *unitp)
>> 		int newsize;
>> 
>> 		newsize = unit + 1;
>> -		dc->devices = reallocf(dc->devices,
>> -		    newsize * sizeof(*dc->devices), M_BUS, M_WAITOK);
>> +		devices = reallocf(dc->devices,
>> +		    newsize * sizeof(*dc->devices), M_BUS, M_NOWAIT);
> 
> I'd recommend against this. From the commit message of f3d3c63442ff, Warner said,
> > In addition, transition to M_WAITOK since this is a sleepable context
> So, the M_WAITOK is intentional.
> 
> Rather than reverting this, the caller devclass_add_device() should use M_WAITOK.
> 
> ```
> -       dev->nameunit = malloc(buflen, M_BUS, M_NOWAIT|M_ZERO);
> -       if (!dev->nameunit)
> -               return (ENOMEM);
> +       dev->nameunit = malloc(buflen, M_BUS, M_WAITOK | M_ZERO);
> ```

Emm, sorry for the noise. I missed the commit message,
> The routine can be called with a mutex held making M_WAITOK illegal.

> 
> Best regards,
> Zhenlei
> 
>> +		if (devices == NULL)
>> +			return (ENOMEM);
>> +		dc->devices = devices;
>> 		memset(dc->devices + dc->maxunit, 0,
>> 		    sizeof(device_t) * (newsize - dc->maxunit));
>> 		dc->maxunit = newsize;