From: KY Srinivasan <kys@xxxxxxxxxxxxx> Date: Sat, 8 Mar 2014 04:12:01 +0000 > > >> -----Original Message----- >> From: David Miller [mailto:davem@xxxxxxxxxxxxx] >> Sent: Saturday, March 8, 2014 3:18 AM >> To: KY Srinivasan >> Cc: netdev@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx; >> devel@xxxxxxxxxxxxxxxxxxxxxx; olaf@xxxxxxxxx; apw@xxxxxxxxxxxxx; >> jasowang@xxxxxxxxxx >> Subject: Re: [PATCH V2 1/6] Drivers: net: hyperv: Enable scatter gather I/O >> >> 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. > > I may be missing something here; but in the case you are describing we would return a value > of one as we should. When offset + len is exactly equal to the page size, we would not increment > the value of j, since at that point len == 0. Since the initial value of j was 0 and we return (j + 1), > we will return 1. That's not what happens, you execute the loop code at least once, and hit: >> > + if (offset == PAGE_SIZE && len) { >> > + page++; >> > + offset = 0; >> > + j++; >> > + } thus incrementing j before the return statement. _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
