Re: CVS commit: src/sys/dev/ic

[Nick Hudson <nick.hudson%gmx.co.uk@localhost>]

On 31/05/2019 07:27, matthew green wrote:
> Nick Hudson writes:
>> On 30/05/2019 21:46, matthew green wrote:
>>>> Committed By:	tnn
>>>> Date:		Thu May 30 07:37:17 UTC 2019
>>>>
>>>> Modified Files:
>>>> 	src/sys/dev/ic: ssdfb.c
>>>>
>>>> Log Message:
>>>> - include uvm.h before uvm_device.h
>>>> - don't need IPL_SCHED here
>>>
>>> the IPL_SCHED change seems backwards to me.
>>>
>>> IPL_VM is the "this driver is not updated to MPSAFE yet" level,
>>> but IPL_SCHED is the MPSAFE level.
>>>
>>> ie, we should be striving to remove any uses of IPL_VM, not
>>> moving (back) to them.
>>
>> In my view we should be using IPL_VM + IST_MPSAFE (and not IPL_SCHED) if
>> the driver is MPSAFE.
>
> why?
>
> IPL_VM is basically synonymous with !MPSAFE in the current
> kernel -- lots of subsystems will take kernel lock if you
> use IPL_VM.


From spl(9)...

              Code running at this level endures the same restrictions as at
              IPL_SCHED, but may use the deprecated malloc(9) or endorsed
              pool_cache(9) interfaces to allocate memory.

              At the time of writing, the global kernel_lock is automatically
              acquired for interrupts at this level, in order to support
              device drivers that do not provide their own multiprocessor syn-
              chronization.  A future release of the system may allow the
              automatic acquisition of kernel_lock to be disabled for individ-
              ual interrupt handlers.

> also, what's IST_MPSAFE?  sounds like some platform specific
> thing.


Sure. Each platform should have it's own. Unless someone wants to make an MI intr_establish.

>  in MI drivers IPL_VM vs IPL_SCHED is the mpsafe tag.

Not according to spl(9).


> if you're wanting to restore some level for drivers to use
> below IPL_SCHED, that may be a reasonable thing to consider
> (though it's been abandoned by most platforms AFAICT), but
> that will require a fairly large rearrangement of spl(9)
> and the interfaces -- either adding a new level entirely
> (IPL_VMSAFE? :-) or adding an mpsafe tag everywhere, so i'm
> really not sure it's a good idea.

Why is the latter not possible?

>
> for now, IPL_VM should be avoided for MPSAFE code.

At this point I disagree :)

Nick