[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity

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 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) {
-                       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,
                size -= bytes;
                offset = 0;

                /* Next page */
                if (size) {


Julien Grall

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.