[Skiboot] [RFC PATCH] FSP: Solidify timeout poll code

[Benjamin Herrenschmidt]

On Mon, 2014-12-08 at 17:41 +0530, Ananth N Mavinakayanahalli wrote:
> Prevent multiple callers to the timeout poller.
> Add more error checking while there.
> 
> Signed-off-by: Ananth N Mavinakayanahalli <ananth at in.ibm.com>
> ---
>  hw/fsp/fsp.c |   13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/fsp/fsp.c b/hw/fsp/fsp.c
> index 8a73f2c..bc5e3e7 100644
> --- a/hw/fsp/fsp.c
> +++ b/hw/fsp/fsp.c
> @@ -91,6 +91,7 @@ static void *fsp_inbound_buf = NULL;
>  static u32 fsp_inbound_off;
>  
>  static struct lock fsp_lock = LOCK_UNLOCKED;
> +static struct lock fsp_timeout_poller_lock = LOCK_UNLOCKED;
>  
>  static u64 fsp_cmdclass_resp_bitmask;
>  static u64 timeout_timer;
> @@ -1904,7 +1905,7 @@ bool fsp_present(void)
>  	return first_fsp != NULL;
>  }
>  
> -static void fsp_timeout_poll(void *data __unused)
> +static void __fsp_timeout_poll(void *data __unused)
>  {
>  	u64 now = mftb();
>  	u64 timeout_val = 0;
> @@ -1952,6 +1953,8 @@ static void fsp_timeout_poll(void *data __unused)
>  				goto next_bit;
>  			}
>  			req = list_top(&cmdclass->msgq,	struct fsp_msg, link);
> +			if (!req)
> +				goto next_bit;
>  			w0 = req->word0;
>  			w1 = req->word1;
>  			mstate = req->state;
> @@ -1976,6 +1979,14 @@ static void fsp_timeout_poll(void *data __unused)
>  	}
>  }
>  
> +static void fsp_timeout_poll(void *data __unused)
> +{
> +	if (try_lock(&fsp_timeout_poller_lock)) {
> +		__fsp_timeout_poll(data);
> +		unlock(&fsp_timeout_poller_lock);
> +	}
> +}

I don't like that, we sometimes call poll in a tight loop , I'd rather
be a little bit smarter and:


	/* The lowest granularity for a message timeout is 30 secs.
	 * So every 30secs, check if there is any message
	 * waiting for a response from the FSP
	 */
	if ((tb_compare(now, timeout_timer) == TB_AAFTERB) ||
		(tb_compare(now, timeout_timer) == TB_AEQUALB))

      ---> take it here

		timeout_timer = now + secs_to_tb(30);


	else
		return;

(We might need to re-check we have expired before updating timeout_timer
so we don't have two racing)

Cheers,
Ben.

>  void fsp_opl(void)
>  {
>  	struct dt_node *iplp;
> 
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot