lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 10 Aug 2015 10:57:48 +0100 From: Julien Grall <julien.grall@...rix.com> To: Wei Liu <wei.liu2@...rix.com> CC: <xen-devel@...ts.xenproject.org>, <linux-arm-kernel@...ts.infradead.org>, <ian.campbell@...rix.com>, <stefano.stabellini@...citrix.com>, <linux-kernel@...r.kernel.org>, <netdev@...r.kernel.org> Subject: Re: [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 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 netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists