Re: [RFC] manage multiple entries of scratch page with scatterlist

["Siluvery, Arun" <arun.siluvery@linux.intel.com>]

On 12/06/2014 08:26, Daniel Vetter wrote:
> On Thu, Jun 12, 2014 at 12:49:47AM +0100, Siluvery, Arun wrote:
>> Hi,
>>
>> I am working on a feature to implement support for gem objects to have
>> variable size and realized a problem with the current implementation.
>> Please advice me how to handle this situation efficiently.
>>
>> In this implementation the backing store of the object is replaced with
>> scratch pages according to input range; Initially I store table entries in
>> an array, replace relevant entries with scratch pages and I am using
>> sg_alloc_table_from_pages() to create new sg_table which is assigned to the
>> object. This implementation works as expected but I realized it is wasting
>> memory as scratch page count increases.
>>
>> Consider the worst case scenario where all pages are replaced with scratch
>> pages.
>>
>> The fn sg_alloc_table_from_pages() first computes the number of chunks based
>> on the page frame numbers. PFNs that are consecutive form a chunk and it
>> allocates scatterlists for each chunk which form the sg_table.
>>
>> In case of scratch pages they get the same pfn for each page and
>> sg_alloc_table_from_pages() considers them not part of a chunk and it
>> allocates scatterlist structure for each scratch page which takes lot of
>> memory as the object size increases.
>>
>> I have to tried to modify sg_alloc_table_from_pages() implementation to
>> check for scratch pfn and consider them as single chunk but after the update
>> when iterating through for_each_sg_page() I am seeing different page
>> addresses instead of all pointing to scratch page.
>>
>> Eg. In an object of size 8 pages, scratch_page = ffffea0001120000 and pfn:
>> 0x00044800, the result I get is,
>>
>> page[0]: ffffea0001120000, pfn: 0x00044800,
>> page[1]: ffffea0001120040, pfn: 0x00044801,
>> page[2]: ffffea0001120080, pfn: 0x00044802,
>> page[3]: ffffea00011200c0, pfn: 0x00044803,
>> page[4]: ffffea0001120100, pfn: 0x00044804,
>> page[5]: ffffea0001120140, pfn: 0x00044805,
>> page[6]: ffffea0001120180, pfn: 0x00044806,
>> page[7]: ffffea00011201c0, pfn: 0x00044807,
>>
>> How to manage multiple pages that have same pfn with a single scatterlist
>> and still have it's length equal to (PAGE_SIZE*chunk_size)?
>>
>> I would really appreciate any suggestions to improve this implementation.
>
> sg tables don't have the idea of repeating a given page, since it doesn't
> make a lot of sense. Is the memory overhead really a big problem?
>
One other use case where it can be useful is for the creation of 
blanking buffer. Considering a frame buffer size of 8MB = 2K pages, each 
scatterlist is 32 bytes which takes 64K for an 8MB object.
I think this overhead is acceptable which also simplifies the 
implementation.

> Extending the sg implementation with a flag somewhere to repeat a given
> page instead of incrementing might be possible. But will be a bit of
> effort to push that through the process since we'll touch code outside of
> drm.
I will explore this option if we see any issues with the overhead.
Thank you for your comments.

regards
Arun

> -Daniel
>