| Message ID | 4FC770E20200007800087298@nat28.tlf.novell.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 31 May 2012, Jan Beulich wrote: > Legacy (non-pvops) gntdev drivers may require this to be done when the > number of grants intended to be used simultaneously exceeds a certain > driver specific default limit. > > Change in v2: Double the number requested, as we need to account for > the allocations needing to happen in contiguous chunks. The worst case > number would be max_req * max_seg + (max_req - 1) * (max_seg - 1) + 1, > but in order to keep things simple just use 2 * max_req * max_seg. > > Change in v3: introduce MAX_GRANTS(), and add a comment explaining its > definition. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> I think it is OK, I'll submit it as a bug fix for QEMU 1.1. It should be applied to qemu-xen-traditional too. > --- a/hw/xen_disk.c > +++ b/hw/xen_disk.c > @@ -537,6 +537,15 @@ static void blk_bh(void *opaque) > blk_handle_requests(blkdev); > } > > +/* > + * We need to account for the grant allocations requiring contiguous > + * chunks; the worst case number would be > + * max_req * max_seg + (max_req - 1) * (max_seg - 1) + 1, > + * but in order to keep things simple just use > + * 2 * max_req * max_seg. > + */ > +#define MAX_GRANTS(max_req, max_seg) (2 * (max_req) * (max_seg)) > + > static void blk_alloc(struct XenDevice *xendev) > { > struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev); > @@ -548,6 +557,11 @@ static void blk_alloc(struct XenDevice * > if (xen_mode != XEN_EMULATE) { > batch_maps = 1; > } > + if (xc_gnttab_set_max_grants(xendev->gnttabdev, > + MAX_GRANTS(max_requests, BLKIF_MAX_SEGMENTS_PER_REQUEST)) < 0) { > + xen_be_printf(xendev, 0, "xc_gnttab_set_max_grants failed: %s\n", > + strerror(errno)); > + } > } > > static int blk_init(struct XenDevice *xendev) > > > >
>>> On 31.05.12 at 13:27, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > On Thu, 31 May 2012, Jan Beulich wrote: >> Legacy (non-pvops) gntdev drivers may require this to be done when the >> number of grants intended to be used simultaneously exceeds a certain >> driver specific default limit. >> >> Change in v2: Double the number requested, as we need to account for >> the allocations needing to happen in contiguous chunks. The worst case >> number would be max_req * max_seg + (max_req - 1) * (max_seg - 1) + 1, >> but in order to keep things simple just use 2 * max_req * max_seg. >> >> Change in v3: introduce MAX_GRANTS(), and add a comment explaining its >> definition. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > I think it is OK, I'll submit it as a bug fix for QEMU 1.1. Thanks! > It should be applied to qemu-xen-traditional too. And the other one ("qemu/xendisk: properly update stats in ioreq_release()") probably too. Jan
On Thu, 31 May 2012, Jan Beulich wrote: > >>> On 31.05.12 at 13:27, Stefano Stabellini <stefano.stabellini@eu.citrix.com> > wrote: > > On Thu, 31 May 2012, Jan Beulich wrote: > >> Legacy (non-pvops) gntdev drivers may require this to be done when the > >> number of grants intended to be used simultaneously exceeds a certain > >> driver specific default limit. > >> > >> Change in v2: Double the number requested, as we need to account for > >> the allocations needing to happen in contiguous chunks. The worst case > >> number would be max_req * max_seg + (max_req - 1) * (max_seg - 1) + 1, > >> but in order to keep things simple just use 2 * max_req * max_seg. > >> > >> Change in v3: introduce MAX_GRANTS(), and add a comment explaining its > >> definition. > >> > >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > > > I think it is OK, I'll submit it as a bug fix for QEMU 1.1. > > Thanks! > > > It should be applied to qemu-xen-traditional too. > > And the other one ("qemu/xendisk: properly update stats in > ioreq_release()") probably too. Right. That patch is already in upstream QEMU and qemu-xen-upstream but it is missing in qemu-xen-traditional.
Stefano Stabellini writes ("Re: [Xen-devel] [PATCH, v3] qemu/xendisk: set maximum number of grants to be used"):
> On Thu, 31 May 2012, Jan Beulich wrote:
> > > It should be applied to qemu-xen-traditional too.
> >
> > And the other one ("qemu/xendisk: properly update stats in
> > ioreq_release()") probably too.
>
> Right.
> That patch is already in upstream QEMU and qemu-xen-upstream but it is
> missing in qemu-xen-traditional.
Can you tell me the commit id in qemu-xen-upstream so I can put it in
the commit message in qemu-xen-traditional ?
Thanks,
Ian.
On Fri, 8 Jun 2012, Ian Jackson wrote: > Stefano Stabellini writes ("Re: [Xen-devel] [PATCH, v3] qemu/xendisk: set maximum number of grants to be used"): > > On Thu, 31 May 2012, Jan Beulich wrote: > > > > It should be applied to qemu-xen-traditional too. > > > > > > And the other one ("qemu/xendisk: properly update stats in > > > ioreq_release()") probably too. > > > > Right. > > That patch is already in upstream QEMU and qemu-xen-upstream but it is > > missing in qemu-xen-traditional. > > Can you tell me the commit id in qemu-xen-upstream so I can put it in > the commit message in qemu-xen-traditional ? I'll let you know when I have one.
On Mon, 11 Jun 2012, Stefano Stabellini wrote: > On Fri, 8 Jun 2012, Ian Jackson wrote: > > Stefano Stabellini writes ("Re: [Xen-devel] [PATCH, v3] qemu/xendisk: set maximum number of grants to be used"): > > > On Thu, 31 May 2012, Jan Beulich wrote: > > > > > It should be applied to qemu-xen-traditional too. > > > > > > > > And the other one ("qemu/xendisk: properly update stats in > > > > ioreq_release()") probably too. > > > > > > Right. > > > That patch is already in upstream QEMU and qemu-xen-upstream but it is > > > missing in qemu-xen-traditional. > > > > Can you tell me the commit id in qemu-xen-upstream so I can put it in > > the commit message in qemu-xen-traditional ? > > I'll let you know when I have one. Reading more carefully, you are actually talking about a different patch from the one in the subject line. Here is what you are looking for: commit ed5477664369c1e9de23b0e7e8f16a418573bd2a Author: Jan Beulich <JBeulich@suse.com> Date: Mon May 14 16:46:33 2012 +0000 xen_disk: properly update stats in ioreq_release() While for the "normal" case (called from blk_send_response_all()) decrementing requests_finished is correct, doing so in the parse error case is wrong; requests_inflight needs to be decremented instead. Signed-off-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> Reviewed-by: Kevin Wolf <kwolf@redhat.com>
>>> On 31.05.12 at 13:27, Stefano Stabellini <stefano.stabellini@eu.citrix.com> wrote: > On Thu, 31 May 2012, Jan Beulich wrote: >> Legacy (non-pvops) gntdev drivers may require this to be done when the >> number of grants intended to be used simultaneously exceeds a certain >> driver specific default limit. >> >> Change in v2: Double the number requested, as we need to account for >> the allocations needing to happen in contiguous chunks. The worst case >> number would be max_req * max_seg + (max_req - 1) * (max_seg - 1) + 1, >> but in order to keep things simple just use 2 * max_req * max_seg. >> >> Change in v3: introduce MAX_GRANTS(), and add a comment explaining its >> definition. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > I think it is OK, I'll submit it as a bug fix for QEMU 1.1. > > It should be applied to qemu-xen-traditional too. This is commit 64c27e5b1fdb6d94bdc0bda3b1869d7383a35c65 in git.qemu.org/?p=qemu.git. Jan >> --- a/hw/xen_disk.c >> +++ b/hw/xen_disk.c >> @@ -537,6 +537,15 @@ static void blk_bh(void *opaque) >> blk_handle_requests(blkdev); >> } >> >> +/* >> + * We need to account for the grant allocations requiring contiguous >> + * chunks; the worst case number would be >> + * max_req * max_seg + (max_req - 1) * (max_seg - 1) + 1, >> + * but in order to keep things simple just use >> + * 2 * max_req * max_seg. >> + */ >> +#define MAX_GRANTS(max_req, max_seg) (2 * (max_req) * (max_seg)) >> + >> static void blk_alloc(struct XenDevice *xendev) >> { >> struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, > xendev); >> @@ -548,6 +557,11 @@ static void blk_alloc(struct XenDevice * >> if (xen_mode != XEN_EMULATE) { >> batch_maps = 1; >> } >> + if (xc_gnttab_set_max_grants(xendev->gnttabdev, >> + MAX_GRANTS(max_requests, BLKIF_MAX_SEGMENTS_PER_REQUEST)) < 0) { >> + xen_be_printf(xendev, 0, "xc_gnttab_set_max_grants failed: %s\n", >> + strerror(errno)); >> + } >> } >> >> static int blk_init(struct XenDevice *xendev) >> >> >> >>
--- a/hw/xen_disk.c +++ b/hw/xen_disk.c @@ -537,6 +537,15 @@ static void blk_bh(void *opaque) blk_handle_requests(blkdev); } +/* + * We need to account for the grant allocations requiring contiguous + * chunks; the worst case number would be + * max_req * max_seg + (max_req - 1) * (max_seg - 1) + 1, + * but in order to keep things simple just use + * 2 * max_req * max_seg. + */ +#define MAX_GRANTS(max_req, max_seg) (2 * (max_req) * (max_seg)) + static void blk_alloc(struct XenDevice *xendev) { struct XenBlkDev *blkdev = container_of(xendev, struct XenBlkDev, xendev); @@ -548,6 +557,11 @@ static void blk_alloc(struct XenDevice * if (xen_mode != XEN_EMULATE) { batch_maps = 1; } + if (xc_gnttab_set_max_grants(xendev->gnttabdev, + MAX_GRANTS(max_requests, BLKIF_MAX_SEGMENTS_PER_REQUEST)) < 0) { + xen_be_printf(xendev, 0, "xc_gnttab_set_max_grants failed: %s\n", + strerror(errno)); + } } static int blk_init(struct XenDevice *xendev)