public inbox for kernel-janitors@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/etnaviv: fix some off by one bugs
@ 2018-07-13 15:00 Dan Carpenter
  2018-12-21 20:24 ` Dan Carpenter
  2018-12-28 22:08 ` Christian Gmeiner
  0 siblings, 2 replies; 6+ messages in thread
From: Dan Carpenter @ 2018-07-13 15:00 UTC (permalink / raw)
  To: Lucas Stach, Christian Gmeiner
  Cc: David Airlie, kernel-janitors, etnaviv, dri-devel, Russell King

The ->nr_signal is the supposed to be the number of elements in the
->signal array.  There was one place where it was 5 but it was supposed
to be 4.  That looks like a copy and paste bug.  There were also two
checks that were off by one.

Fixes: 9e2c2e273012 ("drm/etnaviv: add infrastructure to query perf counter")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
Not tested.

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
index 9980d81a26e3..4227a4006c34 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
@@ -113,7 +113,7 @@ static const struct etnaviv_pm_domain doms_3d[] = {
 		.name = "PE",
 		.profile_read = VIVS_MC_PROFILE_PE_READ,
 		.profile_config = VIVS_MC_PROFILE_CONFIG0,
-		.nr_signals = 5,
+		.nr_signals = 4,
 		.signal = (const struct etnaviv_pm_signal[]) {
 			{
 				"PIXEL_COUNT_KILLED_BY_COLOR_PIPE",
@@ -435,7 +435,7 @@ int etnaviv_pm_query_sig(struct etnaviv_gpu *gpu,
 
 	dom = meta->domains + signal->domain;
 
-	if (signal->iter > dom->nr_signals)
+	if (signal->iter >= dom->nr_signals)
 		return -EINVAL;
 
 	sig = &dom->signal[signal->iter];
@@ -461,7 +461,7 @@ int etnaviv_pm_req_validate(const struct drm_etnaviv_gem_submit_pmr *r,
 
 	dom = meta->domains + r->domain;
 
-	if (r->signal > dom->nr_signals)
+	if (r->signal >= dom->nr_signals)
 		return -EINVAL;
 
 	return 0;

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/etnaviv: fix some off by one bugs
  2018-07-13 15:00 [PATCH] drm/etnaviv: fix some off by one bugs Dan Carpenter
@ 2018-12-21 20:24 ` Dan Carpenter
  2018-12-24  9:32   ` Daniel Vetter
  2018-12-28 22:08 ` Christian Gmeiner
  1 sibling, 1 reply; 6+ messages in thread
From: Dan Carpenter @ 2018-12-21 20:24 UTC (permalink / raw)
  To: Lucas Stach, Christian Gmeiner
  Cc: David Airlie, kernel-janitors, etnaviv, dri-devel, Russell King

I don't think anyone responded to this one?

regards,
dan carpenter

On Fri, Jul 13, 2018 at 06:00:18PM +0300, Dan Carpenter wrote:
> The ->nr_signal is the supposed to be the number of elements in the
> ->signal array.  There was one place where it was 5 but it was supposed
> to be 4.  That looks like a copy and paste bug.  There were also two
> checks that were off by one.
> 
> Fixes: 9e2c2e273012 ("drm/etnaviv: add infrastructure to query perf counter")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> Not tested.
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> index 9980d81a26e3..4227a4006c34 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> @@ -113,7 +113,7 @@ static const struct etnaviv_pm_domain doms_3d[] = {
>  		.name = "PE",
>  		.profile_read = VIVS_MC_PROFILE_PE_READ,
>  		.profile_config = VIVS_MC_PROFILE_CONFIG0,
> -		.nr_signals = 5,
> +		.nr_signals = 4,
>  		.signal = (const struct etnaviv_pm_signal[]) {
>  			{
>  				"PIXEL_COUNT_KILLED_BY_COLOR_PIPE",
> @@ -435,7 +435,7 @@ int etnaviv_pm_query_sig(struct etnaviv_gpu *gpu,
>  
>  	dom = meta->domains + signal->domain;
>  
> -	if (signal->iter > dom->nr_signals)
> +	if (signal->iter >= dom->nr_signals)
>  		return -EINVAL;
>  
>  	sig = &dom->signal[signal->iter];
> @@ -461,7 +461,7 @@ int etnaviv_pm_req_validate(const struct drm_etnaviv_gem_submit_pmr *r,
>  
>  	dom = meta->domains + r->domain;
>  
> -	if (r->signal > dom->nr_signals)
> +	if (r->signal >= dom->nr_signals)
>  		return -EINVAL;
>  
>  	return 0;

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/etnaviv: fix some off by one bugs
  2018-12-21 20:24 ` Dan Carpenter
@ 2018-12-24  9:32   ` Daniel Vetter
  2019-01-07  8:42     ` Lucas Stach
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2018-12-24  9:32 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: David Airlie, kernel-janitors, The etnaviv authors, dri-devel,
	Russell King

On Fri, Dec 21, 2018 at 9:24 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> I don't think anyone responded to this one?

Maybe time to move etnaviv into drm-misc so that there's a notch more
redundancy in maintainers? Lucas, Christian, others?
-Daniel

>
> regards,
> dan carpenter
>
> On Fri, Jul 13, 2018 at 06:00:18PM +0300, Dan Carpenter wrote:
> > The ->nr_signal is the supposed to be the number of elements in the
> > ->signal array.  There was one place where it was 5 but it was supposed
> > to be 4.  That looks like a copy and paste bug.  There were also two
> > checks that were off by one.
> >
> > Fixes: 9e2c2e273012 ("drm/etnaviv: add infrastructure to query perf counter")
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > Not tested.
> >
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > index 9980d81a26e3..4227a4006c34 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > @@ -113,7 +113,7 @@ static const struct etnaviv_pm_domain doms_3d[] = {
> >               .name = "PE",
> >               .profile_read = VIVS_MC_PROFILE_PE_READ,
> >               .profile_config = VIVS_MC_PROFILE_CONFIG0,
> > -             .nr_signals = 5,
> > +             .nr_signals = 4,
> >               .signal = (const struct etnaviv_pm_signal[]) {
> >                       {
> >                               "PIXEL_COUNT_KILLED_BY_COLOR_PIPE",
> > @@ -435,7 +435,7 @@ int etnaviv_pm_query_sig(struct etnaviv_gpu *gpu,
> >
> >       dom = meta->domains + signal->domain;
> >
> > -     if (signal->iter > dom->nr_signals)
> > +     if (signal->iter >= dom->nr_signals)
> >               return -EINVAL;
> >
> >       sig = &dom->signal[signal->iter];
> > @@ -461,7 +461,7 @@ int etnaviv_pm_req_validate(const struct drm_etnaviv_gem_submit_pmr *r,
> >
> >       dom = meta->domains + r->domain;
> >
> > -     if (r->signal > dom->nr_signals)
> > +     if (r->signal >= dom->nr_signals)
> >               return -EINVAL;
> >
> >       return 0;
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/etnaviv: fix some off by one bugs
  2018-07-13 15:00 [PATCH] drm/etnaviv: fix some off by one bugs Dan Carpenter
  2018-12-21 20:24 ` Dan Carpenter
@ 2018-12-28 22:08 ` Christian Gmeiner
  1 sibling, 0 replies; 6+ messages in thread
From: Christian Gmeiner @ 2018-12-28 22:08 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: David Airlie, kernel-janitors, The etnaviv authors,
	DRI mailing list, Russell King

Am Fr., 13. Juli 2018 um 17:00 Uhr schrieb Dan Carpenter
<dan.carpenter@oracle.com>:
>
> The ->nr_signal is the supposed to be the number of elements in the
> ->signal array.  There was one place where it was 5 but it was supposed
> to be 4.  That looks like a copy and paste bug.  There were also two
> checks that were off by one.
>
> Fixes: 9e2c2e273012 ("drm/etnaviv: add infrastructure to query perf counter")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Reviewed-by: Christian Gmeiner <christian.gmeiner@gmail.com>
Tested-by: Christian Gmeiner <christian.gmeiner@gmail.com>

> ---
> Not tested.
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> index 9980d81a26e3..4227a4006c34 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> @@ -113,7 +113,7 @@ static const struct etnaviv_pm_domain doms_3d[] = {
>                 .name = "PE",
>                 .profile_read = VIVS_MC_PROFILE_PE_READ,
>                 .profile_config = VIVS_MC_PROFILE_CONFIG0,
> -               .nr_signals = 5,
> +               .nr_signals = 4,
>                 .signal = (const struct etnaviv_pm_signal[]) {
>                         {
>                                 "PIXEL_COUNT_KILLED_BY_COLOR_PIPE",
> @@ -435,7 +435,7 @@ int etnaviv_pm_query_sig(struct etnaviv_gpu *gpu,
>
>         dom = meta->domains + signal->domain;
>
> -       if (signal->iter > dom->nr_signals)
> +       if (signal->iter >= dom->nr_signals)
>                 return -EINVAL;
>
>         sig = &dom->signal[signal->iter];
> @@ -461,7 +461,7 @@ int etnaviv_pm_req_validate(const struct drm_etnaviv_gem_submit_pmr *r,
>
>         dom = meta->domains + r->domain;
>
> -       if (r->signal > dom->nr_signals)
> +       if (r->signal >= dom->nr_signals)
>                 return -EINVAL;
>
>         return 0;



-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/etnaviv: fix some off by one bugs
  2018-12-24  9:32   ` Daniel Vetter
@ 2019-01-07  8:42     ` Lucas Stach
  2019-01-07 10:00       ` Daniel Vetter
  0 siblings, 1 reply; 6+ messages in thread
From: Lucas Stach @ 2019-01-07  8:42 UTC (permalink / raw)
  To: Daniel Vetter, Dan Carpenter
  Cc: David Airlie, kernel-janitors, The etnaviv authors, dri-devel,
	Russell King

Hi Daniel,

Am Montag, den 24.12.2018, 10:32 +0100 schrieb Daniel Vetter:
> On Fri, Dec 21, 2018 at 9:24 PM Dan Carpenter <dan.carpenter@oracle.c
> om> wrote:
> > 
> > I don't think anyone responded to this one?
> 
> Maybe time to move etnaviv into drm-misc so that there's a notch more
> redundancy in maintainers? Lucas, Christian, others?

Sorry, but no thanks. The current model guarantees that we have at
least some testing of the patches flowing through the etnaviv tree with
realworld use-cases. We certainly don't have the resources to track a
rapidly changing target like drm-misc with our testing.

Regards,
Lucas

> -Daniel
> 
> > 
> > regards,
> > dan carpenter
> > 
> > On Fri, Jul 13, 2018 at 06:00:18PM +0300, Dan Carpenter wrote:
> > > The ->nr_signal is the supposed to be the number of elements in
> > > the
> > > ->signal array.  There was one place where it was 5 but it was
> > > supposed
> > > to be 4.  That looks like a copy and paste bug.  There were also
> > > two
> > > checks that were off by one.
> > > 
> > > Fixes: 9e2c2e273012 ("drm/etnaviv: add infrastructure to query
> > > perf counter")
> > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > ---
> > > Not tested.
> > > 
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > index 9980d81a26e3..4227a4006c34 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > @@ -113,7 +113,7 @@ static const struct etnaviv_pm_domain
> > > doms_3d[] = {
> > >               .name = "PE",
> > >               .profile_read = VIVS_MC_PROFILE_PE_READ,
> > >               .profile_config = VIVS_MC_PROFILE_CONFIG0,
> > > -             .nr_signals = 5,
> > > +             .nr_signals = 4,
> > >               .signal = (const struct etnaviv_pm_signal[]) {
> > >                       {
> > >                               "PIXEL_COUNT_KILLED_BY_COLOR_PIPE",
> > > @@ -435,7 +435,7 @@ int etnaviv_pm_query_sig(struct etnaviv_gpu
> > > *gpu,
> > > 
> > >       dom = meta->domains + signal->domain;
> > > 
> > > -     if (signal->iter > dom->nr_signals)
> > > +     if (signal->iter >= dom->nr_signals)
> > >               return -EINVAL;
> > > 
> > >       sig = &dom->signal[signal->iter];
> > > @@ -461,7 +461,7 @@ int etnaviv_pm_req_validate(const struct
> > > drm_etnaviv_gem_submit_pmr *r,
> > > 
> > >       dom = meta->domains + r->domain;
> > > 
> > > -     if (r->signal > dom->nr_signals)
> > > +     if (r->signal >= dom->nr_signals)
> > >               return -EINVAL;
> > > 
> > >       return 0;
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH] drm/etnaviv: fix some off by one bugs
  2019-01-07  8:42     ` Lucas Stach
@ 2019-01-07 10:00       ` Daniel Vetter
  0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2019-01-07 10:00 UTC (permalink / raw)
  To: Lucas Stach
  Cc: David Airlie, kernel-janitors, The etnaviv authors, dri-devel,
	Russell King, Dan Carpenter

On Mon, Jan 07, 2019 at 09:42:00AM +0100, Lucas Stach wrote:
> Hi Daniel,
> 
> Am Montag, den 24.12.2018, 10:32 +0100 schrieb Daniel Vetter:
> > On Fri, Dec 21, 2018 at 9:24 PM Dan Carpenter <dan.carpenter@oracle.c
> > om> wrote:
> > > 
> > > I don't think anyone responded to this one?
> > 
> > Maybe time to move etnaviv into drm-misc so that there's a notch more
> > redundancy in maintainers? Lucas, Christian, others?
> 
> Sorry, but no thanks. The current model guarantees that we have at
> least some testing of the patches flowing through the etnaviv tree with
> realworld use-cases. We certainly don't have the resources to track a
> rapidly changing target like drm-misc with our testing.

Not following here exactly, whether you do your own tree or reuse some
existing infrastructure, upstream moves as fast as it does no matter how
things work. And you can still do all the usual testing before pushing.

And I just brought this up because the separate tree also doesn't seem to
be entirely perfectly managed either. You wouldn't get me nagging if
the etnaviv pull wouldn't have raised questions :-)
-Daniel

> Regards,
> Lucas
> 
> > -Daniel
> > 
> > > 
> > > regards,
> > > dan carpenter
> > > 
> > > On Fri, Jul 13, 2018 at 06:00:18PM +0300, Dan Carpenter wrote:
> > > > The ->nr_signal is the supposed to be the number of elements in
> > > > the
> > > > ->signal array.  There was one place where it was 5 but it was
> > > > supposed
> > > > to be 4.  That looks like a copy and paste bug.  There were also
> > > > two
> > > > checks that were off by one.
> > > > 
> > > > Fixes: 9e2c2e273012 ("drm/etnaviv: add infrastructure to query
> > > > perf counter")
> > > > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > > > ---
> > > > Not tested.
> > > > 
> > > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > > b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > > index 9980d81a26e3..4227a4006c34 100644
> > > > --- a/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_perfmon.c
> > > > @@ -113,7 +113,7 @@ static const struct etnaviv_pm_domain
> > > > doms_3d[] = {
> > > >               .name = "PE",
> > > >               .profile_read = VIVS_MC_PROFILE_PE_READ,
> > > >               .profile_config = VIVS_MC_PROFILE_CONFIG0,
> > > > -             .nr_signals = 5,
> > > > +             .nr_signals = 4,
> > > >               .signal = (const struct etnaviv_pm_signal[]) {
> > > >                       {
> > > >                               "PIXEL_COUNT_KILLED_BY_COLOR_PIPE",
> > > > @@ -435,7 +435,7 @@ int etnaviv_pm_query_sig(struct etnaviv_gpu
> > > > *gpu,
> > > > 
> > > >       dom = meta->domains + signal->domain;
> > > > 
> > > > -     if (signal->iter > dom->nr_signals)
> > > > +     if (signal->iter >= dom->nr_signals)
> > > >               return -EINVAL;
> > > > 
> > > >       sig = &dom->signal[signal->iter];
> > > > @@ -461,7 +461,7 @@ int etnaviv_pm_req_validate(const struct
> > > > drm_etnaviv_gem_submit_pmr *r,
> > > > 
> > > >       dom = meta->domains + r->domain;
> > > > 
> > > > -     if (r->signal > dom->nr_signals)
> > > > +     if (r->signal >= dom->nr_signals)
> > > >               return -EINVAL;
> > > > 
> > > >       return 0;
> > > 
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> > 
> > 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-01-07 10:00 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-07-13 15:00 [PATCH] drm/etnaviv: fix some off by one bugs Dan Carpenter
2018-12-21 20:24 ` Dan Carpenter
2018-12-24  9:32   ` Daniel Vetter
2019-01-07  8:42     ` Lucas Stach
2019-01-07 10:00       ` Daniel Vetter
2018-12-28 22:08 ` Christian Gmeiner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox