svn commit: r288653 - in head/sys/dev/drm2: . i915

[Konstantin Belousov]

On Tue, Oct 06, 2015 at 10:19:24PM +0200, Jean-S??bastien P??dron wrote:
> On 04.10.2015 11:46, Konstantin Belousov wrote:
> > On Sun, Oct 04, 2015 at 07:45:37AM +0000, Adrian Chadd wrote:
> >>   * Add missing case statement (gen == 3) in intel_gpu_reset().
> > This seems to be wrong.  The i915 and G33 chipsets do not have registers
> > declared in the 8xx chipset documentation.  More, i915 and G33 have different
> > reset procedures.
> > 
> > The absence of '3' case was copied from the corresponding Linux kernel.
> > Was this change tested, or is there a reference to upstream where the
> > handling was added in this manner ?
> 
> You're right, even in Linux 3.8, the switch does not have a case for
> generation 3.
> 
> >>   * Replace M_WAITOK with M_NOWAIT when the return value of malloc is checked (may be incorrect).
> > This is also incorrect.  At least the modesetting pathes are executed in
> > the syscall context, and sleeping is allowed; the modesetting locks were
> > selected to make sleeping possible.  Using nowait causes random syscalls
> > failure where the requests would succeed otherwise.
> 
> My reasoning was that M_WAITOK could make the display hang/unresponsive
> while the memory is under pressure. The caller should be responsible for
> handling the error instead.
The majority of the calls changed were for the modesetting.  In other words,
the failures would probably affect only setup path, and probably leave the
display in half-configured state.

That said, hang is not the expected outcome of M_WAITOK behaviour.
M_WAITOK indeed prevents real-time, but it only causes hang in case
of memory deadlock. M_NOWAIT should only be used in contexts where
sleepable wait for memory or address space is impossible or causes a
damage to the managed hardware.

> 
> In Linux, *alloc() calls may fail so application should already be
> responsible for that.
I believe there are subtle differences between our- and Linux- nowait
behaviour.  I claim (but do not want to take liability of prove it with
references to Linux code) that our M_NOWAIT may fail transiently due to
pagedaemon not keeping up with load, while Linux top-half nowait alloc
only fails for real out-of-resources conditions.

What I am trying to say, leave M_NOWAIT out of syscalls.  Some time ago
M_NOWAIT also means that the caller is allowed to use reserves to satisfy
allocation, but this was fixed.