Re: [RFC] An idea for general kernel post-processing automation in FreeBSD

On May 25, 2023, at 07:03, John Baldwin <jhb@FreeBSD.org> wrote:

> On 5/25/23 4:49 AM, Hans Petter Selasky wrote:
>> On 5/25/23 13:42, Hans Petter Selasky wrote:
>>> On 5/25/23 12:57, Mark Millard wrote:
>>>> The pre-existing code expresses explicitly that no other
>>>> routine is allowed to have its own use of the mutex, a
>>>> design choice enforced by the compiler as things are
>>>> written. (The purpose of the limitation to block scope.)
>>>> 
>>>> Your proposed change removes the compiler enforcement of
>>>> that design, allowing use of the mutex by other code in
>>>> mlx4_main.c without any notification by the compiler.
>>>> 
>>>> Your proposal has a direction of being more fragile for
>>>> bad changes without having to be explicit in code updates
>>>> about the change of status.
>>> 
>>> Hi Mark,
>>> 
>>> Looking only at the mutex part alone, you are right, but not when also
>>> considering the SYSINIT() part, as implemented in LinuxKPI currently.
>> To be more precise:
>> The static mutex can only be accessed from within the routine itself,
>> when it is part of a block scope. That is expected.
>> However the static sysinit, which is also inside the block scope of the
>> function, is accessed from _outside_ the function.
>> This might be viewed as a violation of the block scope limitation.
>> Therefore the DEFINE_MUTEX() should be outside the block scope, due to
>> how it is implemented in the LinuxKPI currently.
> 
> I don't think you are correct here.  A sysinit is much like a constructor,
> and it's perfectly valid to pass a pointer to an object that can not be
> accessed by name outside of a scope to some other bit of code outside of
> that scope (e.g. a constructor).

I guess that must happen indirectly via

mutex_lock(&set_port_type_mutex);
and:
mutex_unlock(&set_port_type_mutex);

but I missed how when I looked.

> It's not even just the SYSINIT.  You are
> passing a pointer to this function-static variable to routines like
> sx_xlock, etc. as well which is also outside of scope, but that's perfectly
> fine.  The scoping rules are about when the compiler can resolve the
> variable by name, not restricting which bits of code are able to access
> the variable by reference/pointer.
> 
> However, I also think that this sub-thread has passed its point of usefulness
> and should end.

Okay. But my understanding was that there was still an
intent to reintroduce the change. I still do not expect
that I've solidly understood the justification for the
original change, if it is, in fact, needed. I did view
the material about the "SYSINIT() part" being what was
involved as some progress for that.

May be off list would be better if I'm to get to a solid
understanding.

===
Mark Millard
marklmi at yahoo.com