* [PATCH] drm/scheduler: add NULL pointer check for run queue @ 2018-07-16 2:59 Junwei Zhang [not found] ` <1531709980-21298-1-git-send-email-Jerry.Zhang-5C7GfCeVMHo@public.gmane.org> 0 siblings, 1 reply; 5+ messages in thread From: Junwei Zhang @ 2018-07-16 2:59 UTC (permalink / raw) To: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Cc: Junwei Zhang, christian.koenig-5C7GfCeVMHo To check rq pointer before adding entity into it. That avoids NULL pointer access in some case. Suggested-by: Christian König <christian.koenig@amd.com> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com> --- drivers/gpu/drm/scheduler/gpu_scheduler.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c index 16bf446..5e5268d 100644 --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c @@ -91,6 +91,10 @@ static void drm_sched_rq_add_entity(struct drm_sched_rq *rq, { if (!list_empty(&entity->list)) return; + if (!rq) { + DRM_ERROR("rq is NULL!\n"); + return; + } spin_lock(&rq->lock); list_add_tail(&entity->list, &rq->entities); spin_unlock(&rq->lock); -- 1.9.1 _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply related [flat|nested] 5+ messages in thread
[parent not found: <1531709980-21298-1-git-send-email-Jerry.Zhang-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH] drm/scheduler: add NULL pointer check for run queue [not found] ` <1531709980-21298-1-git-send-email-Jerry.Zhang-5C7GfCeVMHo@public.gmane.org> @ 2018-07-16 8:31 ` Christian König [not found] ` <045ffcc7-9b9a-2534-9ebe-295e18ce62e7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 0 siblings, 1 reply; 5+ messages in thread From: Christian König @ 2018-07-16 8:31 UTC (permalink / raw) To: Junwei Zhang, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Cc: christian.koenig-5C7GfCeVMHo Am 16.07.2018 um 04:59 schrieb Junwei Zhang: > To check rq pointer before adding entity into it. > That avoids NULL pointer access in some case. > > Suggested-by: Christian König <christian.koenig@amd.com> > Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com> > --- > drivers/gpu/drm/scheduler/gpu_scheduler.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c > index 16bf446..5e5268d 100644 > --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c > +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c > @@ -91,6 +91,10 @@ static void drm_sched_rq_add_entity(struct drm_sched_rq *rq, > { > if (!list_empty(&entity->list)) > return; > + if (!rq) { > + DRM_ERROR("rq is NULL!\n"); > + return; > + } Better put that into drm_sched_entity_push_job(), e.g. something like: /* first job wakes up scheduler */ if (first) { /* Add the entity to the run queue */ spin_lock(&entity->rq_lock); if (!entity->rq) { DRM_ERROR("Trying to push to killed entity!\n"); .... Regards, Christian. > spin_lock(&rq->lock); > list_add_tail(&entity->list, &rq->entities); > spin_unlock(&rq->lock); _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <045ffcc7-9b9a-2534-9ebe-295e18ce62e7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]
* Re: [PATCH] drm/scheduler: add NULL pointer check for run queue [not found] ` <045ffcc7-9b9a-2534-9ebe-295e18ce62e7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> @ 2018-07-16 9:01 ` Zhang, Jerry (Junwei) [not found] ` <5B4C5EFD.60301-5C7GfCeVMHo@public.gmane.org> 0 siblings, 1 reply; 5+ messages in thread From: Zhang, Jerry (Junwei) @ 2018-07-16 9:01 UTC (permalink / raw) To: christian.koenig-5C7GfCeVMHo, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On 07/16/2018 04:31 PM, Christian König wrote: > Am 16.07.2018 um 04:59 schrieb Junwei Zhang: >> To check rq pointer before adding entity into it. >> That avoids NULL pointer access in some case. >> >> Suggested-by: Christian König <christian.koenig@amd.com> >> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com> >> --- >> drivers/gpu/drm/scheduler/gpu_scheduler.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c >> index 16bf446..5e5268d 100644 >> --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c >> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c >> @@ -91,6 +91,10 @@ static void drm_sched_rq_add_entity(struct drm_sched_rq *rq, >> { >> if (!list_empty(&entity->list)) >> return; >> + if (!rq) { >> + DRM_ERROR("rq is NULL!\n"); >> + return; >> + } > > Better put that into drm_sched_entity_push_job(), e.g. something like: Considered that as well. Just be afraid of someone else could call it in another place without rq checking in the future. Regards, Jerry > > /* first job wakes up scheduler */ > if (first) { > /* Add the entity to the run queue */ > spin_lock(&entity->rq_lock); > if (!entity->rq) { > DRM_ERROR("Trying to push to killed entity!\n"); > .... > > Regards, > Christian. > >> spin_lock(&rq->lock); >> list_add_tail(&entity->list, &rq->entities); >> spin_unlock(&rq->lock); > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <5B4C5EFD.60301-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH] drm/scheduler: add NULL pointer check for run queue [not found] ` <5B4C5EFD.60301-5C7GfCeVMHo@public.gmane.org> @ 2018-07-16 9:11 ` Christian König [not found] ` <e2dc7604-e706-c646-bf98-c058d847695e-5C7GfCeVMHo@public.gmane.org> 0 siblings, 1 reply; 5+ messages in thread From: Christian König @ 2018-07-16 9:11 UTC (permalink / raw) To: Zhang, Jerry (Junwei), amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW Am 16.07.2018 um 11:01 schrieb Zhang, Jerry (Junwei): > On 07/16/2018 04:31 PM, Christian König wrote: >> Am 16.07.2018 um 04:59 schrieb Junwei Zhang: >>> To check rq pointer before adding entity into it. >>> That avoids NULL pointer access in some case. >>> >>> Suggested-by: Christian König <christian.koenig@amd.com> >>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com> >>> --- >>> drivers/gpu/drm/scheduler/gpu_scheduler.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c >>> b/drivers/gpu/drm/scheduler/gpu_scheduler.c >>> index 16bf446..5e5268d 100644 >>> --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c >>> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c >>> @@ -91,6 +91,10 @@ static void drm_sched_rq_add_entity(struct >>> drm_sched_rq *rq, >>> { >>> if (!list_empty(&entity->list)) >>> return; >>> + if (!rq) { >>> + DRM_ERROR("rq is NULL!\n"); >>> + return; >>> + } >> >> Better put that into drm_sched_entity_push_job(), e.g. something like: > > Considered that as well. > Just be afraid of someone else could call it in another place without > rq checking in the future. Well that's exactly the reason why I wanted to have the check in drm_sched_rq_add_entity(). Calling drm_sched_rq_add_entity() will a NULL rq is illegal and that should be avoided in the caller instead of more or less silently dropped in the function. Regards, Christian. > > Regards, > Jerry > >> >> /* first job wakes up scheduler */ >> if (first) { >> /* Add the entity to the run queue */ >> spin_lock(&entity->rq_lock); >> if (!entity->rq) { >> DRM_ERROR("Trying to push to killed entity!\n"); >> .... >> >> Regards, >> Christian. >> >>> spin_lock(&rq->lock); >>> list_add_tail(&entity->list, &rq->entities); >>> spin_unlock(&rq->lock); >> _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <e2dc7604-e706-c646-bf98-c058d847695e-5C7GfCeVMHo@public.gmane.org>]
* Re: [PATCH] drm/scheduler: add NULL pointer check for run queue [not found] ` <e2dc7604-e706-c646-bf98-c058d847695e-5C7GfCeVMHo@public.gmane.org> @ 2018-07-16 9:17 ` Zhang, Jerry (Junwei) 0 siblings, 0 replies; 5+ messages in thread From: Zhang, Jerry (Junwei) @ 2018-07-16 9:17 UTC (permalink / raw) To: Christian König, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW On 07/16/2018 05:11 PM, Christian König wrote: > Am 16.07.2018 um 11:01 schrieb Zhang, Jerry (Junwei): >> On 07/16/2018 04:31 PM, Christian König wrote: >>> Am 16.07.2018 um 04:59 schrieb Junwei Zhang: >>>> To check rq pointer before adding entity into it. >>>> That avoids NULL pointer access in some case. >>>> >>>> Suggested-by: Christian König <christian.koenig@amd.com> >>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com> >>>> --- >>>> drivers/gpu/drm/scheduler/gpu_scheduler.c | 4 ++++ >>>> 1 file changed, 4 insertions(+) >>>> >>>> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c >>>> index 16bf446..5e5268d 100644 >>>> --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c >>>> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c >>>> @@ -91,6 +91,10 @@ static void drm_sched_rq_add_entity(struct drm_sched_rq *rq, >>>> { >>>> if (!list_empty(&entity->list)) >>>> return; >>>> + if (!rq) { >>>> + DRM_ERROR("rq is NULL!\n"); >>>> + return; >>>> + } >>> >>> Better put that into drm_sched_entity_push_job(), e.g. something like: >> >> Considered that as well. >> Just be afraid of someone else could call it in another place without rq checking in the future. > > Well that's exactly the reason why I wanted to have the check in drm_sched_rq_add_entity(). > > Calling drm_sched_rq_add_entity() will a NULL rq is illegal and that should be avoided in the caller instead of more or less silently dropped in the function. Fine, to expose the error in place explicitly. Regards, Jerry > > Regards, > Christian. > >> >> Regards, >> Jerry >> >>> >>> /* first job wakes up scheduler */ >>> if (first) { >>> /* Add the entity to the run queue */ >>> spin_lock(&entity->rq_lock); >>> if (!entity->rq) { >>> DRM_ERROR("Trying to push to killed entity!\n"); >>> .... >>> >>> Regards, >>> Christian. >>> >>>> spin_lock(&rq->lock); >>>> list_add_tail(&entity->list, &rq->entities); >>>> spin_unlock(&rq->lock); >>> > > _______________________________________________ > amd-gfx mailing list > amd-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/amd-gfx _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-07-16 9:17 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-07-16 2:59 [PATCH] drm/scheduler: add NULL pointer check for run queue Junwei Zhang [not found] ` <1531709980-21298-1-git-send-email-Jerry.Zhang-5C7GfCeVMHo@public.gmane.org> 2018-07-16 8:31 ` Christian König [not found] ` <045ffcc7-9b9a-2534-9ebe-295e18ce62e7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> 2018-07-16 9:01 ` Zhang, Jerry (Junwei) [not found] ` <5B4C5EFD.60301-5C7GfCeVMHo@public.gmane.org> 2018-07-16 9:11 ` Christian König [not found] ` <e2dc7604-e706-c646-bf98-c058d847695e-5C7GfCeVMHo@public.gmane.org> 2018-07-16 9:17 ` Zhang, Jerry (Junwei)
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.