Hi Wei,
On 08/08/2015 15:55, Wei Liu wrote:
struct xenvif_rx_meta {
int id;
@@ -80,16 +81,18 @@ struct xenvif_rx_meta {
/* Discriminate from any valid pending_idx value. */
#define INVALID_PENDING_IDX 0xFFFF
-#define MAX_BUFFER_OFFSET PAGE_SIZE
+#define MAX_BUFFER_OFFSET XEN_PAGE_SIZE
#define MAX_PENDING_REQS XEN_NETIF_TX_RING_SIZE
+#define MAX_XEN_SKB_FRAGS (65536 / XEN_PAGE_SIZE + 1)
+
It might be clearer if you add a comment saying the maximum number of
frags is derived from the page size of the grant page, which happens to
be XEN_PAGE_SIZE at the moment.
Will do.
In the future we need to figure out the page size of grant page in a
dynamic way. We shall cross the bridge when we get there.
Right, there is few other places where we would need to do that too (see
MAX_BUFFER_OFFSET for instance).
[..]
+ info.page = page;
+ gnttab_foreach_grant_in_range(page, offset, bytes,
+ xenvif_gop_frag_copy_grant,
+ &info);
Looks like I need to at least wait until the API is settle before giving
my ack.
size -= bytes;
+ offset = 0;
This looks wrong. Should be offset += bytes.
With the new implementation of the loop, each iteration will be on a
different page.
So only the first page has an offset different than zero.
- /* Next frame */
- if (offset == PAGE_SIZE && size) {
+ /* Next page */
+ if (size) {
BUG_ON(!PageCompound(page));
page++;
- offset = 0;
And this should not be deleted, I think.
What is the reason for changing offset calculation? I think there is
still compound page when using 64K page.
The compound pages are still working ... gnttab_foreach_grant_in_range
is called once per page. So the offset can be reset to 0 every time. No
need to add code which would make the result less clear.
We only need to know if the size is not 0 to get the next page.
The patch may not be clear enough to see it's working so I've copied the
result loop below:
while (size > 0) {
BUG_ON(offset >= PAGE_SIZE);
bytes = PAGE_SIZE - offset;
if (bytes > size)
bytes = size;
info.page = page;
gnttab_foreach_grant_in_range(page, offset, bytes,
xenvif_gop_frag_copy_grant,
&info);
size -= bytes;
offset = 0;
/* Next page */
if (size) {
BUG_ON(!PageCompound(page));
page++;
}
}
Regards,
--
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
