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

[Ian Abbott]

On 2013-01-09 10:40, Ian Abbott wrote:
> 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?

This comment applies to patches 2 and 3: As there seem to be no previous 
callers of comedi_buf_write_n_available(), just new callers called from 
comedi_buf.c, perhaps make it a static function in comedi_buf.c.  Remove 
the barrier from that function and restore the barrier to 
__comedi_buf_write_alloc().  Also remove the part of 
comedi_buf_write_n_available() that rounds down the number of available 
bytes to a multiple of the sample size, since none of the new callers 
seem to need that.

How does that sound?

>
>> +
>> +/* 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         )=-