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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
