[PATCH] staging: comedi: use dma_mmap_coherent for DMA-able buffer mmap

[Ian Abbott]

On 25/06/2019 12:47, Dan Carpenter wrote:
> On Tue, Jun 25, 2019 at 12:26:59PM +0100, Ian Abbott wrote:
>>   drivers/staging/comedi/comedi_buf.c  | 150 ++++++++++++++++++---------
>>   drivers/staging/comedi/comedi_fops.c |  39 ++++---
>>   2 files changed, 125 insertions(+), 64 deletions(-)
>>
>> diff --git a/drivers/staging/comedi/comedi_buf.c b/drivers/staging/comedi/comedi_buf.c
>> index d2c8cc72a99d..3ef3ddabf139 100644
>> --- a/drivers/staging/comedi/comedi_buf.c
>> +++ b/drivers/staging/comedi/comedi_buf.c
>> @@ -27,18 +27,19 @@ static void comedi_buf_map_kref_release(struct kref *kref)
>>   	unsigned int i;
>>   
>>   	if (bm->page_list) {
>> -		for (i = 0; i < bm->n_pages; i++) {
>> -			buf = &bm->page_list[i];
>> -			clear_bit(PG_reserved,
>> -				  &(virt_to_page(buf->virt_addr)->flags));
>> -			if (bm->dma_dir != DMA_NONE) {
>> -#ifdef CONFIG_HAS_DMA
>> -				dma_free_coherent(bm->dma_hw_dev,
>> -						  PAGE_SIZE,
>> -						  buf->virt_addr,
>> -						  buf->dma_addr);
>> -#endif
>> -			} else {
>> +		if (bm->dma_dir != DMA_NONE) {
>> +			/*
>> +			 * DMA buffer was allocated as a single block.
>> +			 * Address is in page_list[0].
>> +			 */
>> +			buf = &bm->page_list[0];
>> +			dma_free_coherent(bm->dma_hw_dev,
>> +					  PAGE_SIZE * bm->n_pages,
>> +					  buf->virt_addr, buf->dma_addr);
>> +		} else {
>> +			for (i = 0; i < bm->n_pages; i++) {
>> +				buf = &bm->page_list[i];
>> +				ClearPageReserved(virt_to_page(buf->virt_addr));
> 
> I think we need a NULL check here:
> 
> 	if (!buf->virt_addr)
> 		continue;
> 
>>   				free_page((unsigned long)buf->virt_addr);
>>   			}
>>   		}

Hi Dan, I don't think that is strictly required because bm->n_pages gets 
set to the number of successfully allocated pages (not the number of 
required pages) by comedi_buf_map_alloc():

 > +		for (i = 0; i < n_pages; i++) {
 > +			buf = &bm->page_list[i];
 > +			buf->virt_addr = (void *)get_zeroed_page(GFP_KERNEL);
 > +			if (!buf->virt_addr)
 > +				break;
 > +
 > +			SetPageReserved(virt_to_page(buf->virt_addr));
 > +		}
 > +
 > +		bm->n_pages = i;

Here!           ^

 > +		if (i < n_pages)
 > +			goto err;

 > +err:
 > +	comedi_buf_map_put(bm);
 > +	return NULL;



-- 
-=( Ian Abbott <abbotti at mev.co.uk> || Web: www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268.  Registered address:    )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-