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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wednesday, January 09, 2013 4:42 AM, Ian Abbott wrote:
> 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@xxxxxxxxxxxxxxxxxxx>
>>> Cc: Ian Abbott <abbobbi@xxxxxxxxx>
>>> Cc: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
> 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?

Ugh.. Overlooked the change in the barrier.

Your suggestion sounds ok to me. I'll try to get v3 out today.

Assuming v3 looks ok, the 6 patch series to drivers.c should still apply _after_
this series is merged.

Thanks for the review,
Hartley

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/devel


[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux