# HG changeset patch # User Tim Deegan <tim@xen.org> # Date 1360858577 0 # Node ID 68308aac7872c07631cb8424367907a11811ec3d # Parent 5a84cc531072378e6e5ff89b4c0e9a35000dc56f xen/xenoprof: avoid division by 0. Signed-off-by: Tim Deegan <tim@xen.org> diff -r 5a84cc531072 -r 68308aac7872 xen/common/xenoprof.c --- a/xen/common/xenoprof.c Thu Feb 14 15:46:56 2013 +0000 +++ b/xen/common/xenoprof.c Thu Feb 14 16:16:17 2013 +0000 @@ -225,7 +225,7 @@ static int alloc_xenoprof_struct( #endif /* reduce max_samples if necessary to limit pages allocated */ - max_bufsize = (MAX_OPROF_SHARED_PAGES * PAGE_SIZE) / nvcpu; + max_bufsize = (MAX_OPROF_SHARED_PAGES * PAGE_SIZE) / (nvcpu ?: 1); max_max_samples = ( (max_bufsize - bufsize) / i ) + 1; if ( (unsigned)max_samples > max_max_samples ) max_samples = max_max_samples;
>>> On 14.02.13 at 17:16, Tim Deegan <tim@xen.org> wrote: > --- a/xen/common/xenoprof.c Thu Feb 14 15:46:56 2013 +0000 > +++ b/xen/common/xenoprof.c Thu Feb 14 16:16:17 2013 +0000 > @@ -225,7 +225,7 @@ static int alloc_xenoprof_struct( > #endif > > /* reduce max_samples if necessary to limit pages allocated */ > - max_bufsize = (MAX_OPROF_SHARED_PAGES * PAGE_SIZE) / nvcpu; > + max_bufsize = (MAX_OPROF_SHARED_PAGES * PAGE_SIZE) / (nvcpu ?: 1); > max_max_samples = ( (max_bufsize - bufsize) / i ) + 1; > if ( (unsigned)max_samples > max_max_samples ) > max_samples = max_max_samples;I think the function would better return an error in that case. After all there''s little point in setting up anything when we for sure don''t know how many vCPU-s a domain is going to have. Jan
At 16:34 +0000 on 14 Feb (1360859678), Jan Beulich wrote:> >>> On 14.02.13 at 17:16, Tim Deegan <tim@xen.org> wrote: > > --- a/xen/common/xenoprof.c Thu Feb 14 15:46:56 2013 +0000 > > +++ b/xen/common/xenoprof.c Thu Feb 14 16:16:17 2013 +0000 > > @@ -225,7 +225,7 @@ static int alloc_xenoprof_struct( > > #endif > > > > /* reduce max_samples if necessary to limit pages allocated */ > > - max_bufsize = (MAX_OPROF_SHARED_PAGES * PAGE_SIZE) / nvcpu; > > + max_bufsize = (MAX_OPROF_SHARED_PAGES * PAGE_SIZE) / (nvcpu ?: 1); > > max_max_samples = ( (max_bufsize - bufsize) / i ) + 1; > > if ( (unsigned)max_samples > max_max_samples ) > > max_samples = max_max_samples; > > I think the function would better return an error in that case. After > all there''s little point in setting up anything when we for sure don''t > know how many vCPU-s a domain is going to have.Grand so: # HG changeset patch # Parent 5a84cc531072378e6e5ff89b4c0e9a35000dc56f xen/xenoprof: avoid division by 0. Signed-off-by: Tim Deegan <tim@xen.org> diff -r 5a84cc531072 xen/common/xenoprof.c --- a/xen/common/xenoprof.c Thu Feb 14 15:46:56 2013 +0000 +++ b/xen/common/xenoprof.c Thu Feb 14 16:48:49 2013 +0000 @@ -213,6 +213,9 @@ static int alloc_xenoprof_struct( for_each_vcpu ( d, v ) nvcpu++; + if ( !nvcpu ) + return -EINVAL; + bufsize = sizeof(struct xenoprof_buf); i = sizeof(struct event_log); #ifdef CONFIG_COMPAT
>>> On 14.02.13 at 17:50, Tim Deegan <tim@xen.org> wrote: > At 16:34 +0000 on 14 Feb (1360859678), Jan Beulich wrote: >> >>> On 14.02.13 at 17:16, Tim Deegan <tim@xen.org> wrote: >> > --- a/xen/common/xenoprof.c Thu Feb 14 15:46:56 2013 +0000 >> > +++ b/xen/common/xenoprof.c Thu Feb 14 16:16:17 2013 +0000 >> > @@ -225,7 +225,7 @@ static int alloc_xenoprof_struct( >> > #endif >> > >> > /* reduce max_samples if necessary to limit pages allocated */ >> > - max_bufsize = (MAX_OPROF_SHARED_PAGES * PAGE_SIZE) / nvcpu; >> > + max_bufsize = (MAX_OPROF_SHARED_PAGES * PAGE_SIZE) / (nvcpu ?: 1); >> > max_max_samples = ( (max_bufsize - bufsize) / i ) + 1; >> > if ( (unsigned)max_samples > max_max_samples ) >> > max_samples = max_max_samples; >> >> I think the function would better return an error in that case. After >> all there''s little point in setting up anything when we for sure don''t >> know how many vCPU-s a domain is going to have. > > Grand so: > > # HG changeset patch > # Parent 5a84cc531072378e6e5ff89b4c0e9a35000dc56f > xen/xenoprof: avoid division by 0. > > Signed-off-by: Tim Deegan <tim@xen.org> > > diff -r 5a84cc531072 xen/common/xenoprof.c > --- a/xen/common/xenoprof.c Thu Feb 14 15:46:56 2013 +0000 > +++ b/xen/common/xenoprof.c Thu Feb 14 16:48:49 2013 +0000 > @@ -213,6 +213,9 @@ static int alloc_xenoprof_struct( > for_each_vcpu ( d, v ) > nvcpu++; > > + if ( !nvcpu ) > + return -EINVAL;Missing some cleanup here? Or move the preceding loop and this addition to the top of the function? Jan> + > bufsize = sizeof(struct xenoprof_buf); > i = sizeof(struct event_log); > #ifdef CONFIG_COMPAT
At 17:05 +0000 on 14 Feb (1360861530), Jan Beulich wrote:> >>> On 14.02.13 at 17:50, Tim Deegan <tim@xen.org> wrote: > > At 16:34 +0000 on 14 Feb (1360859678), Jan Beulich wrote: > >> >>> On 14.02.13 at 17:16, Tim Deegan <tim@xen.org> wrote: > >> > --- a/xen/common/xenoprof.c Thu Feb 14 15:46:56 2013 +0000 > >> > +++ b/xen/common/xenoprof.c Thu Feb 14 16:16:17 2013 +0000 > >> > @@ -225,7 +225,7 @@ static int alloc_xenoprof_struct( > >> > #endif > >> > > >> > /* reduce max_samples if necessary to limit pages allocated */ > >> > - max_bufsize = (MAX_OPROF_SHARED_PAGES * PAGE_SIZE) / nvcpu; > >> > + max_bufsize = (MAX_OPROF_SHARED_PAGES * PAGE_SIZE) / (nvcpu ?: 1); > >> > max_max_samples = ( (max_bufsize - bufsize) / i ) + 1; > >> > if ( (unsigned)max_samples > max_max_samples ) > >> > max_samples = max_max_samples; > >> > >> I think the function would better return an error in that case. After > >> all there''s little point in setting up anything when we for sure don''t > >> know how many vCPU-s a domain is going to have. > > > > Grand so: > > > > # HG changeset patch > > # Parent 5a84cc531072378e6e5ff89b4c0e9a35000dc56f > > xen/xenoprof: avoid division by 0. > > > > Signed-off-by: Tim Deegan <tim@xen.org> > > > > diff -r 5a84cc531072 xen/common/xenoprof.c > > --- a/xen/common/xenoprof.c Thu Feb 14 15:46:56 2013 +0000 > > +++ b/xen/common/xenoprof.c Thu Feb 14 16:48:49 2013 +0000 > > @@ -213,6 +213,9 @@ static int alloc_xenoprof_struct( > > for_each_vcpu ( d, v ) > > nvcpu++; > > > > + if ( !nvcpu ) > > + return -EINVAL; > > Missing some cleanup here? Or move the preceding loop and this > addition to the top of the function?D''oh, yes. Trying again: # HG changeset patch # Parent 5a84cc531072378e6e5ff89b4c0e9a35000dc56f xen/xenoprof: avoid division by 0. Signed-off-by: Tim Deegan <tim@xen.org> diff -r 5a84cc531072 xen/common/xenoprof.c --- a/xen/common/xenoprof.c Thu Feb 14 15:46:56 2013 +0000 +++ b/xen/common/xenoprof.c Thu Feb 14 17:07:41 2013 +0000 @@ -193,6 +193,13 @@ static int alloc_xenoprof_struct( unsigned max_max_samples; int i; + nvcpu = 0; + for_each_vcpu ( d, v ) + nvcpu++; + + if ( !nvcpu ) + return -EINVAL; + d->xenoprof = xzalloc(struct xenoprof); if ( d->xenoprof == NULL ) { @@ -209,10 +216,6 @@ static int alloc_xenoprof_struct( return -ENOMEM; } - nvcpu = 0; - for_each_vcpu ( d, v ) - nvcpu++; - bufsize = sizeof(struct xenoprof_buf); i = sizeof(struct event_log); #ifdef CONFIG_COMPAT
>>> On 14.02.13 at 18:10, Tim Deegan <tim@xen.org> wrote: > # HG changeset patch > # Parent 5a84cc531072378e6e5ff89b4c0e9a35000dc56f > xen/xenoprof: avoid division by 0. > > Signed-off-by: Tim Deegan <tim@xen.org>Acked-by: Jan Beulich <jbeulich@suse.com>> diff -r 5a84cc531072 xen/common/xenoprof.c > --- a/xen/common/xenoprof.c Thu Feb 14 15:46:56 2013 +0000 > +++ b/xen/common/xenoprof.c Thu Feb 14 17:07:41 2013 +0000 > @@ -193,6 +193,13 @@ static int alloc_xenoprof_struct( > unsigned max_max_samples; > int i; > > + nvcpu = 0; > + for_each_vcpu ( d, v ) > + nvcpu++; > + > + if ( !nvcpu ) > + return -EINVAL; > + > d->xenoprof = xzalloc(struct xenoprof); > if ( d->xenoprof == NULL ) > { > @@ -209,10 +216,6 @@ static int alloc_xenoprof_struct( > return -ENOMEM; > } > > - nvcpu = 0; > - for_each_vcpu ( d, v ) > - nvcpu++; > - > bufsize = sizeof(struct xenoprof_buf); > i = sizeof(struct event_log); > #ifdef CONFIG_COMPAT
On 15/02/2013 08:10, "Jan Beulich" <JBeulich@suse.com> wrote:>>>> On 14.02.13 at 18:10, Tim Deegan <tim@xen.org> wrote: >> # HG changeset patch >> # Parent 5a84cc531072378e6e5ff89b4c0e9a35000dc56f >> xen/xenoprof: avoid division by 0. >> >> Signed-off-by: Tim Deegan <tim@xen.org> > > Acked-by: Jan Beulich <jbeulich@suse.com>Acked-by: Keir Fraser <keir@xen.org>>> diff -r 5a84cc531072 xen/common/xenoprof.c >> --- a/xen/common/xenoprof.c Thu Feb 14 15:46:56 2013 +0000 >> +++ b/xen/common/xenoprof.c Thu Feb 14 17:07:41 2013 +0000 >> @@ -193,6 +193,13 @@ static int alloc_xenoprof_struct( >> unsigned max_max_samples; >> int i; >> >> + nvcpu = 0; >> + for_each_vcpu ( d, v ) >> + nvcpu++; >> + >> + if ( !nvcpu ) >> + return -EINVAL; >> + >> d->xenoprof = xzalloc(struct xenoprof); >> if ( d->xenoprof == NULL ) >> { >> @@ -209,10 +216,6 @@ static int alloc_xenoprof_struct( >> return -ENOMEM; >> } >> >> - nvcpu = 0; >> - for_each_vcpu ( d, v ) >> - nvcpu++; >> - >> bufsize = sizeof(struct xenoprof_buf); >> i = sizeof(struct event_log); >> #ifdef CONFIG_COMPAT > > > > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel