Re: [PATCHv5 08/13] v4l: vb2-dma-contig: add support for scatterlist in userptr mode

[Tomasz Stanislawski <t.stanislaws@samsung.com>]

Hi Subash,
Could you post the code of vb2_dc_pages_to_sgt with all printk in it.
It will help us avoid guessing where and what is debugged in the log.

Moreover, I found a line 'size=4294836224' in the log.
It means that size is equal to -131072 (!?!) or there are some invalid
conversions in printk.

Are you suze that you do not pass size = 0 as the function argument?

Notice that earlier versions of dmabuf-for-vb2 patches has offset2
argument instead of size. It was the offset at the end of the buffer.
In (I guess) 95% of cases this offset was 0.

Could you provide only function arguments that causes the failure?
I mean pages array + size (I assume that offset is zero for your test).

Having the arguments we could reproduce that bug.

Regards,
Tomasz Stanislawski





On 05/09/2012 08:46 AM, Subash Patel wrote:
> Hello Tomasz, Laurent,
> 
> I have printed some logs during the dmabuf export and attach for the SGT issue below. Please find it in the attachment. I hope
> it will be useful.
> 
> Regards,
> Subash
> 
> On 05/08/2012 04:45 PM, Subash Patel wrote:
>> Hi Laurent,
>>
>> On 05/08/2012 02:44 PM, Laurent Pinchart wrote:
>>> Hi Subash,
>>>
>>> On Monday 07 May 2012 20:08:25 Subash Patel wrote:
>>>> Hello Thomasz, Laurent,
>>>>
>>>> I found an issue in the function vb2_dc_pages_to_sgt() below. I saw that
>>>> during the attach, the size of the SGT and size requested mis-matched
>>>> (by atleast 8k bytes). Hence I made a small correction to the code as
>>>> below. I could then attach the importer properly.
>>>
>>> Thank you for the report.
>>>
>>> Could you print the content of the sglist (number of chunks and size
>>> of each
>>> chunk) before and after your modifications, as well as the values of
>>> n_pages,
>>> offset and size ?
>> I will put back all the printk's and generate this. As of now, my setup
>> has changed and will do this when I get sometime.
>>>
>>>> On 04/20/2012 08:15 PM, Tomasz Stanislawski wrote:
>>>
>>> [snip]
>>>
>>>>> +static struct sg_table *vb2_dc_pages_to_sgt(struct page **pages,
>>>>> + unsigned int n_pages, unsigned long offset, unsigned long size)
>>>>> +{
>>>>> + struct sg_table *sgt;
>>>>> + unsigned int chunks;
>>>>> + unsigned int i;
>>>>> + unsigned int cur_page;
>>>>> + int ret;
>>>>> + struct scatterlist *s;
>>>>> +
>>>>> + sgt = kzalloc(sizeof *sgt, GFP_KERNEL);
>>>>> + if (!sgt)
>>>>> + return ERR_PTR(-ENOMEM);
>>>>> +
>>>>> + /* compute number of chunks */
>>>>> + chunks = 1;
>>>>> + for (i = 1; i< n_pages; ++i)
>>>>> + if (pages[i] != pages[i - 1] + 1)
>>>>> + ++chunks;
>>>>> +
>>>>> + ret = sg_alloc_table(sgt, chunks, GFP_KERNEL);
>>>>> + if (ret) {
>>>>> + kfree(sgt);
>>>>> + return ERR_PTR(-ENOMEM);
>>>>> + }
>>>>> +
>>>>> + /* merging chunks and putting them into the scatterlist */
>>>>> + cur_page = 0;
>>>>> + for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
>>>>> + unsigned long chunk_size;
>>>>> + unsigned int j;
>>>>
>>>> size = PAGE_SIZE;
>>>>
>>>>> +
>>>>> + for (j = cur_page + 1; j< n_pages; ++j)
>>>>
>>>> for (j = cur_page + 1; j< n_pages; ++j) {
>>>>
>>>>> + if (pages[j] != pages[j - 1] + 1)
>>>>> + break;
>>>>
>>>> size += PAGE
>>>> }
>>>>
>>>>> +
>>>>> + chunk_size = ((j - cur_page)<< PAGE_SHIFT) - offset;
>>>>> + sg_set_page(s, pages[cur_page], min(size, chunk_size), offset);
>>>>
>>>> [DELETE] size -= chunk_size;
>>>>
>>>>> + offset = 0;
>>>>> + cur_page = j;
>>>>> + }
>>>>> +
>>>>> + return sgt;
>>>>> +}
>>>
>> Regards,
>> Subash