public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* About the wait_for() macro in intel_drv.h
@ 2015-07-15 14:07 Zhi Wang
  2015-07-21 12:46 ` Wang, Zhi A
  0 siblings, 1 reply; 2+ messages in thread
From: Zhi Wang @ 2015-07-15 14:07 UTC (permalink / raw)
  To: intel-gfx@lists.freedesktop.org

Hi Gurus:
    Recently we found that the wait_for() marco in intel_drv.h looks
will cause dead lock. wait_for() macro will try to wait jiffies change
in a condition check loop, but when local cpu interrupt is disabled, it
seems the jiffies will never change under a uni-processor environment.
And in forcewake_get/put path, the local cpu irq will always be disabled
by a spin_lock_irqsave() in genX_read() and some genX_write(), So if the
change of HW register (e.g. forcewake ack) turns slow at this time,
there should be a dead lock here I think?

Thanks,
Zhi.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: About the wait_for() macro in intel_drv.h
  2015-07-15 14:07 About the wait_for() macro in intel_drv.h Zhi Wang
@ 2015-07-21 12:46 ` Wang, Zhi A
  0 siblings, 0 replies; 2+ messages in thread
From: Wang, Zhi A @ 2015-07-21 12:46 UTC (permalink / raw)
  To: Wang, Zhi A, intel-gfx@lists.freedesktop.org

Ping. :)

[code]
#define _wait_for(COND, MS, W) ({ \
        unsigned long timeout__ = jiffies + msecs_to_jiffies(MS) + 1;   \
        int ret__ = 0;                                                  \
        while (!(COND)) {                                               \
                if (time_after(jiffies, timeout__)) {                   \                       <-------- jiffies will only be updated when local CPU interrupt is enabled.
                        if (!(COND))                                    \
                                ret__ = -ETIMEDOUT;                     \
                        break;                                          \
                }                                                       \
                if ((W) && drm_can_sleep()) {                           \
                        usleep_range((W)*1000, (W)*2000);               \
                } else {                                                \
                        cpu_relax();                                    \
                }                                                       \
        }                                                               \
        ret__;                                                          \
})
 
#define wait_for(COND, MS) _wait_for(COND, MS, 1)
#define wait_for_atomic(COND, MS) _wait_for(COND, MS, 0)
#define wait_for_atomic_us(COND, US) _wait_for((COND),
[/code]

But in below functions:
[code]
#define __gen8_write(x) \
static void \
gen8_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \
        GEN6_WRITE_HEADER; \              <------------------------  Disable local CPU interrupt
        hsw_unclaimed_reg_debug(dev_priv, reg, false, true); \
        if (reg < 0x40000 && !is_gen8_shadowed(dev_priv, reg)) \
                __force_wake_get(dev_priv, FORCEWAKE_RENDER); \           <------------- wait_for_atomic() in fw_domain_wait_ack_clear()
        __raw_i915_write##x(dev_priv, reg, val); \                                                                                             
        hsw_unclaimed_reg_debug(dev_priv, reg, false, false); \                                                                   
        hsw_unclaimed_reg_detect(dev_priv); \                                                                                                  
        GEN6_WRITE_FOOTER; \               <----------------------- Re-enable local CPU interrupt.                      
}                                                                                                                                                                                  
[/code]

So if the condition of wait_for_atomic(condition) is not true and wait_for_atomic() is called with local CPU interrupt is disabled like the code piece above, and the system is an UP system, there will be deadlock a we are waitting a interrupt-driven condition with local CPU interrupt is disabled?

Thanks,
Zhi.
-----Original Message-----
From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf Of Zhi Wang
Sent: Wednesday, July 15, 2015 10:08 PM
To: intel-gfx@lists.freedesktop.org
Subject: [Intel-gfx] About the wait_for() macro in intel_drv.h

Hi Gurus:
    Recently we found that the wait_for() marco in intel_drv.h looks will cause dead lock. wait_for() macro will try to wait jiffies change in a condition check loop, but when local cpu interrupt is disabled, it seems the jiffies will never change under a uni-processor environment.
And in forcewake_get/put path, the local cpu irq will always be disabled by a spin_lock_irqsave() in genX_read() and some genX_write(), So if the change of HW register (e.g. forcewake ack) turns slow at this time, there should be a dead lock here I think?

Thanks,
Zhi.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-07-21 12:47 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-15 14:07 About the wait_for() macro in intel_drv.h Zhi Wang
2015-07-21 12:46 ` Wang, Zhi A

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox