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 <[email protected]>
> Cc: Ian Abbott <[email protected]>
> Cc: Greg Kroah-Hartman <[email protected]>
> ---
>  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: <[email protected]>             )=-
-=( Tel: +44 (0)161 477 1898   FAX: +44 (0)161 718 3587              )=-
_______________________________________________
devel mailing list
[email protected]
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel

Reply via email to