[Intel-gfx] [PATCH] drm/i915: Only expand COND once in wait_for()

[Paulo Zanoni]

Em Qua, 2016-09-14 às 13:10 +0100, Dave Gordon escreveu:
> Commentary from Chris Wilson's original version:
> 
> > 
> > I was looking at some wait_for() timeouts on a slow system, with
> > lots of
> > debug enabled (KASAN, lockdep, mmio_debug). Thinking that we were
> > mishandling the timeout, I tried to ensure that we loop at least
> > once
> > after first testing COND. However, the double test of COND either
> > side
> > of the timeout check makes that unlikely. But we can do an
> > equivalent
> > loop, that keeps the COND check after testing for timeout (required
> > so
> > that we are not preempted between testing COND and then testing for
> > a
> > timeout) without expanding COND twice.
> > 
> > The advantage of only expanding COND once is a dramatic reduction
> > in
> > code size:
> > 
> >    text	   data	    bss	    dec	    hex
> >    1308733	   5184	   1152	1315069	 141
> > 0fd	before
> >    1305341	   5184	   1152	1311677	 140
> > 3bd	after
> 
> but it turned out that due to a missing iniitialiser, gcc had "gone
> wild trimming undefined code" :( This version acheives a rather more
> modest (but still worthwhile) gain of ~550 bytes.
> 
> Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
> Original-idea-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Zanoni, Paulo R <paulo.r.zanoni at intel.com>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni at intel.com>

> ---
>  drivers/gpu/drm/i915/intel_drv.h | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index abe7a4d..8fd16ad 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -52,11 +52,15 @@
>   */
>  #define _wait_for(COND, US, W) ({ \
>  	unsigned long timeout__ = jiffies + usecs_to_jiffies(US) +
> 1;	\
> -	int ret__ = 0;						
> 	\
> -	while (!(COND)) {						
> \
> -		if (time_after(jiffies, timeout__)) {		
> 	\
> -			if (!(COND))					
> \
> -				ret__ = -ETIMEDOUT;			
> \
> +	int ret__;							
> \
> +	for (;;) {							
> \
> +		bool expired__ = time_after(jiffies, timeout__);	
> \
> +		if (COND) {						
> \
> +			ret__ = 0;					
> \
> +			break;					
> 	\
> +		}							
> \
> +		if (expired__) {					
> \
> +			ret__ = -ETIMEDOUT;				
> \
>  			break;					
> 	\
>  		}							
> \
>  		if ((W) && drm_can_sleep()) {			
> 	\