Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1559578

Re: [Intel-gfx] [PATCH v4] lib/scatterlist: Avoid potential scatterlist entry overflow

Show all headers | View raw

On 13/01/2017 22:23, Andy Shevchenko wrote:
>>>> @@ -402,9 +403,16 @@ int sg_alloc_table_from_pages(struct sg_table *sgt,
>>>>
>>>>         /* compute number of contiguous chunks */
>>>>         chunks = 1;
>>>> -       for (i = 1; i < n_pages; ++i)
>>>> -               if (page_to_pfn(pages[i]) != page_to_pfn(pages[i - 1]) +
>>>> 1)
>>>> +       seg_len = PAGE_SIZE;
>>>> +       for (i = 1; i < n_pages; ++i) {
>>>> +               if (seg_len >= max_segment ||
>>>> +                   page_to_pfn(pages[i]) != page_to_pfn(pages[i - 1]) +
>>>> 1) {
>>>>                         ++chunks;
>>>> +                       seg_len = PAGE_SIZE;
>>>> +               } else {
>>>> +                       seg_len += PAGE_SIZE;
>>>> +               }
>>>> +       }
>>>
>>>
>>> Wouldn't be following looks more readable?
>>>
>>> seg_len = 0;
>>> // Are compilers so stupid doing calculation per iteration in
>>> for-conditional?
>>> // for (i = 0; i + 1 < n_pages; i++) ?
>>
>>
>> I didn't get what you meant here?
>
> Why do we start from 1? I see here two micro (?) optimizations:
> 1) starting from 1 on believe that compiler dumb enough to every time
> do a calculation in condition;

The existing code starts from 1 because the pfn condition looks up page 
i - 1. I don't feel there is a need to change that as well.

> 2) ++i instead of i++, but this is just matter of style, it's not a c++.

Note that I haven't changed the existing code in this respect. I am 
happy to change it though.

>>> for (i = 1; i < n_pages; ++i) {
>>>   seg_len += PAGE_SIZE;
>>>   if (seg_len >= max_segment || page_to_pfn(pages[i]) !=
>>> page_to_pfn(pages[i - 1]) + 1) {
>>>     ++chunks;
>>>     seg_len = PAGE_SIZE;
>>>   }
>>> }
>>
>>
>> Tried it in my unit tester but it doesn't work for all scenarios, guess
>> there is a subtle bug somewhere. I don't find it that unreadable so would
>> prefer to leave it since it works.
>
> Last seems has to be
> seg_len = 0;

Oh right, of course. Your suggestion generates a tiny bit smaller binary 
so I am happy to change that as well. I'll resend the patch hopefully today.

Regards,

Tvrtko

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 1/4] lib/scatterlist: Fix offset type in sg_alloc_table_from_pages Tvrtko Ursulin <tursulin@ursulin.net> - 2017-01-11 10:10 +0100
  [PATCH 3/4] lib/scatterlist: Introduce and export __sg_alloc_table_from_pages Tvrtko Ursulin <tursulin@ursulin.net> - 2017-01-11 10:10 +0100
  [PATCH 4/4] drm/i915: Use __sg_alloc_table_from_pages for userptr allocations Tvrtko Ursulin <tursulin@ursulin.net> - 2017-01-11 10:10 +0100
    [PATCH v6] drm/i915: Use __sg_alloc_table_from_pages for userptr allocations Tvrtko Ursulin <tursulin@ursulin.net> - 2017-01-11 13:10 +0100
  [PATCH 2/4] lib/scatterlist: Avoid potential scatterlist entry overflow Tvrtko Ursulin <tursulin@ursulin.net> - 2017-01-11 10:10 +0100
    [PATCH v4] lib/scatterlist: Avoid potential scatterlist entry overflow Tvrtko Ursulin <tursulin@ursulin.net> - 2017-01-11 13:00 +0100
      Re: [PATCH v4] lib/scatterlist: Avoid potential scatterlist entry overflow Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-01-12 01:10 +0100
        Re: [Intel-gfx] [PATCH v4] lib/scatterlist: Avoid potential  scatterlist entry overflow Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> - 2017-01-13 09:40 +0100
          Re: [Intel-gfx] [PATCH v4] lib/scatterlist: Avoid potential  scatterlist entry overflow Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-01-13 23:30 +0100
            Re: [Intel-gfx] [PATCH v4] lib/scatterlist: Avoid potential  scatterlist entry overflow Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> - 2017-01-16 11:10 +0100
              Re: [Intel-gfx] [PATCH v4] lib/scatterlist: Avoid potential  scatterlist entry overflow Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-01-16 11:20 +0100
  [PATCH v5] lib/scatterlist: Introduce and export __sg_alloc_table_from_pages Tvrtko Ursulin <tursulin@ursulin.net> - 2017-01-11 13:00 +0100

csiph-web