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