Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1202904 > unrolled thread
| Started by | Julien Grall <julien.grall@citrix.com> |
|---|---|
| First post | 2015-08-07 18:50 +0200 |
| Last post | 2015-08-10 15:00 +0200 |
| Articles | 18 on this page of 38 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH v3 00/20] xen/arm64: Add support for 64KB page Julien Grall <julien.grall@citrix.com> - 2015-08-07 18:50 +0200
[PATCH v3 09/20] xen/biomerge: Don't allow biovec to be merge when Linux is not using 4KB page Julien Grall <julien.grall@citrix.com> - 2015-08-07 18:50 +0200
Re: [PATCH v3 09/20] xen/biomerge: Don't allow biovec to be merge when Linux is not using 4KB page Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-08-10 13:00 +0200
Re: [Xen-devel] [PATCH v3 09/20] xen/biomerge: Don't allow biovec to be merge when Linux is not using 4KB page Julien Grall <julien.grall@citrix.com> - 2015-08-10 13:30 +0200
Re: [Xen-devel] [PATCH v3 09/20] xen/biomerge: Don't allow biovec to be merge when Linux is not using 4KB page Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-08-10 13:30 +0200
Re: [Xen-devel] [PATCH v3 09/20] xen/biomerge: Don't allow biovec to be merge when Linux is not using 4KB page Julien Grall <julien.grall@citrix.com> - 2015-08-10 13:40 +0200
Re: [Xen-devel] [PATCH v3 09/20] xen/biomerge: Don't allow biovec to be merge when Linux is not using 4KB page David Vrabel <david.vrabel@citrix.com> - 2015-08-10 14:50 +0200
[PATCH v3 07/20] block/xen-blkfront: Store a page rather a pfn in the grant structure Julien Grall <julien.grall@citrix.com> - 2015-08-07 18:50 +0200
[PATCH v3 06/20] block/xen-blkfront: Split blkif_queue_request in 2 Julien Grall <julien.grall@citrix.com> - 2015-08-07 18:50 +0200
[PATCH v3 03/20] xen: Add Xen specific page definition Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:00 +0200
Re: [PATCH v3 03/20] xen: Add Xen specific page definition Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-08-10 12:50 +0200
[PATCH v3 01/20] net/xen-netback: xenvif_gop_frag_copy: move GSO check out of the loop Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:00 +0200
Re: [PATCH v3 01/20] net/xen-netback: xenvif_gop_frag_copy: move GSO check out of the loop Wei Liu <wei.liu2@citrix.com> - 2015-08-08 16:00 +0200
[PATCH v3 14/20] xen/grant-table: Make it running on 64KB granularity Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
Re: [Xen-devel] [PATCH v3 00/20] xen/arm64: Add support for 64KB page Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
[PATCH v3 10/20] xen/xenbus: Use Xen page definition Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
[PATCH v3 17/20] net/xen-netfront: Make it running on 64KB page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
[PATCH v3 11/20] tty/hvc: xen: Use xen page definition Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
[PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
Re: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-08-10 13:20 +0200
Re: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux Julien Grall <julien.grall@citrix.com> - 2015-08-10 13:40 +0200
Re: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-08-10 15:00 +0200
Re: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux Julien Grall <julien.grall@citrix.com> - 2015-08-10 15:40 +0200
[PATCH v3 13/20] xen/events: fifo: Make it running on 64KB granularity Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
[PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
Re: [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-08-10 14:10 +0200
Re: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity David Vrabel <david.vrabel@citrix.com> - 2015-08-10 14:20 +0200
Re: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-08-10 15:00 +0200
Re: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-10 15:30 +0200
[PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
Re: [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity Wei Liu <wei.liu2@citrix.com> - 2015-08-08 17:00 +0200
Re: [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-10 12:00 +0200
Re: [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity Wei Liu <wei.liu2@citrix.com> - 2015-08-10 13:40 +0200
Re: [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-10 14:10 +0200
[PATCH v3 15/20] block/xen-blkfront: Make it running on 64KB page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
[PATCH v3 16/20] block/xen-blkback: Make it running on 64KB page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
[PATCH v3 20/20] arm/xen: Add support for 64KB page granularity Julien Grall <julien.grall@citrix.com> - 2015-08-07 19:20 +0200
Re: [PATCH v3 20/20] arm/xen: Add support for 64KB page granularity Stefano Stabellini <stefano.stabellini@eu.citrix.com> - 2015-08-10 15:00 +0200
Page 2 of 2 — ← Prev page 1 [2]
| From | Julien Grall <julien.grall@citrix.com> |
|---|---|
| Date | 2015-08-10 13:40 +0200 |
| Subject | Re: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux |
| Message-ID | <pVTAS-53A-25@gated-at.bofh.it> |
| In reply to | #1204174 |
Hi Stefano,
On 10/08/15 12:18, Stefano Stabellini wrote:
>> /* Link back into the page tables if not highmem. */
>> @@ -396,14 +413,15 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
>> static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
>> {
>> enum bp_state state = BP_DONE;
>> - unsigned long pfn, i;
>> + unsigned long i;
>> struct page *page;
>> int ret;
>> struct xen_memory_reservation reservation = {
>> .address_bits = 0,
>> - .extent_order = 0,
>> + .extent_order = EXTENT_ORDER,
>> .domid = DOMID_SELF
>> };
>> + static struct page *pages[ARRAY_SIZE(frame_list)];
>
> This array can be rather large: I would try to avoid it, see below.
[..]
>
> I would simply and avoid introducing a new array:
> pfn = (frame_list[i] << XEN_PAGE_SHIFT) >> PAGE_SHIFT;
> page = pfn_to_page(pfn);
Which won't work because the frame_list contains a gfn and not a pfn.
We need to translate back the gfn into a pfn and the into a page.
The cost of the translation may be big and I wanted to avoid anymore
XEN_PAGE_SHIFT in the code. In general we should avoid to deal with 4KB
PFN when it's not necessary, it make the code more confusing to read.
If your only concern is the size of the array, we could decrease the
number of frames by batch. Or allocation the variable once a boot time.
Regards,
--
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Stefano Stabellini <stefano.stabellini@eu.citrix.com> |
|---|---|
| Date | 2015-08-10 15:00 +0200 |
| Subject | Re: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux |
| Message-ID | <pVUQj-6Mt-23@gated-at.bofh.it> |
| In reply to | #1204183 |
On Mon, 10 Aug 2015, Julien Grall wrote:
> Hi Stefano,
>
> On 10/08/15 12:18, Stefano Stabellini wrote:
> >> /* Link back into the page tables if not highmem. */
> >> @@ -396,14 +413,15 @@ static enum bp_state increase_reservation(unsigned long nr_pages)
> >> static enum bp_state decrease_reservation(unsigned long nr_pages, gfp_t gfp)
> >> {
> >> enum bp_state state = BP_DONE;
> >> - unsigned long pfn, i;
> >> + unsigned long i;
> >> struct page *page;
> >> int ret;
> >> struct xen_memory_reservation reservation = {
> >> .address_bits = 0,
> >> - .extent_order = 0,
> >> + .extent_order = EXTENT_ORDER,
> >> .domid = DOMID_SELF
> >> };
> >> + static struct page *pages[ARRAY_SIZE(frame_list)];
> >
> > This array can be rather large: I would try to avoid it, see below.
>
> [..]
>
> >
> > I would simply and avoid introducing a new array:
> > pfn = (frame_list[i] << XEN_PAGE_SHIFT) >> PAGE_SHIFT;
> > page = pfn_to_page(pfn);
>
> Which won't work because the frame_list contains a gfn and not a pfn.
> We need to translate back the gfn into a pfn and the into a page.
>
> The cost of the translation may be big and I wanted to avoid anymore
> XEN_PAGE_SHIFT in the code. In general we should avoid to deal with 4KB
> PFN when it's not necessary, it make the code more confusing to read.
That is true
> If your only concern is the size of the array, we could decrease the
> number of frames by batch. Or allocation the variable once a boot time.
Yes, that is my only concern. Allocating only nr_pages new struct page*
would be good enough I guess.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Julien Grall <julien.grall@citrix.com> |
|---|---|
| Date | 2015-08-10 15:40 +0200 |
| Subject | Re: [PATCH v3 12/20] xen/balloon: Don't rely on the page granularity is the same for Xen and Linux |
| Message-ID | <pVVt0-7LE-23@gated-at.bofh.it> |
| In reply to | #1204233 |
On 10/08/15 13:55, Stefano Stabellini wrote: >> If your only concern is the size of the array, we could decrease the >> number of frames by batch. Or allocation the variable once a boot time. > > Yes, that is my only concern. Allocating only nr_pages new struct page* > would be good enough I guess. That would be even worst. We shouldn't allocate the array at every call, but at boot time. Note that frame_list is already a static variable use 64KB when 64KB page is used. I guess this will be unlikely to remove that much frame in a single batch. But I will keep this optimization for later. Anyway, I'm wondering if we could re-use the lru field to link the page when allocate them and retrieve in the second loop in order to avoid the pages array. Regards, -- Julien Grall -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Julien Grall <julien.grall@citrix.com> |
|---|---|
| Date | 2015-08-07 19:20 +0200 |
| Subject | [PATCH v3 13/20] xen/events: fifo: Make it running on 64KB granularity |
| Message-ID | <pUTtg-61C-17@gated-at.bofh.it> |
| In reply to | #1202904 |
Only use the first 4KB of the page to store the events channel info. It
means that we will waste 60KB every time we allocate page for:
* control block: a page is allocating per CPU
* event array: a page is allocating everytime we need to expand it
I think we can reduce the memory waste for the 2 areas by:
* control block: sharing between multiple vCPUs. Although it will
require some bookkeeping in order to not free the page when the CPU
goes offline and the other CPUs sharing the page still there
* event array: always extend the array event by 64K (i.e 16 4K
chunk). That would require more care when we fail to expand the
event channel.
Signed-off-by: Julien Grall <julien.grall@citrix.com>
Reviewed-by: David Vrabel <david.vrabel@citrix.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@citrix.com>
---
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Note I haven't updated the suggestion to reduce the memory waste
after David's email [1]. I can do it if necessary.
Changes in v3:
- Add David and Stefano's reviewed-by
[1] http://lists.xen.org/archives/html/xen-devel/2015-07/msg04596.html
---
drivers/xen/events/events_base.c | 2 +-
drivers/xen/events/events_fifo.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
index c49bb7a..00dd923 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -40,11 +40,11 @@
#include <asm/idle.h>
#include <asm/io_apic.h>
#include <asm/xen/pci.h>
-#include <xen/page.h>
#endif
#include <asm/sync_bitops.h>
#include <asm/xen/hypercall.h>
#include <asm/xen/hypervisor.h>
+#include <xen/page.h>
#include <xen/xen.h>
#include <xen/hvm.h>
diff --git a/drivers/xen/events/events_fifo.c b/drivers/xen/events/events_fifo.c
index 1d4baf5..e3e9e3d 100644
--- a/drivers/xen/events/events_fifo.c
+++ b/drivers/xen/events/events_fifo.c
@@ -54,7 +54,7 @@
#include "events_internal.h"
-#define EVENT_WORDS_PER_PAGE (PAGE_SIZE / sizeof(event_word_t))
+#define EVENT_WORDS_PER_PAGE (XEN_PAGE_SIZE / sizeof(event_word_t))
#define MAX_EVENT_ARRAY_PAGES (EVTCHN_FIFO_NR_CHANNELS / EVENT_WORDS_PER_PAGE)
struct evtchn_fifo_queue {
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Julien Grall <julien.grall@citrix.com> |
|---|---|
| Date | 2015-08-07 19:20 +0200 |
| Subject | [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity |
| Message-ID | <pUTtg-61C-15@gated-at.bofh.it> |
| In reply to | #1202904 |
The hypercall interface (as well as the toolstack) is always using 4KB
page granularity. When the toolstack is asking for mapping a series of
guest PFN in a batch, it expects to have the page map contiguously in
its virtual memory.
When Linux is using 64KB page granularity, the privcmd driver will have
to map multiple Xen PFN in a single Linux page.
Note that this solution works on page granularity which is a multiple of
4KB.
Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
I kept the hypercall arguments in remap_data to avoid allocating them on
the stack every time that remap_pte_fn is called.
I will keep like that unless someone is strongly disagree.
Changes in v3:
- The function to split a Linux page in mutiple Xen page has
been moved internally. It was the only use (not used anymore in
the balloon) and it's not quite clear what should be the common
interface. Differ the question until someone need to use it.
- s/nr_pfn/numgfns/ to make clear that we are dealing with GFN
- Use DIV_ROUND_UP rather round_up and fix the usage in
xen_xlate_unmap_gfn_range
Changes in v2:
- Use xen_apply_to_page
---
drivers/xen/privcmd.c | 8 ++--
drivers/xen/xlate_mmu.c | 124 ++++++++++++++++++++++++++++++++----------------
2 files changed, 89 insertions(+), 43 deletions(-)
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index c6deb87..c8798ee 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -446,7 +446,7 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
return -EINVAL;
}
- nr_pages = m.num;
+ nr_pages = DIV_ROUND_UP(m.num, XEN_PFN_PER_PAGE);
if ((m.num <= 0) || (nr_pages > (LONG_MAX >> PAGE_SHIFT)))
return -EINVAL;
@@ -494,7 +494,7 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
goto out_unlock;
}
if (xen_feature(XENFEAT_auto_translated_physmap)) {
- ret = alloc_empty_pages(vma, m.num);
+ ret = alloc_empty_pages(vma, nr_pages);
if (ret < 0)
goto out_unlock;
} else
@@ -518,6 +518,7 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
state.global_error = 0;
state.version = version;
+ BUILD_BUG_ON(((PAGE_SIZE / sizeof(xen_pfn_t)) % XEN_PFN_PER_PAGE) != 0);
/* mmap_batch_fn guarantees ret == 0 */
BUG_ON(traverse_pages_block(m.num, sizeof(xen_pfn_t),
&pagelist, mmap_batch_fn, &state));
@@ -582,12 +583,13 @@ static void privcmd_close(struct vm_area_struct *vma)
{
struct page **pages = vma->vm_private_data;
int numpgs = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
+ int numgfns = (vma->vm_end - vma->vm_start) >> XEN_PAGE_SHIFT;
int rc;
if (!xen_feature(XENFEAT_auto_translated_physmap) || !numpgs || !pages)
return;
- rc = xen_unmap_domain_gfn_range(vma, numpgs, pages);
+ rc = xen_unmap_domain_gfn_range(vma, numgfns, pages);
if (rc == 0)
free_xenballooned_pages(numpgs, pages);
else
diff --git a/drivers/xen/xlate_mmu.c b/drivers/xen/xlate_mmu.c
index cff2387..a1d3904 100644
--- a/drivers/xen/xlate_mmu.c
+++ b/drivers/xen/xlate_mmu.c
@@ -38,31 +38,28 @@
#include <xen/interface/xen.h>
#include <xen/interface/memory.h>
-/* map fgfn of domid to lpfn in the current domain */
-static int map_foreign_page(unsigned long lpfn, unsigned long fgfn,
- unsigned int domid)
-{
- int rc;
- struct xen_add_to_physmap_range xatp = {
- .domid = DOMID_SELF,
- .foreign_domid = domid,
- .size = 1,
- .space = XENMAPSPACE_gmfn_foreign,
- };
- xen_ulong_t idx = fgfn;
- xen_pfn_t gpfn = lpfn;
- int err = 0;
+typedef void (*xen_gfn_fn_t)(unsigned long gfn, void *data);
- set_xen_guest_handle(xatp.idxs, &idx);
- set_xen_guest_handle(xatp.gpfns, &gpfn);
- set_xen_guest_handle(xatp.errs, &err);
+/* Break down the pages in 4KB chunk and call fn for each gfn */
+static void xen_for_each_gfn(struct page **pages, unsigned nr_gfn,
+ xen_gfn_fn_t fn, void *data)
+{
+ unsigned long xen_pfn = 0;
+ struct page *page;
+ int i;
- rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
- return rc < 0 ? rc : err;
+ for (i = 0; i < nr_gfn; i++) {
+ if ((i % XEN_PFN_PER_PAGE) == 0) {
+ page = pages[i / XEN_PFN_PER_PAGE];
+ xen_pfn = xen_page_to_pfn(page);
+ }
+ fn(pfn_to_gfn(xen_pfn++), data);
+ }
}
struct remap_data {
xen_pfn_t *fgfn; /* foreign domain's gfn */
+ int nr_fgfn; /* Number of foreign gfn left to map */
pgprot_t prot;
domid_t domid;
struct vm_area_struct *vma;
@@ -71,24 +68,71 @@ struct remap_data {
struct xen_remap_gfn_info *info;
int *err_ptr;
int mapped;
+
+ /* Hypercall parameters */
+ int h_errs[XEN_PFN_PER_PAGE];
+ xen_ulong_t h_idxs[XEN_PFN_PER_PAGE];
+ xen_pfn_t h_gpfns[XEN_PFN_PER_PAGE];
+
+ int h_iter; /* Iterator */
};
+static void setup_hparams(unsigned long gfn, void *data)
+{
+ struct remap_data *info = data;
+
+ info->h_idxs[info->h_iter] = *info->fgfn;
+ info->h_gpfns[info->h_iter] = gfn;
+ info->h_errs[info->h_iter] = 0;
+
+ info->h_iter++;
+ info->fgfn++;
+}
+
static int remap_pte_fn(pte_t *ptep, pgtable_t token, unsigned long addr,
void *data)
{
struct remap_data *info = data;
struct page *page = info->pages[info->index++];
- unsigned long pfn = page_to_pfn(page);
- pte_t pte = pte_mkspecial(pfn_pte(pfn, info->prot));
- int rc;
+ pte_t pte = pte_mkspecial(pfn_pte(page_to_pfn(page), info->prot));
+ int rc, nr_gfn;
+ uint32_t i;
+ struct xen_add_to_physmap_range xatp = {
+ .domid = DOMID_SELF,
+ .foreign_domid = info->domid,
+ .space = XENMAPSPACE_gmfn_foreign,
+ };
- rc = map_foreign_page(pfn, *info->fgfn, info->domid);
- *info->err_ptr++ = rc;
- if (!rc) {
- set_pte_at(info->vma->vm_mm, addr, ptep, pte);
- info->mapped++;
+ nr_gfn = min_t(typeof(info->nr_fgfn), XEN_PFN_PER_PAGE, info->nr_fgfn);
+ info->nr_fgfn -= nr_gfn;
+
+ info->h_iter = 0;
+ xen_for_each_gfn(&page, nr_gfn, setup_hparams, info);
+ BUG_ON(info->h_iter != nr_gfn);
+
+ set_xen_guest_handle(xatp.idxs, info->h_idxs);
+ set_xen_guest_handle(xatp.gpfns, info->h_gpfns);
+ set_xen_guest_handle(xatp.errs, info->h_errs);
+ xatp.size = nr_gfn;
+
+ rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
+
+ /* info->err_ptr expect to have one error status per Xen PFN */
+ for (i = 0; i < nr_gfn; i++) {
+ int err = (rc < 0) ? rc : info->h_errs[i];
+
+ *(info->err_ptr++) = err;
+ if (!err)
+ info->mapped++;
}
- info->fgfn++;
+
+ /*
+ * Note: The hypercall will return 0 in most of the case if even if
+ * all the fgmfn are not mapped. We still have to update the pte
+ * as the userspace may decide to continue.
+ */
+ if (!rc)
+ set_pte_at(info->vma->vm_mm, addr, ptep, pte);
return 0;
}
@@ -102,13 +146,14 @@ int xen_xlate_remap_gfn_array(struct vm_area_struct *vma,
{
int err;
struct remap_data data;
- unsigned long range = nr << PAGE_SHIFT;
+ unsigned long range = DIV_ROUND_UP(nr, XEN_PFN_PER_PAGE) << PAGE_SHIFT;
/* Kept here for the purpose of making sure code doesn't break
x86 PVOPS */
BUG_ON(!((vma->vm_flags & (VM_PFNMAP | VM_IO)) == (VM_PFNMAP | VM_IO)));
data.fgfn = gfn;
+ data.nr_fgfn = nr;
data.prot = prot;
data.domid = domid;
data.vma = vma;
@@ -123,21 +168,20 @@ int xen_xlate_remap_gfn_array(struct vm_area_struct *vma,
}
EXPORT_SYMBOL_GPL(xen_xlate_remap_gfn_array);
-int xen_xlate_unmap_gfn_range(struct vm_area_struct *vma,
- int nr, struct page **pages)
+static void unmap_gfn(unsigned long gfn, void *data)
{
- int i;
+ struct xen_remove_from_physmap xrp;
- for (i = 0; i < nr; i++) {
- struct xen_remove_from_physmap xrp;
- unsigned long pfn;
+ xrp.domid = DOMID_SELF;
+ xrp.gpfn = gfn;
+ (void)HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrp);
+}
- pfn = page_to_pfn(pages[i]);
+int xen_xlate_unmap_gfn_range(struct vm_area_struct *vma,
+ int nr, struct page **pages)
+{
+ xen_for_each_gfn(pages, nr, unmap_gfn, NULL);
- xrp.domid = DOMID_SELF;
- xrp.gpfn = pfn;
- (void)HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrp);
- }
return 0;
}
EXPORT_SYMBOL_GPL(xen_xlate_unmap_gfn_range);
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Stefano Stabellini <stefano.stabellini@eu.citrix.com> |
|---|---|
| Date | 2015-08-10 14:10 +0200 |
| Subject | Re: [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity |
| Message-ID | <pVU3U-5R8-9@gated-at.bofh.it> |
| In reply to | #1202946 |
On Fri, 7 Aug 2015, Julien Grall wrote:
> The hypercall interface (as well as the toolstack) is always using 4KB
> page granularity. When the toolstack is asking for mapping a series of
> guest PFN in a batch, it expects to have the page map contiguously in
> its virtual memory.
>
> When Linux is using 64KB page granularity, the privcmd driver will have
> to map multiple Xen PFN in a single Linux page.
>
> Note that this solution works on page granularity which is a multiple of
> 4KB.
>
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
>
> ---
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
> Cc: David Vrabel <david.vrabel@citrix.com>
>
> I kept the hypercall arguments in remap_data to avoid allocating them on
> the stack every time that remap_pte_fn is called.
> I will keep like that unless someone is strongly disagree.
>
> Changes in v3:
> - The function to split a Linux page in mutiple Xen page has
> been moved internally. It was the only use (not used anymore in
> the balloon) and it's not quite clear what should be the common
> interface. Differ the question until someone need to use it.
> - s/nr_pfn/numgfns/ to make clear that we are dealing with GFN
> - Use DIV_ROUND_UP rather round_up and fix the usage in
> xen_xlate_unmap_gfn_range
>
> Changes in v2:
> - Use xen_apply_to_page
> ---
> drivers/xen/privcmd.c | 8 ++--
> drivers/xen/xlate_mmu.c | 124 ++++++++++++++++++++++++++++++++----------------
> 2 files changed, 89 insertions(+), 43 deletions(-)
>
> diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
> index c6deb87..c8798ee 100644
> --- a/drivers/xen/privcmd.c
> +++ b/drivers/xen/privcmd.c
> @@ -446,7 +446,7 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
> return -EINVAL;
> }
>
> - nr_pages = m.num;
> + nr_pages = DIV_ROUND_UP(m.num, XEN_PFN_PER_PAGE);
> if ((m.num <= 0) || (nr_pages > (LONG_MAX >> PAGE_SHIFT)))
> return -EINVAL;
>
> @@ -494,7 +494,7 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
> goto out_unlock;
> }
> if (xen_feature(XENFEAT_auto_translated_physmap)) {
> - ret = alloc_empty_pages(vma, m.num);
> + ret = alloc_empty_pages(vma, nr_pages);
> if (ret < 0)
> goto out_unlock;
> } else
> @@ -518,6 +518,7 @@ static long privcmd_ioctl_mmap_batch(void __user *udata, int version)
> state.global_error = 0;
> state.version = version;
>
> + BUILD_BUG_ON(((PAGE_SIZE / sizeof(xen_pfn_t)) % XEN_PFN_PER_PAGE) != 0);
> /* mmap_batch_fn guarantees ret == 0 */
> BUG_ON(traverse_pages_block(m.num, sizeof(xen_pfn_t),
> &pagelist, mmap_batch_fn, &state));
> @@ -582,12 +583,13 @@ static void privcmd_close(struct vm_area_struct *vma)
> {
> struct page **pages = vma->vm_private_data;
> int numpgs = (vma->vm_end - vma->vm_start) >> PAGE_SHIFT;
> + int numgfns = (vma->vm_end - vma->vm_start) >> XEN_PAGE_SHIFT;
> int rc;
>
> if (!xen_feature(XENFEAT_auto_translated_physmap) || !numpgs || !pages)
> return;
>
> - rc = xen_unmap_domain_gfn_range(vma, numpgs, pages);
> + rc = xen_unmap_domain_gfn_range(vma, numgfns, pages);
> if (rc == 0)
> free_xenballooned_pages(numpgs, pages);
> else
> diff --git a/drivers/xen/xlate_mmu.c b/drivers/xen/xlate_mmu.c
> index cff2387..a1d3904 100644
> --- a/drivers/xen/xlate_mmu.c
> +++ b/drivers/xen/xlate_mmu.c
> @@ -38,31 +38,28 @@
> #include <xen/interface/xen.h>
> #include <xen/interface/memory.h>
>
> -/* map fgfn of domid to lpfn in the current domain */
> -static int map_foreign_page(unsigned long lpfn, unsigned long fgfn,
> - unsigned int domid)
> -{
> - int rc;
> - struct xen_add_to_physmap_range xatp = {
> - .domid = DOMID_SELF,
> - .foreign_domid = domid,
> - .size = 1,
> - .space = XENMAPSPACE_gmfn_foreign,
> - };
> - xen_ulong_t idx = fgfn;
> - xen_pfn_t gpfn = lpfn;
> - int err = 0;
> +typedef void (*xen_gfn_fn_t)(unsigned long gfn, void *data);
>
> - set_xen_guest_handle(xatp.idxs, &idx);
> - set_xen_guest_handle(xatp.gpfns, &gpfn);
> - set_xen_guest_handle(xatp.errs, &err);
> +/* Break down the pages in 4KB chunk and call fn for each gfn */
> +static void xen_for_each_gfn(struct page **pages, unsigned nr_gfn,
> + xen_gfn_fn_t fn, void *data)
> +{
> + unsigned long xen_pfn = 0;
> + struct page *page;
> + int i;
>
> - rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
> - return rc < 0 ? rc : err;
> + for (i = 0; i < nr_gfn; i++) {
> + if ((i % XEN_PFN_PER_PAGE) == 0) {
> + page = pages[i / XEN_PFN_PER_PAGE];
If this function is going to be called very frequently you might want to
consider using a shift instead.
page = pages[i >> 4];
With an appropriate macro of course.
> + xen_pfn = xen_page_to_pfn(page);
> + }
> + fn(pfn_to_gfn(xen_pfn++), data);
What is the purpose of incrementing xen_pfn here?
> + }
> }
>
> struct remap_data {
> xen_pfn_t *fgfn; /* foreign domain's gfn */
> + int nr_fgfn; /* Number of foreign gfn left to map */
> pgprot_t prot;
> domid_t domid;
> struct vm_area_struct *vma;
> @@ -71,24 +68,71 @@ struct remap_data {
> struct xen_remap_gfn_info *info;
> int *err_ptr;
> int mapped;
> +
> + /* Hypercall parameters */
> + int h_errs[XEN_PFN_PER_PAGE];
> + xen_ulong_t h_idxs[XEN_PFN_PER_PAGE];
> + xen_pfn_t h_gpfns[XEN_PFN_PER_PAGE];
> +
> + int h_iter; /* Iterator */
> };
>
> +static void setup_hparams(unsigned long gfn, void *data)
> +{
> + struct remap_data *info = data;
> +
> + info->h_idxs[info->h_iter] = *info->fgfn;
> + info->h_gpfns[info->h_iter] = gfn;
> + info->h_errs[info->h_iter] = 0;
> +
> + info->h_iter++;
> + info->fgfn++;
> +}
> +
> static int remap_pte_fn(pte_t *ptep, pgtable_t token, unsigned long addr,
> void *data)
> {
> struct remap_data *info = data;
> struct page *page = info->pages[info->index++];
> - unsigned long pfn = page_to_pfn(page);
> - pte_t pte = pte_mkspecial(pfn_pte(pfn, info->prot));
> - int rc;
> + pte_t pte = pte_mkspecial(pfn_pte(page_to_pfn(page), info->prot));
> + int rc, nr_gfn;
> + uint32_t i;
> + struct xen_add_to_physmap_range xatp = {
> + .domid = DOMID_SELF,
> + .foreign_domid = info->domid,
> + .space = XENMAPSPACE_gmfn_foreign,
> + };
>
> - rc = map_foreign_page(pfn, *info->fgfn, info->domid);
> - *info->err_ptr++ = rc;
> - if (!rc) {
> - set_pte_at(info->vma->vm_mm, addr, ptep, pte);
> - info->mapped++;
> + nr_gfn = min_t(typeof(info->nr_fgfn), XEN_PFN_PER_PAGE, info->nr_fgfn);
> + info->nr_fgfn -= nr_gfn;
> +
> + info->h_iter = 0;
> + xen_for_each_gfn(&page, nr_gfn, setup_hparams, info);
> + BUG_ON(info->h_iter != nr_gfn);
> +
> + set_xen_guest_handle(xatp.idxs, info->h_idxs);
> + set_xen_guest_handle(xatp.gpfns, info->h_gpfns);
> + set_xen_guest_handle(xatp.errs, info->h_errs);
> + xatp.size = nr_gfn;
> +
> + rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
> +
> + /* info->err_ptr expect to have one error status per Xen PFN */
> + for (i = 0; i < nr_gfn; i++) {
> + int err = (rc < 0) ? rc : info->h_errs[i];
> +
> + *(info->err_ptr++) = err;
> + if (!err)
> + info->mapped++;
> }
> - info->fgfn++;
> +
> + /*
> + * Note: The hypercall will return 0 in most of the case if even if
> + * all the fgmfn are not mapped. We still have to update the pte
> + * as the userspace may decide to continue.
> + */
> + if (!rc)
> + set_pte_at(info->vma->vm_mm, addr, ptep, pte);
>
> return 0;
> }
> @@ -102,13 +146,14 @@ int xen_xlate_remap_gfn_array(struct vm_area_struct *vma,
> {
> int err;
> struct remap_data data;
> - unsigned long range = nr << PAGE_SHIFT;
> + unsigned long range = DIV_ROUND_UP(nr, XEN_PFN_PER_PAGE) << PAGE_SHIFT;
>
> /* Kept here for the purpose of making sure code doesn't break
> x86 PVOPS */
> BUG_ON(!((vma->vm_flags & (VM_PFNMAP | VM_IO)) == (VM_PFNMAP | VM_IO)));
>
> data.fgfn = gfn;
> + data.nr_fgfn = nr;
> data.prot = prot;
> data.domid = domid;
> data.vma = vma;
> @@ -123,21 +168,20 @@ int xen_xlate_remap_gfn_array(struct vm_area_struct *vma,
> }
> EXPORT_SYMBOL_GPL(xen_xlate_remap_gfn_array);
>
> -int xen_xlate_unmap_gfn_range(struct vm_area_struct *vma,
> - int nr, struct page **pages)
> +static void unmap_gfn(unsigned long gfn, void *data)
> {
> - int i;
> + struct xen_remove_from_physmap xrp;
>
> - for (i = 0; i < nr; i++) {
> - struct xen_remove_from_physmap xrp;
> - unsigned long pfn;
> + xrp.domid = DOMID_SELF;
> + xrp.gpfn = gfn;
> + (void)HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrp);
> +}
>
> - pfn = page_to_pfn(pages[i]);
> +int xen_xlate_unmap_gfn_range(struct vm_area_struct *vma,
> + int nr, struct page **pages)
> +{
> + xen_for_each_gfn(pages, nr, unmap_gfn, NULL);
>
> - xrp.domid = DOMID_SELF;
> - xrp.gpfn = pfn;
> - (void)HYPERVISOR_memory_op(XENMEM_remove_from_physmap, &xrp);
> - }
> return 0;
> }
> EXPORT_SYMBOL_GPL(xen_xlate_unmap_gfn_range);
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | David Vrabel <david.vrabel@citrix.com> |
|---|---|
| Date | 2015-08-10 14:20 +0200 |
| Subject | Re: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity |
| Message-ID | <pVUdA-62u-25@gated-at.bofh.it> |
| In reply to | #1204204 |
On 10/08/15 13:03, Stefano Stabellini wrote:
> On Fri, 7 Aug 2015, Julien Grall wrote:
>> - rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
>> - return rc < 0 ? rc : err;
>> + for (i = 0; i < nr_gfn; i++) {
>> + if ((i % XEN_PFN_PER_PAGE) == 0) {
>> + page = pages[i / XEN_PFN_PER_PAGE];
>
> If this function is going to be called very frequently you might want to
> consider using a shift instead.
>
> page = pages[i >> 4];
>
> With an appropriate macro of course.
This change isn't necessary. Compilers already turn divides into
suitable shifts.
David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Stefano Stabellini <stefano.stabellini@eu.citrix.com> |
|---|---|
| Date | 2015-08-10 15:00 +0200 |
| Subject | Re: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity |
| Message-ID | <pVUQi-6Mt-5@gated-at.bofh.it> |
| In reply to | #1204212 |
On Mon, 10 Aug 2015, David Vrabel wrote:
> On 10/08/15 13:03, Stefano Stabellini wrote:
> > On Fri, 7 Aug 2015, Julien Grall wrote:
> >> - rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
> >> - return rc < 0 ? rc : err;
> >> + for (i = 0; i < nr_gfn; i++) {
> >> + if ((i % XEN_PFN_PER_PAGE) == 0) {
> >> + page = pages[i / XEN_PFN_PER_PAGE];
> >
> > If this function is going to be called very frequently you might want to
> > consider using a shift instead.
> >
> > page = pages[i >> 4];
> >
> > With an appropriate macro of course.
>
> This change isn't necessary. Compilers already turn divides into
> suitable shifts.
The ARM compiler I used last time I tested this did not, but that was 1
or 2 years ago. In any case to be clear this change is not required.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Julien Grall <julien.grall@citrix.com> |
|---|---|
| Date | 2015-08-10 15:30 +0200 |
| Subject | Re: [Xen-devel] [PATCH v3 19/20] xen/privcmd: Add support for Linux 64KB page granularity |
| Message-ID | <pVVjl-7A8-33@gated-at.bofh.it> |
| In reply to | #1204232 |
Hi Stefano,
On 10/08/15 13:57, Stefano Stabellini wrote:
> On Mon, 10 Aug 2015, David Vrabel wrote:
>> On 10/08/15 13:03, Stefano Stabellini wrote:
>>> On Fri, 7 Aug 2015, Julien Grall wrote:
>>>> - rc = HYPERVISOR_memory_op(XENMEM_add_to_physmap_range, &xatp);
>>>> - return rc < 0 ? rc : err;
>>>> + for (i = 0; i < nr_gfn; i++) {
>>>> + if ((i % XEN_PFN_PER_PAGE) == 0) {
>>>> + page = pages[i / XEN_PFN_PER_PAGE];
>>>
>>> If this function is going to be called very frequently you might want to
>>> consider using a shift instead.
>>>
>>> page = pages[i >> 4];
>>>
>>> With an appropriate macro of course.
>>
>> This change isn't necessary. Compilers already turn divides into
>> suitable shifts.
>
> The ARM compiler I used last time I tested this did not, but that was 1
> or 2 years ago. In any case to be clear this change is not required.
I gave a try on the compiler used by Debian Jessy (gcc 4.9.2). It turns
divides into suitable shifts.
Anyway, if it may happen that older ARM compiler doesn't do this change,
I sure we would have to modify many other places in order to make the
code efficient.
Regards,
--
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Julien Grall <julien.grall@citrix.com> |
|---|---|
| Date | 2015-08-07 19:20 +0200 |
| Subject | [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity |
| Message-ID | <pUTtg-61C-21@gated-at.bofh.it> |
| In reply to | #1202904 |
The PV network protocol is using 4KB page granularity. The goal of this
patch is to allow a Linux using 64KB page granularity working as a
network backend on a non-modified Xen.
It's only necessary to adapt the ring size and break skb data in small
chunk of 4KB. The rest of the code is relying on the grant table code.
Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
Cc: Ian Campbell <ian.campbell@citrix.com>
Cc: Wei Liu <wei.liu2@citrix.com>
Cc: netdev@vger.kernel.org
Improvement such as support of 64KB grant is not taken into
consideration in this patch because we have the requirement to run a
Linux using 64KB pages on a non-modified Xen.
Changes in v3:
- Fix errors reported by checkpatch.pl
- s/mfn/gfn/ based on the new naming
- gnttab_foreach_grant has been renamed to gnttab_forach_grant_in_range
- The grant callback doesn't allow anymore to use less data. An
helpers has been added in netback to handle this.
Changes in v2:
- Correctly set MAX_GRANT_COPY_OPS and XEN_NETBK_RX_SLOTS_MAX
- Don't use XEN_PAGE_SIZE in handle_frag_list as we coalesce
fragment into a new skb
- Use gnntab_foreach_grant to split a Linux page into grant
---
drivers/net/xen-netback/common.h | 15 ++--
drivers/net/xen-netback/netback.c | 153 ++++++++++++++++++++++++--------------
2 files changed, 107 insertions(+), 61 deletions(-)
diff --git a/drivers/net/xen-netback/common.h b/drivers/net/xen-netback/common.h
index 8a495b3..bb68211 100644
--- a/drivers/net/xen-netback/common.h
+++ b/drivers/net/xen-netback/common.h
@@ -44,6 +44,7 @@
#include <xen/interface/grant_table.h>
#include <xen/grant_table.h>
#include <xen/xenbus.h>
+#include <xen/page.h>
#include <linux/debugfs.h>
typedef unsigned int pending_ring_idx_t;
@@ -64,8 +65,8 @@ struct pending_tx_info {
struct ubuf_info callback_struct;
};
-#define XEN_NETIF_TX_RING_SIZE __CONST_RING_SIZE(xen_netif_tx, PAGE_SIZE)
-#define XEN_NETIF_RX_RING_SIZE __CONST_RING_SIZE(xen_netif_rx, PAGE_SIZE)
+#define XEN_NETIF_TX_RING_SIZE __CONST_RING_SIZE(xen_netif_tx, XEN_PAGE_SIZE)
+#define XEN_NETIF_RX_RING_SIZE __CONST_RING_SIZE(xen_netif_rx, XEN_PAGE_SIZE)
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's possible for an skb to have a maximal number of frags
* but still be less than MAX_BUFFER_OFFSET in size. Thus the
- * worst-case number of copy operations is MAX_SKB_FRAGS per
+ * worst-case number of copy operations is MAX_XEN_SKB_FRAGS per
* ring slot.
*/
-#define MAX_GRANT_COPY_OPS (MAX_SKB_FRAGS * XEN_NETIF_RX_RING_SIZE)
+#define MAX_GRANT_COPY_OPS (MAX_XEN_SKB_FRAGS * XEN_NETIF_RX_RING_SIZE)
#define NETBACK_INVALID_HANDLE -1
@@ -203,7 +206,7 @@ struct xenvif_queue { /* Per-queue data for xenvif */
/* Maximum number of Rx slots a to-guest packet may use, including the
* slot needed for GSO meta-data.
*/
-#define XEN_NETBK_RX_SLOTS_MAX (MAX_SKB_FRAGS + 1)
+#define XEN_NETBK_RX_SLOTS_MAX ((MAX_XEN_SKB_FRAGS + 1))
enum state_bit_shift {
/* This bit marks that the vif is connected */
diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
index 66f1780..c32a9f2 100644
--- a/drivers/net/xen-netback/netback.c
+++ b/drivers/net/xen-netback/netback.c
@@ -263,6 +263,80 @@ static struct xenvif_rx_meta *get_next_rx_buffer(struct xenvif_queue *queue,
return meta;
}
+struct gop_frag_copy {
+ struct xenvif_queue *queue;
+ struct netrx_pending_operations *npo;
+ struct xenvif_rx_meta *meta;
+ int head;
+ int gso_type;
+
+ struct page *page;
+};
+
+static void xenvif_setup_copy_gop(unsigned long gfn,
+ unsigned int offset,
+ unsigned int *len,
+ struct gop_frag_copy *info)
+{
+ struct gnttab_copy *copy_gop;
+ struct xen_page_foreign *foreign;
+ /* Convenient aliases */
+ struct xenvif_queue *queue = info->queue;
+ struct netrx_pending_operations *npo = info->npo;
+ struct page *page = info->page;
+
+ BUG_ON(npo->copy_off > MAX_BUFFER_OFFSET);
+
+ if (npo->copy_off == MAX_BUFFER_OFFSET)
+ info->meta = get_next_rx_buffer(queue, npo);
+
+ if (npo->copy_off + *len > MAX_BUFFER_OFFSET)
+ *len = MAX_BUFFER_OFFSET - npo->copy_off;
+
+ copy_gop = npo->copy + npo->copy_prod++;
+ copy_gop->flags = GNTCOPY_dest_gref;
+ copy_gop->len = *len;
+
+ foreign = xen_page_foreign(page);
+ if (foreign) {
+ copy_gop->source.domid = foreign->domid;
+ copy_gop->source.u.ref = foreign->gref;
+ copy_gop->flags |= GNTCOPY_source_gref;
+ } else {
+ copy_gop->source.domid = DOMID_SELF;
+ copy_gop->source.u.gmfn = gfn;
+ }
+ copy_gop->source.offset = offset;
+
+ copy_gop->dest.domid = queue->vif->domid;
+ copy_gop->dest.offset = npo->copy_off;
+ copy_gop->dest.u.ref = npo->copy_gref;
+
+ npo->copy_off += *len;
+ info->meta->size += *len;
+
+ /* Leave a gap for the GSO descriptor. */
+ if (info->head && ((1 << info->gso_type) & queue->vif->gso_mask))
+ queue->rx.req_cons++;
+
+ info->head = 0; /* There must be something in this buffer now */
+}
+
+static void xenvif_gop_frag_copy_grant(unsigned long gfn,
+ unsigned offset,
+ unsigned int len,
+ void *data)
+{
+ unsigned int bytes;
+
+ while (len) {
+ bytes = len;
+ xenvif_setup_copy_gop(gfn, offset, &bytes, data);
+ offset += bytes;
+ len -= bytes;
+ }
+}
+
/*
* Set up the grant operations for this fragment. If it's a flipping
* interface, we also set up the unmap request from here.
@@ -272,83 +346,52 @@ static void xenvif_gop_frag_copy(struct xenvif_queue *queue, struct sk_buff *skb
struct page *page, unsigned long size,
unsigned long offset, int *head)
{
- struct gnttab_copy *copy_gop;
- struct xenvif_rx_meta *meta;
+ struct gop_frag_copy info = {
+ .queue = queue,
+ .npo = npo,
+ .head = *head,
+ .gso_type = XEN_NETIF_GSO_TYPE_NONE,
+ };
unsigned long bytes;
- int gso_type = XEN_NETIF_GSO_TYPE_NONE;
if (skb_is_gso(skb)) {
if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
- gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
+ info.gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
- gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
+ info.gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
}
/* Data must not cross a page boundary. */
BUG_ON(size + offset > PAGE_SIZE<<compound_order(page));
- meta = npo->meta + npo->meta_prod - 1;
+ info.meta = npo->meta + npo->meta_prod - 1;
/* Skip unused frames from start of page */
page += offset >> PAGE_SHIFT;
offset &= ~PAGE_MASK;
while (size > 0) {
- struct xen_page_foreign *foreign;
-
BUG_ON(offset >= PAGE_SIZE);
- BUG_ON(npo->copy_off > MAX_BUFFER_OFFSET);
-
- if (npo->copy_off == MAX_BUFFER_OFFSET)
- meta = get_next_rx_buffer(queue, npo);
bytes = PAGE_SIZE - offset;
if (bytes > size)
bytes = size;
- if (npo->copy_off + bytes > MAX_BUFFER_OFFSET)
- bytes = MAX_BUFFER_OFFSET - npo->copy_off;
-
- copy_gop = npo->copy + npo->copy_prod++;
- copy_gop->flags = GNTCOPY_dest_gref;
- copy_gop->len = bytes;
-
- foreign = xen_page_foreign(page);
- if (foreign) {
- copy_gop->source.domid = foreign->domid;
- copy_gop->source.u.ref = foreign->gref;
- copy_gop->flags |= GNTCOPY_source_gref;
- } else {
- copy_gop->source.domid = DOMID_SELF;
- copy_gop->source.u.gmfn =
- virt_to_gfn(page_address(page));
- }
- copy_gop->source.offset = offset;
-
- copy_gop->dest.domid = queue->vif->domid;
- copy_gop->dest.offset = npo->copy_off;
- copy_gop->dest.u.ref = npo->copy_gref;
-
- npo->copy_off += bytes;
- meta->size += bytes;
-
- offset += bytes;
+ info.page = page;
+ gnttab_foreach_grant_in_range(page, offset, bytes,
+ xenvif_gop_frag_copy_grant,
+ &info);
size -= bytes;
+ offset = 0;
- /* Next frame */
- if (offset == PAGE_SIZE && size) {
+ /* Next page */
+ if (size) {
BUG_ON(!PageCompound(page));
page++;
- offset = 0;
}
-
- /* Leave a gap for the GSO descriptor. */
- if (*head && ((1 << gso_type) & queue->vif->gso_mask))
- queue->rx.req_cons++;
-
- *head = 0; /* There must be something in this buffer now. */
-
}
+
+ *head = info.head;
}
/*
@@ -747,7 +790,7 @@ static int xenvif_count_requests(struct xenvif_queue *queue,
first->size -= txp->size;
slots++;
- if (unlikely((txp->offset + txp->size) > PAGE_SIZE)) {
+ if (unlikely((txp->offset + txp->size) > XEN_PAGE_SIZE)) {
netdev_err(queue->vif->dev, "Cross page boundary, txp->offset: %u, size: %u\n",
txp->offset, txp->size);
xenvif_fatal_tx_err(queue->vif);
@@ -1241,11 +1284,11 @@ static void xenvif_tx_build_gops(struct xenvif_queue *queue,
}
/* No crossing a page as the payload mustn't fragment. */
- if (unlikely((txreq.offset + txreq.size) > PAGE_SIZE)) {
+ if (unlikely((txreq.offset + txreq.size) > XEN_PAGE_SIZE)) {
netdev_err(queue->vif->dev,
"txreq.offset: %u, size: %u, end: %lu\n",
txreq.offset, txreq.size,
- (unsigned long)(txreq.offset&~PAGE_MASK) + txreq.size);
+ (unsigned long)(txreq.offset&~XEN_PAGE_MASK) + txreq.size);
xenvif_fatal_tx_err(queue->vif);
break;
}
@@ -1287,7 +1330,7 @@ static void xenvif_tx_build_gops(struct xenvif_queue *queue,
virt_to_gfn(skb->data);
queue->tx_copy_ops[*copy_ops].dest.domid = DOMID_SELF;
queue->tx_copy_ops[*copy_ops].dest.offset =
- offset_in_page(skb->data);
+ offset_in_page(skb->data) & ~XEN_PAGE_MASK;
queue->tx_copy_ops[*copy_ops].len = data_len;
queue->tx_copy_ops[*copy_ops].flags = GNTCOPY_source_gref;
@@ -1780,7 +1823,7 @@ int xenvif_map_frontend_rings(struct xenvif_queue *queue,
goto err;
txs = (struct xen_netif_tx_sring *)addr;
- BACK_RING_INIT(&queue->tx, txs, PAGE_SIZE);
+ BACK_RING_INIT(&queue->tx, txs, XEN_PAGE_SIZE);
err = xenbus_map_ring_valloc(xenvif_to_xenbus_device(queue->vif),
&rx_ring_ref, 1, &addr);
@@ -1788,7 +1831,7 @@ int xenvif_map_frontend_rings(struct xenvif_queue *queue,
goto err;
rxs = (struct xen_netif_rx_sring *)addr;
- BACK_RING_INIT(&queue->rx, rxs, PAGE_SIZE);
+ BACK_RING_INIT(&queue->rx, rxs, XEN_PAGE_SIZE);
return 0;
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Wei Liu <wei.liu2@citrix.com> |
|---|---|
| Date | 2015-08-08 17:00 +0200 |
| Subject | Re: [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity |
| Message-ID | <pVdLk-1Rl-5@gated-at.bofh.it> |
| In reply to | #1202947 |
On Fri, Aug 07, 2015 at 05:46:57PM +0100, Julien Grall wrote:
> The PV network protocol is using 4KB page granularity. The goal of this
> patch is to allow a Linux using 64KB page granularity working as a
> network backend on a non-modified Xen.
>
> It's only necessary to adapt the ring size and break skb data in small
> chunk of 4KB. The rest of the code is relying on the grant table code.
>
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
>
> ---
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: Wei Liu <wei.liu2@citrix.com>
> Cc: netdev@vger.kernel.org
>
[...]
> +#define XEN_NETIF_TX_RING_SIZE __CONST_RING_SIZE(xen_netif_tx, XEN_PAGE_SIZE)
> +#define XEN_NETIF_RX_RING_SIZE __CONST_RING_SIZE(xen_netif_rx, XEN_PAGE_SIZE)
>
> 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.
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.
> /* It's possible for an skb to have a maximal number of frags
> * but still be less than MAX_BUFFER_OFFSET in size. Thus the
> - * worst-case number of copy operations is MAX_SKB_FRAGS per
> + * worst-case number of copy operations is MAX_XEN_SKB_FRAGS per
> * ring slot.
> */
> -#define MAX_GRANT_COPY_OPS (MAX_SKB_FRAGS * XEN_NETIF_RX_RING_SIZE)
> +#define MAX_GRANT_COPY_OPS (MAX_XEN_SKB_FRAGS * XEN_NETIF_RX_RING_SIZE)
>
> #define NETBACK_INVALID_HANDLE -1
>
> @@ -203,7 +206,7 @@ struct xenvif_queue { /* Per-queue data for xenvif */
> /* Maximum number of Rx slots a to-guest packet may use, including the
> * slot needed for GSO meta-data.
> */
> -#define XEN_NETBK_RX_SLOTS_MAX (MAX_SKB_FRAGS + 1)
> +#define XEN_NETBK_RX_SLOTS_MAX ((MAX_XEN_SKB_FRAGS + 1))
>
> enum state_bit_shift {
> /* This bit marks that the vif is connected */
> diff --git a/drivers/net/xen-netback/netback.c b/drivers/net/xen-netback/netback.c
> index 66f1780..c32a9f2 100644
> --- a/drivers/net/xen-netback/netback.c
> +++ b/drivers/net/xen-netback/netback.c
> @@ -263,6 +263,80 @@ static struct xenvif_rx_meta *get_next_rx_buffer(struct xenvif_queue *queue,
> return meta;
> }
>
[...]
> * Set up the grant operations for this fragment. If it's a flipping
> * interface, we also set up the unmap request from here.
> @@ -272,83 +346,52 @@ static void xenvif_gop_frag_copy(struct xenvif_queue *queue, struct sk_buff *skb
> struct page *page, unsigned long size,
> unsigned long offset, int *head)
> {
> - struct gnttab_copy *copy_gop;
> - struct xenvif_rx_meta *meta;
> + struct gop_frag_copy info = {
> + .queue = queue,
> + .npo = npo,
> + .head = *head,
> + .gso_type = XEN_NETIF_GSO_TYPE_NONE,
> + };
> unsigned long bytes;
> - int gso_type = XEN_NETIF_GSO_TYPE_NONE;
>
> if (skb_is_gso(skb)) {
> if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV4)
> - gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
> + info.gso_type = XEN_NETIF_GSO_TYPE_TCPV4;
> else if (skb_shinfo(skb)->gso_type & SKB_GSO_TCPV6)
> - gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
> + info.gso_type = XEN_NETIF_GSO_TYPE_TCPV6;
> }
>
> /* Data must not cross a page boundary. */
> BUG_ON(size + offset > PAGE_SIZE<<compound_order(page));
>
> - meta = npo->meta + npo->meta_prod - 1;
> + info.meta = npo->meta + npo->meta_prod - 1;
>
> /* Skip unused frames from start of page */
> page += offset >> PAGE_SHIFT;
> offset &= ~PAGE_MASK;
>
> while (size > 0) {
> - struct xen_page_foreign *foreign;
> -
> BUG_ON(offset >= PAGE_SIZE);
> - BUG_ON(npo->copy_off > MAX_BUFFER_OFFSET);
> -
> - if (npo->copy_off == MAX_BUFFER_OFFSET)
> - meta = get_next_rx_buffer(queue, npo);
>
> bytes = PAGE_SIZE - offset;
> if (bytes > size)
> bytes = size;
>
> - if (npo->copy_off + bytes > MAX_BUFFER_OFFSET)
> - bytes = MAX_BUFFER_OFFSET - npo->copy_off;
> -
> - copy_gop = npo->copy + npo->copy_prod++;
> - copy_gop->flags = GNTCOPY_dest_gref;
> - copy_gop->len = bytes;
> -
> - foreign = xen_page_foreign(page);
> - if (foreign) {
> - copy_gop->source.domid = foreign->domid;
> - copy_gop->source.u.ref = foreign->gref;
> - copy_gop->flags |= GNTCOPY_source_gref;
> - } else {
> - copy_gop->source.domid = DOMID_SELF;
> - copy_gop->source.u.gmfn =
> - virt_to_gfn(page_address(page));
> - }
> - copy_gop->source.offset = offset;
> -
> - copy_gop->dest.domid = queue->vif->domid;
> - copy_gop->dest.offset = npo->copy_off;
> - copy_gop->dest.u.ref = npo->copy_gref;
> -
> - npo->copy_off += bytes;
> - meta->size += bytes;
> -
> - offset += bytes;
> + 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.
>
> - /* 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.
> }
> -
> - /* Leave a gap for the GSO descriptor. */
> - if (*head && ((1 << gso_type) & queue->vif->gso_mask))
> - queue->rx.req_cons++;
> -
> - *head = 0; /* There must be something in this buffer now. */
> -
> }
> +
> + *head = info.head;
> }
>
The reset looks OK.
Wei.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Julien Grall <julien.grall@citrix.com> |
|---|---|
| Date | 2015-08-10 12:00 +0200 |
| Subject | Re: [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity |
| Message-ID | <pVS28-2wD-49@gated-at.bofh.it> |
| In reply to | #1203278 |
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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Wei Liu <wei.liu2@citrix.com> |
|---|---|
| Date | 2015-08-10 13:40 +0200 |
| Subject | Re: [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity |
| Message-ID | <pVTAR-53A-5@gated-at.bofh.it> |
| In reply to | #1203967 |
On Mon, Aug 10, 2015 at 10:57:48AM +0100, Julien Grall wrote:
[...]
>
> >>+ 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++;
> }
> }
>
Right. That doesn't mean the original code was wrong or anything. But I
don't want to bikeshed about this.
Please add a comment saying that offset is always 0 starting from second
iteration because the gnttab_foreach_grant_in_range makes sure we handle
one page in one go.
Wei.
> Regards,
>
> --
> Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Julien Grall <julien.grall@citrix.com> |
|---|---|
| Date | 2015-08-10 14:10 +0200 |
| Subject | Re: [PATCH v3 18/20] net/xen-netback: Make it running on 64KB page granularity |
| Message-ID | <pVU3U-5R8-11@gated-at.bofh.it> |
| In reply to | #1204181 |
On 10/08/15 12:39, Wei Liu wrote:
> On Mon, Aug 10, 2015 at 10:57:48AM +0100, Julien Grall wrote:
>> 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++;
>> }
>> }
>>
>
> Right. That doesn't mean the original code was wrong or anything. But I
> don't want to bikeshed about this.
I never said the original code was wrong... The original code was
allowing the possibility to copy less data than the length contained in
page.
In the new version, it has been pushed with the callback
xenvif_gop_frag_copy_grant.
> Please add a comment saying that offset is always 0 starting from second
> iteration because the gnttab_foreach_grant_in_range makes sure we handle
> one page in one go.
I think this is superfluous. To be honest, the comment should have been
on the original version and not in the new one. The construction of the
loop was far from obvious that we copied less data.
In this new version, the reason is not because of
gnttab_foreach_grant_in_range is always a page but how the loop has been
constructed.
If you look how bytes has been defined, it will always contain
min(PAGE_SIZE - offset, size)
So for the first page, this will be PAGE_SIZE - offset. A the end of the
loop we reset the offset 0, indeed we copy all the data of the first
page. For the second page and onwards this will always be PAGE_SIZE
except for the last one where we took size.
Regards,
--
Julien Grall
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Julien Grall <julien.grall@citrix.com> |
|---|---|
| Date | 2015-08-07 19:20 +0200 |
| Subject | [PATCH v3 15/20] block/xen-blkfront: Make it running on 64KB page granularity |
| Message-ID | <pUTtg-61C-25@gated-at.bofh.it> |
| In reply to | #1202904 |
The PV block protocol is using 4KB page granularity. The goal of this
patch is to allow a Linux using 64KB page granularity using block
device on a non-modified Xen.
The block API is using segment which should at least be the size of a
Linux page. Therefore, the driver will have to break the page in chunk
of 4K before giving the page to the backend.
Breaking a 64KB segment in 4KB chunk will result to have some chunk with
no data. As the PV protocol always require to have data in the chunk, we
have to count the number of Xen page which will be in use and avoid to
sent empty chunk.
Note that, a pre-defined number of grant is reserved before preparing
the request. This pre-defined number is based on the number and the
maximum size of the segments. If each segment contain a very small
amount of data, the driver may reserve too much grant (16 grant is
reserved per segment with 64KB page granularity).
Futhermore, in the case of persistent grant we allocate one Linux page
per grant although only the 4KB of the page will be effectively use.
This could be improved by share the page with multiple grants.
Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Improvement such as support 64KB grant is not taken into consideration in
this patch because we have the requirement to run a Linux using 64KB page
on a non-modified Xen.
Royger, I haven't retain your acked-by because of some changes which
were not because of naming modifications.
Changes in v3:
- Use DIV_ROUND_UP in INDIRECT_GREFS
- Split lines over 80 characters whenever it's possible
- s/mfn/gfn/ based on the new naming
- The grant callback doesn't allow anymore to change the len
(wasn't used here).
- gnttab_foreach_grant has been renamed to gnttab_foreach_grant_in_range
- Use gnttab_count_grant to get the number of grants in a sg
- Do some renaming to use the correct variable every time
Changes in v2:
- Use gnttab_foreach_grant to split a Linux page into grant
---
drivers/block/xen-blkfront.c | 324 ++++++++++++++++++++++++++++---------------
1 file changed, 213 insertions(+), 111 deletions(-)
diff --git a/drivers/block/xen-blkfront.c b/drivers/block/xen-blkfront.c
index 52105e7..acc4208 100644
--- a/drivers/block/xen-blkfront.c
+++ b/drivers/block/xen-blkfront.c
@@ -78,6 +78,7 @@ struct blk_shadow {
struct grant **grants_used;
struct grant **indirect_grants;
struct scatterlist *sg;
+ unsigned int num_sg;
};
struct split_bio {
@@ -107,8 +108,12 @@ static unsigned int xen_blkif_max_ring_order;
module_param_named(max_ring_page_order, xen_blkif_max_ring_order, int, S_IRUGO);
MODULE_PARM_DESC(max_ring_page_order, "Maximum order of pages to be used for the shared ring");
-#define BLK_RING_SIZE(info) __CONST_RING_SIZE(blkif, PAGE_SIZE * (info)->nr_ring_pages)
-#define BLK_MAX_RING_SIZE __CONST_RING_SIZE(blkif, PAGE_SIZE * XENBUS_MAX_RING_PAGES)
+#define BLK_RING_SIZE(info) \
+ __CONST_RING_SIZE(blkif, XEN_PAGE_SIZE * (info)->nr_ring_pages)
+
+#define BLK_MAX_RING_SIZE \
+ __CONST_RING_SIZE(blkif, XEN_PAGE_SIZE * XENBUS_MAX_RING_PAGES)
+
/*
* ring-ref%i i=(-1UL) would take 11 characters + 'ring-ref' is 8, so 19
* characters are enough. Define to 20 to keep consist with backend.
@@ -147,6 +152,7 @@ struct blkfront_info
unsigned int discard_granularity;
unsigned int discard_alignment;
unsigned int feature_persistent:1;
+ /* Number of 4KB segments handled */
unsigned int max_indirect_segments;
int is_ready;
struct blk_mq_tag_set tag_set;
@@ -175,10 +181,23 @@ static DEFINE_SPINLOCK(minor_lock);
#define DEV_NAME "xvd" /* name in /dev */
-#define SEGS_PER_INDIRECT_FRAME \
- (PAGE_SIZE/sizeof(struct blkif_request_segment))
-#define INDIRECT_GREFS(_segs) \
- ((_segs + SEGS_PER_INDIRECT_FRAME - 1)/SEGS_PER_INDIRECT_FRAME)
+/*
+ * Grants are always the same size as a Xen page (i.e 4KB).
+ * A physical segment is always the same size as a Linux page.
+ * Number of grants per physical segment
+ */
+#define GRANTS_PER_PSEG (PAGE_SIZE / XEN_PAGE_SIZE)
+
+#define GRANTS_PER_INDIRECT_FRAME \
+ (XEN_PAGE_SIZE / sizeof(struct blkif_request_segment))
+
+#define PSEGS_PER_INDIRECT_FRAME \
+ (GRANTS_INDIRECT_FRAME / GRANTS_PSEGS)
+
+#define INDIRECT_GREFS(_grants) \
+ DIV_ROUND_UP(_grants, GRANTS_PER_INDIRECT_FRAME)
+
+#define GREFS(_psegs) ((_psegs) * GRANTS_PER_PSEG)
static int blkfront_setup_indirect(struct blkfront_info *info);
@@ -465,14 +484,100 @@ static int blkif_queue_discard_req(struct request *req)
return 0;
}
+struct setup_rw_req {
+ unsigned int grant_idx;
+ struct blkif_request_segment *segments;
+ struct blkfront_info *info;
+ struct blkif_request *ring_req;
+ grant_ref_t gref_head;
+ unsigned int id;
+ /* Only used when persistent grant is used and it's a read request */
+ bool need_copy;
+ unsigned int bvec_off;
+ char *bvec_data;
+};
+
+static void blkif_setup_rw_req_grant(unsigned long gfn, unsigned int offset,
+ unsigned int len, void *data)
+{
+ struct setup_rw_req *setup = data;
+ int n, ref;
+ struct grant *gnt_list_entry;
+ unsigned int fsect, lsect;
+ /* Convenient aliases */
+ unsigned int grant_idx = setup->grant_idx;
+ struct blkif_request *ring_req = setup->ring_req;
+ struct blkfront_info *info = setup->info;
+ struct blk_shadow *shadow = &info->shadow[setup->id];
+
+ if ((ring_req->operation == BLKIF_OP_INDIRECT) &&
+ (grant_idx % GRANTS_PER_INDIRECT_FRAME == 0)) {
+ if (setup->segments)
+ kunmap_atomic(setup->segments);
+
+ n = grant_idx / GRANTS_PER_INDIRECT_FRAME;
+ gnt_list_entry = get_indirect_grant(&setup->gref_head, info);
+ shadow->indirect_grants[n] = gnt_list_entry;
+ setup->segments = kmap_atomic(gnt_list_entry->page);
+ ring_req->u.indirect.indirect_grefs[n] = gnt_list_entry->gref;
+ }
+
+ gnt_list_entry = get_grant(&setup->gref_head, gfn, info);
+ ref = gnt_list_entry->gref;
+ shadow->grants_used[grant_idx] = gnt_list_entry;
+
+ if (setup->need_copy) {
+ void *shared_data;
+
+ shared_data = kmap_atomic(gnt_list_entry->page);
+ /*
+ * this does not wipe data stored outside the
+ * range sg->offset..sg->offset+sg->length.
+ * Therefore, blkback *could* see data from
+ * previous requests. This is OK as long as
+ * persistent grants are shared with just one
+ * domain. It may need refactoring if this
+ * changes
+ */
+ memcpy(shared_data + offset,
+ setup->bvec_data + setup->bvec_off,
+ len);
+
+ kunmap_atomic(shared_data);
+ setup->bvec_off += len;
+ }
+
+ fsect = offset >> 9;
+ lsect = fsect + (len >> 9) - 1;
+ if (ring_req->operation != BLKIF_OP_INDIRECT) {
+ ring_req->u.rw.seg[grant_idx] =
+ (struct blkif_request_segment) {
+ .gref = ref,
+ .first_sect = fsect,
+ .last_sect = lsect };
+ } else {
+ setup->segments[grant_idx % GRANTS_PER_INDIRECT_FRAME] =
+ (struct blkif_request_segment) {
+ .gref = ref,
+ .first_sect = fsect,
+ .last_sect = lsect };
+ }
+
+ (setup->grant_idx)++;
+}
+
static int blkif_queue_rw_req(struct request *req)
{
struct blkfront_info *info = req->rq_disk->private_data;
struct blkif_request *ring_req;
unsigned long id;
- unsigned int fsect, lsect;
- int i, ref, n;
- struct blkif_request_segment *segments = NULL;
+ int i;
+ struct setup_rw_req setup = {
+ .grant_idx = 0,
+ .segments = NULL,
+ .info = info,
+ .need_copy = rq_data_dir(req) && info->feature_persistent,
+ };
/*
* Used to store if we are able to queue the request by just using
@@ -480,25 +585,23 @@ static int blkif_queue_rw_req(struct request *req)
* as there are not sufficiently many free.
*/
bool new_persistent_gnts;
- grant_ref_t gref_head;
- struct grant *gnt_list_entry = NULL;
struct scatterlist *sg;
- int nseg, max_grefs;
+ int num_sg, max_grefs, num_grant;
- max_grefs = req->nr_phys_segments;
+ max_grefs = req->nr_phys_segments * GRANTS_PER_PSEG;
if (max_grefs > BLKIF_MAX_SEGMENTS_PER_REQUEST)
/*
* If we are using indirect segments we need to account
* for the indirect grefs used in the request.
*/
- max_grefs += INDIRECT_GREFS(req->nr_phys_segments);
+ max_grefs += INDIRECT_GREFS(max_grefs);
/* Check if we have enough grants to allocate a requests */
if (info->persistent_gnts_c < max_grefs) {
new_persistent_gnts = 1;
if (gnttab_alloc_grant_references(
max_grefs - info->persistent_gnts_c,
- &gref_head) < 0) {
+ &setup.gref_head) < 0) {
gnttab_request_free_callback(
&info->callback,
blkif_restart_queue_callback,
@@ -515,12 +618,19 @@ static int blkif_queue_rw_req(struct request *req)
info->shadow[id].request = req;
BUG_ON(info->max_indirect_segments == 0 &&
- req->nr_phys_segments > BLKIF_MAX_SEGMENTS_PER_REQUEST);
+ GREFS(req->nr_phys_segments) > BLKIF_MAX_SEGMENTS_PER_REQUEST);
BUG_ON(info->max_indirect_segments &&
- req->nr_phys_segments > info->max_indirect_segments);
- nseg = blk_rq_map_sg(req->q, req, info->shadow[id].sg);
+ GREFS(req->nr_phys_segments) > info->max_indirect_segments);
+
+ num_sg = blk_rq_map_sg(req->q, req, info->shadow[id].sg);
+ num_grant = 0;
+ /* Calculate the number of grant used */
+ for_each_sg(info->shadow[id].sg, sg, num_sg, i)
+ num_grant += gnttab_count_grant(sg->offset, sg->length);
+
ring_req->u.rw.id = id;
- if (nseg > BLKIF_MAX_SEGMENTS_PER_REQUEST) {
+ info->shadow[id].num_sg = num_sg;
+ if (num_grant > BLKIF_MAX_SEGMENTS_PER_REQUEST) {
/*
* The indirect operation can only be a BLKIF_OP_READ or
* BLKIF_OP_WRITE
@@ -531,7 +641,7 @@ static int blkif_queue_rw_req(struct request *req)
BLKIF_OP_WRITE : BLKIF_OP_READ;
ring_req->u.indirect.sector_number = (blkif_sector_t)blk_rq_pos(req);
ring_req->u.indirect.handle = info->handle;
- ring_req->u.indirect.nr_segments = nseg;
+ ring_req->u.indirect.nr_segments = num_grant;
} else {
ring_req->u.rw.sector_number = (blkif_sector_t)blk_rq_pos(req);
ring_req->u.rw.handle = info->handle;
@@ -559,73 +669,30 @@ static int blkif_queue_rw_req(struct request *req)
ring_req->operation = 0;
}
}
- ring_req->u.rw.nr_segments = nseg;
- }
- for_each_sg(info->shadow[id].sg, sg, nseg, i) {
- fsect = sg->offset >> 9;
- lsect = fsect + (sg->length >> 9) - 1;
-
- if ((ring_req->operation == BLKIF_OP_INDIRECT) &&
- (i % SEGS_PER_INDIRECT_FRAME == 0)) {
- if (segments)
- kunmap_atomic(segments);
-
- n = i / SEGS_PER_INDIRECT_FRAME;
- gnt_list_entry = get_indirect_grant(&gref_head, info);
- info->shadow[id].indirect_grants[n] = gnt_list_entry;
- segments = kmap_atomic(gnt_list_entry->page);
- ring_req->u.indirect.indirect_grefs[n] = gnt_list_entry->gref;
- }
-
- gnt_list_entry = get_grant(&gref_head,
- xen_page_to_gfn(sg_page(sg)),
- info);
- ref = gnt_list_entry->gref;
-
- info->shadow[id].grants_used[i] = gnt_list_entry;
-
- if (rq_data_dir(req) && info->feature_persistent) {
- char *bvec_data;
- void *shared_data;
+ ring_req->u.rw.nr_segments = num_grant;
+ }
- BUG_ON(sg->offset + sg->length > PAGE_SIZE);
+ setup.ring_req = ring_req;
+ setup.id = id;
+ for_each_sg(info->shadow[id].sg, sg, num_sg, i) {
+ BUG_ON(sg->offset + sg->length > PAGE_SIZE);
- shared_data = kmap_atomic(gnt_list_entry->page);
- bvec_data = kmap_atomic(sg_page(sg));
+ if (setup.need_copy) {
+ setup.bvec_off = sg->offset;
+ setup.bvec_data = kmap_atomic(sg_page(sg));
+ }
- /*
- * this does not wipe data stored outside the
- * range sg->offset..sg->offset+sg->length.
- * Therefore, blkback *could* see data from
- * previous requests. This is OK as long as
- * persistent grants are shared with just one
- * domain. It may need refactoring if this
- * changes
- */
- memcpy(shared_data + sg->offset,
- bvec_data + sg->offset,
- sg->length);
+ gnttab_foreach_grant_in_range(sg_page(sg),
+ sg->offset,
+ sg->length,
+ blkif_setup_rw_req_grant,
+ &setup);
- kunmap_atomic(bvec_data);
- kunmap_atomic(shared_data);
- }
- if (ring_req->operation != BLKIF_OP_INDIRECT) {
- ring_req->u.rw.seg[i] =
- (struct blkif_request_segment) {
- .gref = ref,
- .first_sect = fsect,
- .last_sect = lsect };
- } else {
- n = i % SEGS_PER_INDIRECT_FRAME;
- segments[n] =
- (struct blkif_request_segment) {
- .gref = ref,
- .first_sect = fsect,
- .last_sect = lsect };
- }
+ if (setup.need_copy)
+ kunmap_atomic(setup.bvec_data);
}
- if (segments)
- kunmap_atomic(segments);
+ if (setup.segments)
+ kunmap_atomic(setup.segments);
info->ring.req_prod_pvt++;
@@ -633,7 +700,7 @@ static int blkif_queue_rw_req(struct request *req)
info->shadow[id].req = *ring_req;
if (new_persistent_gnts)
- gnttab_free_grant_references(gref_head);
+ gnttab_free_grant_references(setup.gref_head);
return 0;
}
@@ -750,14 +817,14 @@ static int xlvbd_init_blk_queue(struct gendisk *gd, u16 sector_size,
/* Hard sector size and max sectors impersonate the equiv. hardware. */
blk_queue_logical_block_size(rq, sector_size);
blk_queue_physical_block_size(rq, physical_sector_size);
- blk_queue_max_hw_sectors(rq, (segments * PAGE_SIZE) / 512);
+ blk_queue_max_hw_sectors(rq, (segments * XEN_PAGE_SIZE) / 512);
/* Each segment in a request is up to an aligned page in size. */
blk_queue_segment_boundary(rq, PAGE_SIZE - 1);
blk_queue_max_segment_size(rq, PAGE_SIZE);
/* Ensure a merged request will fit in a single I/O ring slot. */
- blk_queue_max_segments(rq, segments);
+ blk_queue_max_segments(rq, segments / GRANTS_PER_PSEG);
/* Make sure buffer addresses are sector-aligned. */
blk_queue_dma_alignment(rq, 511);
@@ -1116,32 +1183,65 @@ free_shadow:
}
+struct copy_from_grant {
+ const struct blk_shadow *s;
+ unsigned int grant_idx;
+ unsigned int bvec_offset;
+ char *bvec_data;
+};
+
+static void blkif_copy_from_grant(unsigned long gfn, unsigned int offset,
+ unsigned int len, void *data)
+{
+ struct copy_from_grant *info = data;
+ char *shared_data;
+ /* Convenient aliases */
+ const struct blk_shadow *s = info->s;
+
+ shared_data = kmap_atomic(s->grants_used[info->grant_idx]->page);
+
+ memcpy(info->bvec_data + info->bvec_offset,
+ shared_data + offset, len);
+
+ info->bvec_offset += len;
+ info->grant_idx++;
+
+ kunmap_atomic(shared_data);
+}
+
static void blkif_completion(struct blk_shadow *s, struct blkfront_info *info,
struct blkif_response *bret)
{
int i = 0;
struct scatterlist *sg;
- char *bvec_data;
- void *shared_data;
- int nseg;
+ int num_sg, num_grant;
+ struct copy_from_grant data = {
+ .s = s,
+ .grant_idx = 0,
+ };
- nseg = s->req.operation == BLKIF_OP_INDIRECT ?
+ num_grant = s->req.operation == BLKIF_OP_INDIRECT ?
s->req.u.indirect.nr_segments : s->req.u.rw.nr_segments;
+ num_sg = s->num_sg;
if (bret->operation == BLKIF_OP_READ && info->feature_persistent) {
- for_each_sg(s->sg, sg, nseg, i) {
+ for_each_sg(s->sg, sg, num_sg, i) {
BUG_ON(sg->offset + sg->length > PAGE_SIZE);
- shared_data = kmap_atomic(s->grants_used[i]->page);
- bvec_data = kmap_atomic(sg_page(sg));
- memcpy(bvec_data + sg->offset,
- shared_data + sg->offset,
- sg->length);
- kunmap_atomic(bvec_data);
- kunmap_atomic(shared_data);
+
+ data.bvec_offset = sg->offset;
+ data.bvec_data = kmap_atomic(sg_page(sg));
+
+ gnttab_foreach_grant_in_range(sg_page(sg),
+ sg->offset,
+ sg->length,
+ blkif_copy_from_grant,
+ &data);
+
+ kunmap_atomic(data.bvec_data);
}
}
/* Add the persistent grant into the list of free grants */
- for (i = 0; i < nseg; i++) {
+ for (i = 0; i < num_grant; i++) {
if (gnttab_query_foreign_access(s->grants_used[i]->gref)) {
/*
* If the grant is still mapped by the backend (the
@@ -1167,7 +1267,7 @@ static void blkif_completion(struct blk_shadow *s, struct blkfront_info *info,
}
}
if (s->req.operation == BLKIF_OP_INDIRECT) {
- for (i = 0; i < INDIRECT_GREFS(nseg); i++) {
+ for (i = 0; i < INDIRECT_GREFS(num_grant); i++) {
if (gnttab_query_foreign_access(s->indirect_grants[i]->gref)) {
if (!info->feature_persistent)
pr_alert_ratelimited("backed has not unmapped grant: %u\n",
@@ -1309,7 +1409,7 @@ static int setup_blkring(struct xenbus_device *dev,
{
struct blkif_sring *sring;
int err, i;
- unsigned long ring_size = info->nr_ring_pages * PAGE_SIZE;
+ unsigned long ring_size = info->nr_ring_pages * XEN_PAGE_SIZE;
grant_ref_t gref[XENBUS_MAX_RING_PAGES];
for (i = 0; i < info->nr_ring_pages; i++)
@@ -1640,8 +1740,8 @@ static int blkif_recover(struct blkfront_info *info)
atomic_set(&split_bio->pending, pending);
split_bio->bio = bio;
for (i = 0; i < pending; i++) {
- offset = (i * segs * PAGE_SIZE) >> 9;
- size = min((unsigned int)(segs * PAGE_SIZE) >> 9,
+ offset = (i * segs * XEN_PAGE_SIZE) >> 9;
+ size = min((unsigned int)(segs * XEN_PAGE_SIZE) >> 9,
(unsigned int)bio_sectors(bio) - offset);
cloned_bio = bio_clone(bio, GFP_NOIO);
BUG_ON(cloned_bio == NULL);
@@ -1752,7 +1852,7 @@ static void blkfront_setup_discard(struct blkfront_info *info)
static int blkfront_setup_indirect(struct blkfront_info *info)
{
- unsigned int indirect_segments, segs;
+ unsigned int indirect_segments, psegs, grants;
int err, i;
err = xenbus_gather(XBT_NIL, info->xbdev->otherend,
@@ -1760,14 +1860,16 @@ static int blkfront_setup_indirect(struct blkfront_info *info)
NULL);
if (err) {
info->max_indirect_segments = 0;
- segs = BLKIF_MAX_SEGMENTS_PER_REQUEST;
+ grants = BLKIF_MAX_SEGMENTS_PER_REQUEST;
} else {
info->max_indirect_segments = min(indirect_segments,
xen_blkif_max_segments);
- segs = info->max_indirect_segments;
+ grants = info->max_indirect_segments;
}
+ psegs = grants / GRANTS_PER_PSEG;
- err = fill_grant_buffer(info, (segs + INDIRECT_GREFS(segs)) * BLK_RING_SIZE(info));
+ err = fill_grant_buffer(info,
+ (grants + INDIRECT_GREFS(grants)) * BLK_RING_SIZE(info));
if (err)
goto out_of_memory;
@@ -1777,7 +1879,7 @@ static int blkfront_setup_indirect(struct blkfront_info *info)
* grants, we need to allocate a set of pages that can be
* used for mapping indirect grefs
*/
- int num = INDIRECT_GREFS(segs) * BLK_RING_SIZE(info);
+ int num = INDIRECT_GREFS(grants) * BLK_RING_SIZE(info);
BUG_ON(!list_empty(&info->indirect_pages));
for (i = 0; i < num; i++) {
@@ -1790,20 +1892,20 @@ static int blkfront_setup_indirect(struct blkfront_info *info)
for (i = 0; i < BLK_RING_SIZE(info); i++) {
info->shadow[i].grants_used = kzalloc(
- sizeof(info->shadow[i].grants_used[0]) * segs,
+ sizeof(info->shadow[i].grants_used[0]) * grants,
GFP_NOIO);
- info->shadow[i].sg = kzalloc(sizeof(info->shadow[i].sg[0]) * segs, GFP_NOIO);
+ info->shadow[i].sg = kzalloc(sizeof(info->shadow[i].sg[0]) * psegs, GFP_NOIO);
if (info->max_indirect_segments)
info->shadow[i].indirect_grants = kzalloc(
sizeof(info->shadow[i].indirect_grants[0]) *
- INDIRECT_GREFS(segs),
+ INDIRECT_GREFS(grants),
GFP_NOIO);
if ((info->shadow[i].grants_used == NULL) ||
(info->shadow[i].sg == NULL) ||
(info->max_indirect_segments &&
(info->shadow[i].indirect_grants == NULL)))
goto out_of_memory;
- sg_init_table(info->shadow[i].sg, segs);
+ sg_init_table(info->shadow[i].sg, psegs);
}
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Julien Grall <julien.grall@citrix.com> |
|---|---|
| Date | 2015-08-07 19:20 +0200 |
| Subject | [PATCH v3 16/20] block/xen-blkback: Make it running on 64KB page granularity |
| Message-ID | <pUTtg-61C-27@gated-at.bofh.it> |
| In reply to | #1202904 |
The PV block protocol is using 4KB page granularity. The goal of this
patch is to allow a Linux using 64KB page granularity behaving as a
block backend on a non-modified Xen.
It's only necessary to adapt the ring size and the number of request per
indirect frames. The rest of the code is relying on the grant table
code.
Note that the grant table code is allocating a Linux page per grant
which will result to waste 6OKB for every grant when Linux is using 64KB
page granularity. This could be improved by sharing the page between
multiple grants.
Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: "Roger Pau Monné" <roger.pau@citrix.com>
Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Cc: David Vrabel <david.vrabel@citrix.com>
Improvement such as support of 64KB grant is not taken into
consideration in this patch because we have the requirement to run a
Linux using 64KB pages on a non-modified Xen.
This has been tested only with a loop device. I plan to test passing
hard drive partition but I didn't yet convert the swiotlb code.
Changes in v3:
- Use DIV_ROUND_UP in INDIRECT_PAGES to avoid a line over 80
characters
---
drivers/block/xen-blkback/blkback.c | 5 +++--
drivers/block/xen-blkback/common.h | 17 +++++++++++++----
drivers/block/xen-blkback/xenbus.c | 9 ++++++---
3 files changed, 22 insertions(+), 9 deletions(-)
diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkback/blkback.c
index ced9677..d5cce8c 100644
--- a/drivers/block/xen-blkback/blkback.c
+++ b/drivers/block/xen-blkback/blkback.c
@@ -961,7 +961,7 @@ static int xen_blkbk_parse_indirect(struct blkif_request *req,
seg[n].nsec = segments[i].last_sect -
segments[i].first_sect + 1;
seg[n].offset = (segments[i].first_sect << 9);
- if ((segments[i].last_sect >= (PAGE_SIZE >> 9)) ||
+ if ((segments[i].last_sect >= (XEN_PAGE_SIZE >> 9)) ||
(segments[i].last_sect < segments[i].first_sect)) {
rc = -EINVAL;
goto unmap;
@@ -1210,6 +1210,7 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif,
req_operation = req->operation == BLKIF_OP_INDIRECT ?
req->u.indirect.indirect_op : req->operation;
+
if ((req->operation == BLKIF_OP_INDIRECT) &&
(req_operation != BLKIF_OP_READ) &&
(req_operation != BLKIF_OP_WRITE)) {
@@ -1268,7 +1269,7 @@ static int dispatch_rw_block_io(struct xen_blkif *blkif,
seg[i].nsec = req->u.rw.seg[i].last_sect -
req->u.rw.seg[i].first_sect + 1;
seg[i].offset = (req->u.rw.seg[i].first_sect << 9);
- if ((req->u.rw.seg[i].last_sect >= (PAGE_SIZE >> 9)) ||
+ if ((req->u.rw.seg[i].last_sect >= (XEN_PAGE_SIZE >> 9)) ||
(req->u.rw.seg[i].last_sect <
req->u.rw.seg[i].first_sect))
goto fail_response;
diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h
index 45a044a..68e87a0 100644
--- a/drivers/block/xen-blkback/common.h
+++ b/drivers/block/xen-blkback/common.h
@@ -39,6 +39,7 @@
#include <asm/pgalloc.h>
#include <asm/hypervisor.h>
#include <xen/grant_table.h>
+#include <xen/page.h>
#include <xen/xenbus.h>
#include <xen/interface/io/ring.h>
#include <xen/interface/io/blkif.h>
@@ -51,12 +52,20 @@ extern unsigned int xen_blkif_max_ring_order;
*/
#define MAX_INDIRECT_SEGMENTS 256
-#define SEGS_PER_INDIRECT_FRAME \
- (PAGE_SIZE/sizeof(struct blkif_request_segment))
+/*
+ * Xen use 4K pages. The guest may use different page size (4K or 64K)
+ * Number of Xen pages per segment
+ */
+#define XEN_PAGES_PER_SEGMENT (PAGE_SIZE / XEN_PAGE_SIZE)
+
+#define XEN_PAGES_PER_INDIRECT_FRAME \
+ (XEN_PAGE_SIZE/sizeof(struct blkif_request_segment))
+#define SEGS_PER_INDIRECT_FRAME \
+ (XEN_PAGES_PER_INDIRECT_FRAME / XEN_PAGES_PER_SEGMENT)
+
#define MAX_INDIRECT_PAGES \
((MAX_INDIRECT_SEGMENTS + SEGS_PER_INDIRECT_FRAME - 1)/SEGS_PER_INDIRECT_FRAME)
-#define INDIRECT_PAGES(_segs) \
- ((_segs + SEGS_PER_INDIRECT_FRAME - 1)/SEGS_PER_INDIRECT_FRAME)
+#define INDIRECT_PAGES(_segs) DIV_ROUND_UP(_segs, XEN_PAGES_PER_INDIRECT_FRAME)
/* Not a real protocol. Used to generate ring structs which contain
* the elements common to all protocols only. This way we get a
diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c
index deb3f00..edd27e4 100644
--- a/drivers/block/xen-blkback/xenbus.c
+++ b/drivers/block/xen-blkback/xenbus.c
@@ -176,21 +176,24 @@ static int xen_blkif_map(struct xen_blkif *blkif, grant_ref_t *gref,
{
struct blkif_sring *sring;
sring = (struct blkif_sring *)blkif->blk_ring;
- BACK_RING_INIT(&blkif->blk_rings.native, sring, PAGE_SIZE * nr_grefs);
+ BACK_RING_INIT(&blkif->blk_rings.native, sring,
+ XEN_PAGE_SIZE * nr_grefs);
break;
}
case BLKIF_PROTOCOL_X86_32:
{
struct blkif_x86_32_sring *sring_x86_32;
sring_x86_32 = (struct blkif_x86_32_sring *)blkif->blk_ring;
- BACK_RING_INIT(&blkif->blk_rings.x86_32, sring_x86_32, PAGE_SIZE * nr_grefs);
+ BACK_RING_INIT(&blkif->blk_rings.x86_32, sring_x86_32,
+ XEN_PAGE_SIZE * nr_grefs);
break;
}
case BLKIF_PROTOCOL_X86_64:
{
struct blkif_x86_64_sring *sring_x86_64;
sring_x86_64 = (struct blkif_x86_64_sring *)blkif->blk_ring;
- BACK_RING_INIT(&blkif->blk_rings.x86_64, sring_x86_64, PAGE_SIZE * nr_grefs);
+ BACK_RING_INIT(&blkif->blk_rings.x86_64, sring_x86_64,
+ XEN_PAGE_SIZE * nr_grefs);
break;
}
default:
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Julien Grall <julien.grall@citrix.com> |
|---|---|
| Date | 2015-08-07 19:20 +0200 |
| Subject | [PATCH v3 20/20] arm/xen: Add support for 64KB page granularity |
| Message-ID | <pUTtg-61C-29@gated-at.bofh.it> |
| In reply to | #1202904 |
The hypercall interface is always using 4KB page granularity. This is
requiring to use xen page definition macro when we deal with hypercall.
Note that pfn_to_gfn is working with a Xen pfn (i.e 4KB). We may want to
rename pfn_gfn to make this explicit.
We also allocate a 64KB page for the shared page even though only the
first 4KB is used. I don't think this is really important for now as it
helps to have the pointer 4KB aligned (XENMEM_add_to_physmap is taking a
Xen PFN).
Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: Russell King <linux@arm.linux.org.uk>
Stefano, I've dropped your reviewed-by given I've updated the doc and do
changes to avoid usage of XEN_PAGE_SHIFT
Changes in v3:
- s/MFN/GFN/ base on the new naming
- Use virt_to_gfn to avoid use XEN_PAGE_SHIFT
- Drop Stefano's reviewed-by
- Add some docs in arch/arm/asm/xen/page.h
Changes in v2
- Add Stefano's reviewed-by
---
arch/arm/include/asm/xen/page.h | 15 +++++++++++++--
arch/arm/xen/enlighten.c | 6 +++---
2 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/arch/arm/include/asm/xen/page.h b/arch/arm/include/asm/xen/page.h
index 98c9fc3..e3d94cf 100644
--- a/arch/arm/include/asm/xen/page.h
+++ b/arch/arm/include/asm/xen/page.h
@@ -28,6 +28,17 @@ typedef struct xpaddr {
#define INVALID_P2M_ENTRY (~0UL)
+/*
+ * The pseudo-physical frame (pfn) used in all the helpers is always based
+ * on Xen page granularity (i.e 4KB).
+ *
+ * A Linux page may be split across multiple non-contiguous Xen page so we
+ * have to keep track with frame based on 4KB page granularity.
+ *
+ * PV drivers should never make a direct usage of those helpers (particularly
+ * pfn_to_gfn and gfn_to_pfn).
+ */
+
unsigned long __pfn_to_mfn(unsigned long pfn);
extern struct rb_root phys_to_mach;
@@ -64,8 +75,8 @@ static inline unsigned long bfn_to_pfn(unsigned long bfn)
#define bfn_to_local_pfn(bfn) bfn_to_pfn(bfn)
/* VIRT <-> GUEST conversion */
-#define virt_to_gfn(v) (pfn_to_gfn(virt_to_pfn(v)))
-#define gfn_to_virt(m) (__va(gfn_to_pfn(m) << PAGE_SHIFT))
+#define virt_to_gfn(v) (pfn_to_gfn(virt_to_phys(v) >> XEN_PAGE_SHIFT))
+#define gfn_to_virt(m) (__va(gfn_to_pfn(m) << XEN_PAGE_SHIFT))
/* Only used in PV code. But ARM guests are always HVM. */
static inline xmaddr_t arbitrary_virt_to_machine(void *vaddr)
diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index eeeab07..50b4769 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -89,8 +89,8 @@ static void xen_percpu_init(void)
pr_info("Xen: initializing cpu%d\n", cpu);
vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
- info.mfn = __pa(vcpup) >> PAGE_SHIFT;
- info.offset = offset_in_page(vcpup);
+ info.mfn = virt_to_gfn(vcpup);
+ info.offset = xen_offset_in_page(vcpup);
err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info, cpu, &info);
BUG_ON(err);
@@ -213,7 +213,7 @@ static int __init xen_guest_init(void)
xatp.domid = DOMID_SELF;
xatp.idx = 0;
xatp.space = XENMAPSPACE_shared_info;
- xatp.gpfn = __pa(shared_info_page) >> PAGE_SHIFT;
+ xatp.gpfn = virt_to_gfn(shared_info_page);
if (HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp))
BUG();
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Stefano Stabellini <stefano.stabellini@eu.citrix.com> |
|---|---|
| Date | 2015-08-10 15:00 +0200 |
| Subject | Re: [PATCH v3 20/20] arm/xen: Add support for 64KB page granularity |
| Message-ID | <pVUQj-6Mt-25@gated-at.bofh.it> |
| In reply to | #1202950 |
On Fri, 7 Aug 2015, Julien Grall wrote:
> The hypercall interface is always using 4KB page granularity. This is
> requiring to use xen page definition macro when we deal with hypercall.
>
> Note that pfn_to_gfn is working with a Xen pfn (i.e 4KB). We may want to
> rename pfn_gfn to make this explicit.
>
> We also allocate a 64KB page for the shared page even though only the
> first 4KB is used. I don't think this is really important for now as it
> helps to have the pointer 4KB aligned (XENMEM_add_to_physmap is taking a
> Xen PFN).
>
> Signed-off-by: Julien Grall <julien.grall@citrix.com>
Reviewed-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
> Cc: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> Cc: Russell King <linux@arm.linux.org.uk>
>
> Stefano, I've dropped your reviewed-by given I've updated the doc and do
> changes to avoid usage of XEN_PAGE_SHIFT
>
> Changes in v3:
> - s/MFN/GFN/ base on the new naming
> - Use virt_to_gfn to avoid use XEN_PAGE_SHIFT
> - Drop Stefano's reviewed-by
> - Add some docs in arch/arm/asm/xen/page.h
>
> Changes in v2
> - Add Stefano's reviewed-by
> ---
> arch/arm/include/asm/xen/page.h | 15 +++++++++++++--
> arch/arm/xen/enlighten.c | 6 +++---
> 2 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/arch/arm/include/asm/xen/page.h b/arch/arm/include/asm/xen/page.h
> index 98c9fc3..e3d94cf 100644
> --- a/arch/arm/include/asm/xen/page.h
> +++ b/arch/arm/include/asm/xen/page.h
> @@ -28,6 +28,17 @@ typedef struct xpaddr {
>
> #define INVALID_P2M_ENTRY (~0UL)
>
> +/*
> + * The pseudo-physical frame (pfn) used in all the helpers is always based
> + * on Xen page granularity (i.e 4KB).
> + *
> + * A Linux page may be split across multiple non-contiguous Xen page so we
> + * have to keep track with frame based on 4KB page granularity.
> + *
> + * PV drivers should never make a direct usage of those helpers (particularly
> + * pfn_to_gfn and gfn_to_pfn).
> + */
> +
> unsigned long __pfn_to_mfn(unsigned long pfn);
> extern struct rb_root phys_to_mach;
>
> @@ -64,8 +75,8 @@ static inline unsigned long bfn_to_pfn(unsigned long bfn)
> #define bfn_to_local_pfn(bfn) bfn_to_pfn(bfn)
>
> /* VIRT <-> GUEST conversion */
> -#define virt_to_gfn(v) (pfn_to_gfn(virt_to_pfn(v)))
> -#define gfn_to_virt(m) (__va(gfn_to_pfn(m) << PAGE_SHIFT))
> +#define virt_to_gfn(v) (pfn_to_gfn(virt_to_phys(v) >> XEN_PAGE_SHIFT))
> +#define gfn_to_virt(m) (__va(gfn_to_pfn(m) << XEN_PAGE_SHIFT))
>
> /* Only used in PV code. But ARM guests are always HVM. */
> static inline xmaddr_t arbitrary_virt_to_machine(void *vaddr)
> diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
> index eeeab07..50b4769 100644
> --- a/arch/arm/xen/enlighten.c
> +++ b/arch/arm/xen/enlighten.c
> @@ -89,8 +89,8 @@ static void xen_percpu_init(void)
> pr_info("Xen: initializing cpu%d\n", cpu);
> vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
>
> - info.mfn = __pa(vcpup) >> PAGE_SHIFT;
> - info.offset = offset_in_page(vcpup);
> + info.mfn = virt_to_gfn(vcpup);
> + info.offset = xen_offset_in_page(vcpup);
>
> err = HYPERVISOR_vcpu_op(VCPUOP_register_vcpu_info, cpu, &info);
> BUG_ON(err);
> @@ -213,7 +213,7 @@ static int __init xen_guest_init(void)
> xatp.domid = DOMID_SELF;
> xatp.idx = 0;
> xatp.space = XENMAPSPACE_shared_info;
> - xatp.gpfn = __pa(shared_info_page) >> PAGE_SHIFT;
> + xatp.gpfn = virt_to_gfn(shared_info_page);
> if (HYPERVISOR_memory_op(XENMEM_add_to_physmap, &xatp))
> BUG();
>
> --
> 2.1.4
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web