[Skiboot] [PATCH 3/4] core/flash: Don't hold flash_lock for the entirety of an opal_flash_op()

[Alistair Popple]

On Thu, 29 Jun 2017 09:38:03 AM Cyril Bur wrote:
> It doesn't make sense to hold the lock to the flash for an entire flash
> op. The flash_lock provides mutual exclusion to the flashes structure
> and each flash element has a busy boolean which ensures that mutual
> exclusion on access of the flash.
> 
> Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
> ---
>  core/flash.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
> 
> diff --git a/core/flash.c b/core/flash.c
> index a905e986..177f7ae1 100644
> --- a/core/flash.c
> +++ b/core/flash.c
> @@ -335,23 +335,16 @@ static int64_t opal_flash_op(enum flash_op op, uint64_t id, uint64_t offset,

I can't remember how we dealt with multiple "sides" on a single flash chip - did
we create a flash struct for each side? Or do we only have one struct flash per
physical chip? If not would this remove multual exclusion on the underlying
hardware accesses?

- Alistair

>  	struct flash *flash = NULL;
>  	int rc;
>  
> -	if (!try_lock(&flash_lock))
> -		return OPAL_BUSY;
> -
>  	list_for_each(&flashes, flash, list)
>  		if (flash->id == id)
>  			break;
>  
> -	if (flash->id != id) {
> +	if (flash->id != id)
>  		/* Couldn't find the flash */
> -		rc = OPAL_PARAMETER;
> -		goto err;
> -	}
> +		return OPAL_PARAMETER;
>  
> -	if (flash->busy) {
> -		rc = OPAL_BUSY;
> -		goto err;
> -	}
> +	if (!flash_reserve(flash))
> +		return OPAL_BUSY;
>  
>  	if (size > flash->size || offset >= flash->size
>  			|| offset + size > flash->size) {
> @@ -387,13 +380,13 @@ static int64_t opal_flash_op(enum flash_op op, uint64_t id, uint64_t offset,
>  		goto err;
>  	}
>  
> -	unlock(&flash_lock);
> +	flash_release(flash);
>  
>  	opal_queue_msg(OPAL_MSG_ASYNC_COMP, NULL, NULL, token, rc);
>  	return OPAL_ASYNC_COMPLETION;
>  
>  err:
> -	unlock(&flash_lock);
> +	flash_release(flash);
>  	return rc;
>  }
>  
>