[PATCH v2 03/11] staging: comedi: comedi_buf: factor out common comedi_buf_write_alloc_* code

Ian Abbott abbotti at mev.co.uk
Wed Jan 9 10:40:45 UTC 2013


On 2013/01/08 11:11 PM, H Hartley Sweeten wrote:
> The only difference between comedi_buf_write_alloc() and the *_strict()
> version is that the *_strict() one will only allocate the chunk if it
> can completely fulfill the request.
> 
> Factor out the common code and add a flag parameter to indicate the 'strict'
> usage. Change the exported functions so they are just wrappers around the
> common function.
> 
> Cleanup the common function a bit and use the comedi_buf_write_n_available()
> helper to determine the number of bytes available.
> 
> Signed-off-by: H Hartley Sweeten <hsweeten at visionengravers.com>
> Cc: Ian Abbott <abbobbi at mev.co.uk>
> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
> ---
>  drivers/staging/comedi/comedi_buf.c | 33 ++++++++++++++-------------------
>  1 file changed, 14 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/staging/comedi/comedi_buf.c b/drivers/staging/comedi/comedi_buf.c
> index a6b22c5..56793d0 100644
> --- a/drivers/staging/comedi/comedi_buf.c
> +++ b/drivers/staging/comedi/comedi_buf.c
> @@ -196,37 +196,32 @@ unsigned int comedi_buf_write_n_available(struct comedi_async *async)
>  	return nbytes;
>  }
>  
> -/* allocates chunk for the writer from free buffer space */
> -unsigned int comedi_buf_write_alloc(struct comedi_async *async,
> -				    unsigned int nbytes)
> +static unsigned int __comedi_buf_write_alloc(struct comedi_async *async,
> +					     unsigned int nbytes, int strict)
>  {
> -	unsigned int free_end = async->buf_read_count + async->prealloc_bufsz;
> +	unsigned int available = comedi_buf_write_n_available(async);
>  
> -	if ((int)(async->buf_write_alloc_count + nbytes - free_end) > 0)
> -		nbytes = free_end - async->buf_write_alloc_count;
> +	if (nbytes > available)
> +		nbytes = strict ? 0 : available;
>  
>  	async->buf_write_alloc_count += nbytes;
> -	/* barrier insures the read of buf_read_count above occurs before
> -	   we write data to the write-alloc'ed buffer space */
> -	smp_mb();
> +
>  	return nbytes;
>  }

Nak.

This code seems to be moving barriers around (even though the comments
may or may not be accurate).  The inserted call to
comedi_buf_write_n_available() includes a barrier, but that occurs
_before_ async->buf_write_alloc_count is updated, rather than after.  Is
there a valid justification for that?

> +
> +/* allocates chunk for the writer from free buffer space */
> +unsigned int comedi_buf_write_alloc(struct comedi_async *async,
> +				    unsigned int nbytes)
> +{
> +	return __comedi_buf_write_alloc(async, nbytes, 0);
> +}
>  EXPORT_SYMBOL(comedi_buf_write_alloc);
>  
>  /* allocates nothing unless it can completely fulfill the request */
>  unsigned int comedi_buf_write_alloc_strict(struct comedi_async *async,
>  					   unsigned int nbytes)
>  {
> -	unsigned int free_end = async->buf_read_count + async->prealloc_bufsz;
> -
> -	if ((int)(async->buf_write_alloc_count + nbytes - free_end) > 0)
> -		nbytes = 0;
> -
> -	async->buf_write_alloc_count += nbytes;
> -	/* barrier insures the read of buf_read_count above occurs before
> -	   we write data to the write-alloc'ed buffer space */
> -	smp_mb();
> -	return nbytes;
> +	return __comedi_buf_write_alloc(async, nbytes, 1);
>  }
>  
>  /* munging is applied to data by core as it passes between user
> 


-- 
-=( Ian Abbott @ MEV Ltd.    E-mail: <abbotti at mev.co.uk>             )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587              )=-



More information about the devel mailing list