From: "K. Y. Srinivasan" <kys@xxxxxxxxxxxxx>
Date: Thu, 6 Mar 2014 21:32:36 -0800
> +static u32 fill_pg_buf(struct page *page, u32 offset, u32 len,
> + struct hv_page_buffer *pb)
> +{
> + int j = 0;
> +
> + /* Deal with compund pages by ignoring unused part
> + * of the page.
> + */
> + page += offset >> PAGE_SHIFT;
Please only one space between "offset" and ">>"
> + offset &= ~PAGE_MASK;
> +
> + while (len > 0) {
> + unsigned long bytes;
> +
> + bytes = PAGE_SIZE - offset;
> + if (bytes > len)
> + bytes = len;
> + pb[j].pfn = page_to_pfn(page);
> + pb[j].offset = offset;
> + pb[j].len = bytes;
> +
> + offset += bytes;
> + len -= bytes;
> +
> + if (offset == PAGE_SIZE && len) {
> + page++;
> + offset = 0;
> + j++;
> + }
> + }
> +
> + return j + 1;
> +}
I think this function has some edge case errors.
As I understand it, this function returns how many page buffer entries
were filled in.
But if we fill exactly the end of a page, we will report one too many
in the return value.
Consider an "offset" of X, where X is smaller than PAGE_SIZE. And a
"len" which is exactly "PAGE_SIZE - offset".
We will fill in exactly one page buffer entry, however we will
erroneously return 2 from this function.
I believe the fix is to first replace the test at the end of the loop
with:
page++;
offset = 0;
j++;
And subsequently change the function to just return plain "j".
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
