The IDR deletion interface now returns the deleted entry or NULL if it was not present. So we don't have to do the extra work of checking if we have a reference on the drm_gem_object, this can be handled by checking the return value from idr_remove() and the extra locks can be dropped.
Signed-off-by: Aishwarya Pant aishpant@gmail.com --- drivers/gpu/drm/drm_gem.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index c55f338..f62757a 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -282,29 +282,12 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle) { struct drm_gem_object *obj;
- /* This is gross. The idr system doesn't let us try a delete and - * return an error code. It just spews if you fail at deleting. - * So, we have to grab a lock around finding the object and then - * doing the delete on it and dropping the refcount, or the user - * could race us to double-decrement the refcount and cause a - * use-after-free later. Given the frequency of our handle lookups, - * we may want to use ida for number allocation and a hash table - * for the pointers, anyway. - */ spin_lock(&filp->table_lock); - - /* Check if we currently have a reference on the object */ - obj = idr_replace(&filp->object_idr, NULL, handle); - spin_unlock(&filp->table_lock); - if (IS_ERR_OR_NULL(obj)) + obj = idr_remove(&filp->object_idr, handle); + if (!obj) return -EINVAL; - /* Release driver's reference and decrement refcount. */ drm_gem_object_release_handle(handle, obj, filp); - - /* And finally make the handle available for future allocations. */ - spin_lock(&filp->table_lock); - idr_remove(&filp->object_idr, handle); spin_unlock(&filp->table_lock);
return 0;
On Tue, Sep 26, 2017 at 12:17:28AM +0530, Aishwarya Pant wrote:
The IDR deletion interface now returns the deleted entry or NULL if it was not present. So we don't have to do the extra work of checking if we have a reference on the drm_gem_object, this can be handled by checking the return value from idr_remove() and the extra locks can be dropped.
Signed-off-by: Aishwarya Pant aishpant@gmail.com
Haneen is already working on this task, with the patch just merged. Please coordinate with mentors (me or Sean Paul) before starting on something to avoid such clashes in the future.
Note also that some todo items are just ideas, and need confirmation with relevant maintainers first.
Thanks, Daniel
drivers/gpu/drm/drm_gem.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index c55f338..f62757a 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -282,29 +282,12 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle) { struct drm_gem_object *obj;
- /* This is gross. The idr system doesn't let us try a delete and
* return an error code. It just spews if you fail at deleting.* So, we have to grab a lock around finding the object and then* doing the delete on it and dropping the refcount, or the user* could race us to double-decrement the refcount and cause a* use-after-free later. Given the frequency of our handle lookups,* we may want to use ida for number allocation and a hash table* for the pointers, anyway.*/- /* Check if we currently have a reference on the object */
- obj = idr_replace(&filp->object_idr, NULL, handle);
- spin_unlock(&filp->table_lock);
- if (IS_ERR_OR_NULL(obj))
- obj = idr_remove(&filp->object_idr, handle);
- if (!obj) return -EINVAL;
/* Release driver's reference and decrement refcount. */ drm_gem_object_release_handle(handle, obj, filp);
/* And finally make the handle available for future allocations. */
spin_lock(&filp->table_lock);
idr_remove(&filp->object_idr, handle); spin_unlock(&filp->table_lock);
return 0;
-- 2.7.4
-- You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. To post to this group, send email to outreachy-kernel@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170925184728.GA8861%40a.... For more options, visit https://groups.google.com/d/optout.
On Tue, 26 Sep 2017, Daniel Vetter wrote:
On Tue, Sep 26, 2017 at 12:17:28AM +0530, Aishwarya Pant wrote:
The IDR deletion interface now returns the deleted entry or NULL if it was not present. So we don't have to do the extra work of checking if we have a reference on the drm_gem_object, this can be handled by checking the return value from idr_remove() and the extra locks can be dropped.
Signed-off-by: Aishwarya Pant aishpant@gmail.com
Haneen is already working on this task, with the patch just merged. Please coordinate with mentors (me or Sean Paul) before starting on something to avoid such clashes in the future.
Note also that some todo items are just ideas, and need confirmation with relevant maintainers first.
Not sure where the mixup happened, but anyone who starts on a project specific task should note that on the outreachy kernel wiki and before starting on a project specific task one should check that no one is working on it. If someone started some time ago, and the code doesn't seem to have changed, ask the person who claimed the task or the mentor.
thanks,
julia
Thanks, Daniel
drivers/gpu/drm/drm_gem.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index c55f338..f62757a 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -282,29 +282,12 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle) { struct drm_gem_object *obj;
- /* This is gross. The idr system doesn't let us try a delete and
* return an error code. It just spews if you fail at deleting.* So, we have to grab a lock around finding the object and then* doing the delete on it and dropping the refcount, or the user* could race us to double-decrement the refcount and cause a* use-after-free later. Given the frequency of our handle lookups,* we may want to use ida for number allocation and a hash table* for the pointers, anyway.*/- /* Check if we currently have a reference on the object */
- obj = idr_replace(&filp->object_idr, NULL, handle);
- spin_unlock(&filp->table_lock);
- if (IS_ERR_OR_NULL(obj))
- obj = idr_remove(&filp->object_idr, handle);
- if (!obj) return -EINVAL;
/* Release driver's reference and decrement refcount. */ drm_gem_object_release_handle(handle, obj, filp);
/* And finally make the handle available for future allocations. */
spin_lock(&filp->table_lock);
idr_remove(&filp->object_idr, handle); spin_unlock(&filp->table_lock);
return 0;
-- 2.7.4
-- You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. To post to this group, send email to outreachy-kernel@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170925184728.GA8861%40a.... For more options, visit https://groups.google.com/d/optout.
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
-- You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. To post to this group, send email to outreachy-kernel@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170926082047.eqsd2lslpq.... For more options, visit https://groups.google.com/d/optout.
On Tue, Sep 26, 2017 at 10:38 AM, Julia Lawall julia.lawall@lip6.fr wrote:
On Tue, 26 Sep 2017, Daniel Vetter wrote:
On Tue, Sep 26, 2017 at 12:17:28AM +0530, Aishwarya Pant wrote:
The IDR deletion interface now returns the deleted entry or NULL if it was not present. So we don't have to do the extra work of checking if we have a reference on the drm_gem_object, this can be handled by checking the return value from idr_remove() and the extra locks can be dropped.
Signed-off-by: Aishwarya Pant aishpant@gmail.com
Haneen is already working on this task, with the patch just merged. Please coordinate with mentors (me or Sean Paul) before starting on something to avoid such clashes in the future.
Note also that some todo items are just ideas, and need confirmation with relevant maintainers first.
Not sure where the mixup happened, but anyone who starts on a project specific task should note that on the outreachy kernel wiki and before starting on a project specific task one should check that no one is working on it. If someone started some time ago, and the code doesn't seem to have changed, ask the person who claimed the task or the mentor.
Hm, the dri-devel tasks aren't on the wiki, but in the kernel sources. Where should we put the scratch-pad to avoid such conflicts in the future? E.g. the IIO subsytems has it's task on the wiki, where this works much better. -Daniel
thanks,
julia
Thanks, Daniel
drivers/gpu/drm/drm_gem.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index c55f338..f62757a 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -282,29 +282,12 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle) { struct drm_gem_object *obj;
- /* This is gross. The idr system doesn't let us try a delete and
- return an error code. It just spews if you fail at deleting.
- So, we have to grab a lock around finding the object and then
- doing the delete on it and dropping the refcount, or the user
- could race us to double-decrement the refcount and cause a
- use-after-free later. Given the frequency of our handle lookups,
- we may want to use ida for number allocation and a hash table
- for the pointers, anyway.
- */ spin_lock(&filp->table_lock);
- /* Check if we currently have a reference on the object */
- obj = idr_replace(&filp->object_idr, NULL, handle);
- spin_unlock(&filp->table_lock);
- if (IS_ERR_OR_NULL(obj))
- obj = idr_remove(&filp->object_idr, handle);
- if (!obj) return -EINVAL;
/* Release driver's reference and decrement refcount. */ drm_gem_object_release_handle(handle, obj, filp);
/* And finally make the handle available for future allocations. */
spin_lock(&filp->table_lock);
idr_remove(&filp->object_idr, handle); spin_unlock(&filp->table_lock);
return 0;
-- 2.7.4
-- You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. To post to this group, send email to outreachy-kernel@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170925184728.GA8861%40a.... For more options, visit https://groups.google.com/d/optout.
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
-- You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. To post to this group, send email to outreachy-kernel@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170926082047.eqsd2lslpq.... For more options, visit https://groups.google.com/d/optout.
On Tue, Sep 26, 2017 at 10:47 AM, Daniel Vetter daniel@ffwll.ch wrote:
On Tue, Sep 26, 2017 at 10:38 AM, Julia Lawall julia.lawall@lip6.fr wrote:
On Tue, 26 Sep 2017, Daniel Vetter wrote:
On Tue, Sep 26, 2017 at 12:17:28AM +0530, Aishwarya Pant wrote:
The IDR deletion interface now returns the deleted entry or NULL if it was not present. So we don't have to do the extra work of checking if we have a reference on the drm_gem_object, this can be handled by checking the return value from idr_remove() and the extra locks can be dropped.
Signed-off-by: Aishwarya Pant aishpant@gmail.com
Haneen is already working on this task, with the patch just merged. Please coordinate with mentors (me or Sean Paul) before starting on something to avoid such clashes in the future.
Note also that some todo items are just ideas, and need confirmation with relevant maintainers first.
Not sure where the mixup happened, but anyone who starts on a project specific task should note that on the outreachy kernel wiki and before starting on a project specific task one should check that no one is working on it. If someone started some time ago, and the code doesn't seem to have changed, ask the person who claimed the task or the mentor.
Hm, the dri-devel tasks aren't on the wiki, but in the kernel sources. Where should we put the scratch-pad to avoid such conflicts in the future? E.g. the IIO subsytems has it's task on the wiki, where this works much better.
Ok, I found it. Looks like a few signed up for stuff, but without chatting with us first, and ended up picking tasks that need some serious GPU expertise. I.e. maybe suitable for a full internship, definitely not as starter tasks. I'll send out mails. -Daniel
-Daniel
thanks,
julia
Thanks, Daniel
drivers/gpu/drm/drm_gem.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index c55f338..f62757a 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -282,29 +282,12 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle) { struct drm_gem_object *obj;
- /* This is gross. The idr system doesn't let us try a delete and
- return an error code. It just spews if you fail at deleting.
- So, we have to grab a lock around finding the object and then
- doing the delete on it and dropping the refcount, or the user
- could race us to double-decrement the refcount and cause a
- use-after-free later. Given the frequency of our handle lookups,
- we may want to use ida for number allocation and a hash table
- for the pointers, anyway.
- */ spin_lock(&filp->table_lock);
- /* Check if we currently have a reference on the object */
- obj = idr_replace(&filp->object_idr, NULL, handle);
- spin_unlock(&filp->table_lock);
- if (IS_ERR_OR_NULL(obj))
- obj = idr_remove(&filp->object_idr, handle);
- if (!obj) return -EINVAL;
/* Release driver's reference and decrement refcount. */ drm_gem_object_release_handle(handle, obj, filp);
/* And finally make the handle available for future allocations. */
spin_lock(&filp->table_lock);
idr_remove(&filp->object_idr, handle); spin_unlock(&filp->table_lock);
return 0;
-- 2.7.4
-- You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. To post to this group, send email to outreachy-kernel@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170925184728.GA8861%40a.... For more options, visit https://groups.google.com/d/optout.
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
-- You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. To post to this group, send email to outreachy-kernel@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170926082047.eqsd2lslpq.... For more options, visit https://groups.google.com/d/optout.
-- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Tue, 26 Sep 2017, Daniel Vetter wrote:
On Tue, Sep 26, 2017 at 10:47 AM, Daniel Vetter daniel@ffwll.ch wrote:
On Tue, Sep 26, 2017 at 10:38 AM, Julia Lawall julia.lawall@lip6.fr wrote:
On Tue, 26 Sep 2017, Daniel Vetter wrote:
On Tue, Sep 26, 2017 at 12:17:28AM +0530, Aishwarya Pant wrote:
The IDR deletion interface now returns the deleted entry or NULL if it was not present. So we don't have to do the extra work of checking if we have a reference on the drm_gem_object, this can be handled by checking the return value from idr_remove() and the extra locks can be dropped.
Signed-off-by: Aishwarya Pant aishpant@gmail.com
Haneen is already working on this task, with the patch just merged. Please coordinate with mentors (me or Sean Paul) before starting on something to avoid such clashes in the future.
Note also that some todo items are just ideas, and need confirmation with relevant maintainers first.
Not sure where the mixup happened, but anyone who starts on a project specific task should note that on the outreachy kernel wiki and before starting on a project specific task one should check that no one is working on it. If someone started some time ago, and the code doesn't seem to have changed, ask the person who claimed the task or the mentor.
Hm, the dri-devel tasks aren't on the wiki, but in the kernel sources. Where should we put the scratch-pad to avoid such conflicts in the future? E.g. the IIO subsytems has it's task on the wiki, where this works much better.
Ok, I found it. Looks like a few signed up for stuff, but without chatting with us first, and ended up picking tasks that need some serious GPU expertise. I.e. maybe suitable for a full internship, definitely not as starter tasks. I'll send out mails.
Thanks. Everyone, please discuss with the mentor before picking a task.
julia
-Daniel
-Daniel
thanks,
julia
Thanks, Daniel
drivers/gpu/drm/drm_gem.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index c55f338..f62757a 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -282,29 +282,12 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle) { struct drm_gem_object *obj;
- /* This is gross. The idr system doesn't let us try a delete and
- return an error code. It just spews if you fail at deleting.
- So, we have to grab a lock around finding the object and then
- doing the delete on it and dropping the refcount, or the user
- could race us to double-decrement the refcount and cause a
- use-after-free later. Given the frequency of our handle lookups,
- we may want to use ida for number allocation and a hash table
- for the pointers, anyway.
- */ spin_lock(&filp->table_lock);
- /* Check if we currently have a reference on the object */
- obj = idr_replace(&filp->object_idr, NULL, handle);
- spin_unlock(&filp->table_lock);
- if (IS_ERR_OR_NULL(obj))
- obj = idr_remove(&filp->object_idr, handle);
- if (!obj) return -EINVAL;
/* Release driver's reference and decrement refcount. */ drm_gem_object_release_handle(handle, obj, filp);
/* And finally make the handle available for future allocations. */
spin_lock(&filp->table_lock);
idr_remove(&filp->object_idr, handle); spin_unlock(&filp->table_lock);
return 0;
-- 2.7.4
-- You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. To post to this group, send email to outreachy-kernel@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170925184728.GA8861%40a.... For more options, visit https://groups.google.com/d/optout.
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
-- You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. To post to this group, send email to outreachy-kernel@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170926082047.eqsd2lslpq.... For more options, visit https://groups.google.com/d/optout.
-- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
-- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
-- You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. To post to this group, send email to outreachy-kernel@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/CAKMK7uHia9TC2mVAGLnx22S2.... For more options, visit https://groups.google.com/d/optout.
On Tue, Sep 26, 2017 at 10:20:47AM +0200, Daniel Vetter wrote:
On Tue, Sep 26, 2017 at 12:17:28AM +0530, Aishwarya Pant wrote:
The IDR deletion interface now returns the deleted entry or NULL if it was not present. So we don't have to do the extra work of checking if we have a reference on the drm_gem_object, this can be handled by checking the return value from idr_remove() and the extra locks can be dropped.
Signed-off-by: Aishwarya Pant aishpant@gmail.com
Haneen is already working on this task, with the patch just merged. Please coordinate with mentors (me or Sean Paul) before starting on something to avoid such clashes in the future.
Apologies for the mix-up, I'll make sure to check in with the mentors before starting a task.
I looked over the other patch sent by Haneen today, there is one thing that I could use some clarity on. I'm not sure how the -this is gross- comment is obsolete since we can drop the check to idr_replace() and use the return value from idr_remove() which seems neater in my opinion.
Note also that some todo items are just ideas, and need confirmation with relevant maintainers first.
Thanks, Daniel
drivers/gpu/drm/drm_gem.c | 21 ++------------------- 1 file changed, 2 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index c55f338..f62757a 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -282,29 +282,12 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle) { struct drm_gem_object *obj;
- /* This is gross. The idr system doesn't let us try a delete and
* return an error code. It just spews if you fail at deleting.* So, we have to grab a lock around finding the object and then* doing the delete on it and dropping the refcount, or the user* could race us to double-decrement the refcount and cause a* use-after-free later. Given the frequency of our handle lookups,* we may want to use ida for number allocation and a hash table* for the pointers, anyway.*/- /* Check if we currently have a reference on the object */
- obj = idr_replace(&filp->object_idr, NULL, handle);
- spin_unlock(&filp->table_lock);
- if (IS_ERR_OR_NULL(obj))
- obj = idr_remove(&filp->object_idr, handle);
- if (!obj) return -EINVAL;
/* Release driver's reference and decrement refcount. */ drm_gem_object_release_handle(handle, obj, filp);
/* And finally make the handle available for future allocations. */
spin_lock(&filp->table_lock);
idr_remove(&filp->object_idr, handle); spin_unlock(&filp->table_lock);
return 0;
-- 2.7.4
-- You received this message because you are subscribed to the Google Groups "outreachy-kernel" group. To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com. To post to this group, send email to outreachy-kernel@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170925184728.GA8861%40a.... For more options, visit https://groups.google.com/d/optout.
-- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Quoting Aishwarya Pant (2017-09-25 19:47:28)
The IDR deletion interface now returns the deleted entry or NULL if it was not present. So we don't have to do the extra work of checking if we have a reference on the drm_gem_object, this can be handled by checking the return value from idr_remove() and the extra locks can be dropped.
Signed-off-by: Aishwarya Pant aishpant@gmail.com
This reintroduces the bug were the idr entry is available for reuse before the drivers have had the change to release their resources. So a new handle may be reused that is then hooked up to the old private data. See commit f6cd7daecff558fab2c45d15283d3e52f688342d Author: Chris Wilson chris@chris-wilson.co.uk Date: Fri Apr 15 12:55:08 2016 +0100
drm: Release driver references to handle before making it available again -Chris
On Tue, Sep 26, 2017 at 10:12:12AM +0100, Chris Wilson wrote:
Quoting Aishwarya Pant (2017-09-25 19:47:28)
The IDR deletion interface now returns the deleted entry or NULL if it was not present. So we don't have to do the extra work of checking if we have a reference on the drm_gem_object, this can be handled by checking the return value from idr_remove() and the extra locks can be dropped.
Signed-off-by: Aishwarya Pant aishpant@gmail.com
This reintroduces the bug were the idr entry is available for reuse before the drivers have had the change to release their resources. So a new handle may be reused that is then hooked up to the old private data. See commit f6cd7daecff558fab2c45d15283d3e52f688342d Author: Chris Wilson chris@chris-wilson.co.uk Date: Fri Apr 15 12:55:08 2016 +0100
drm: Release driver references to handle before making it available again
Thanks, this makes sense now.
-Chris
FYI, we noticed the following commit:
commit: 05eadb82a771dd64588809b382fc437aac5c981f ("drm/gem: use new idr deletion interface to cleanup drm_gem_handle_delete()") url: https://github.com/0day-ci/linux/commits/Aishwarya-Pant/drm-gem-use-new-idr-... base: git://people.freedesktop.org/~airlied/linux.git drm-next
in testcase: phoronix-test-suite with following parameters:
need_x: true test: unvanquished-1.5.0 cpufreq_governor: performance
test-description: The Phoronix Test Suite is the most comprehensive testing and benchmarking platform available that provides an extensible framework for which new tests can be easily added. test-url: http://www.phoronix-test-suite.com/
on test machine: 4 threads Intel(R) Core(TM) i5-2500K CPU @ 3.30GHz with 6G memory
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
+-------------------------------+------------+------------+ | | 47e0cd6b1d | 05eadb82a7 | +-------------------------------+------------+------------+ | boot_successes | 11 | 153 | | boot_failures | 0 | 2 | | BUG:kernel_hang_in_test_stage | 0 | 2 | +-------------------------------+------------+------------+
kern :err : [ 144.487222] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:238 kern :err : [ 144.497601] in_atomic(): 1, irqs_disabled(): 0, pid: 1125, name: Xorg kern :warn : [ 144.504124] CPU: 1 PID: 1125 Comm: Xorg Not tainted 4.13.0-rc5-01130-g05eadb8 #1 kern :warn : [ 144.511622] Hardware name: System manufacturer System Product Name/P8H67-M PRO, BIOS 1002 04/01/2011 kern :warn : [ 144.520763] Call Trace: kern :warn : [ 144.523244] dump_stack+0x63/0x86 kern :warn : [ 144.526586] ___might_sleep+0xf1/0x110 kern :warn : [ 144.530366] __might_sleep+0x4a/0x80 kern :warn : [ 144.534005] mutex_lock+0x20/0x50 kern :warn : [ 144.537413] drm_gem_object_release_handle+0x6d/0x90 [drm] kern :warn : [ 144.542936] drm_gem_handle_delete+0x42/0x70 [drm] kern :warn : [ 144.547773] ? drm_gem_handle_create+0x40/0x40 [drm] kern :warn : [ 144.552789] drm_gem_close_ioctl+0x20/0x30 [drm] kern :warn : [ 144.557461] drm_ioctl_kernel+0x69/0xb0 [drm] kern :warn : [ 144.561877] ? __might_fault+0x2f/0x40 kern :warn : [ 144.565678] drm_ioctl+0x2f9/0x3d0 [drm] kern :warn : [ 144.569629] ? drm_gem_handle_create+0x40/0x40 [drm] kern :warn : [ 144.574652] ? __might_sleep+0x4a/0x80 kern :warn : [ 144.578491] ? selinux_file_ioctl+0x109/0x1b0 kern :warn : [ 144.582928] do_vfs_ioctl+0x95/0x670 kern :warn : [ 144.586586] ? security_file_ioctl+0x43/0x60 kern :warn : [ 144.590936] SyS_ioctl+0x79/0x90 kern :warn : [ 144.594195] entry_SYSCALL_64_fastpath+0x1a/0xa5 kern :warn : [ 144.598900] RIP: 0033:0x7f849fe14ca7 kern :warn : [ 144.602496] RSP: 002b:00007ffd3ae8bd38 EFLAGS: 00003246 ORIG_RAX: 0000000000000010 kern :warn : [ 144.610151] RAX: ffffffffffffffda RBX: 00007f849b2e9148 RCX: 00007f849fe14ca7 kern :warn : [ 144.617412] RDX: 00007ffd3ae8bd70 RSI: 0000000040086409 RDI: 000000000000000d kern :warn : [ 144.624649] RBP: 00007f849adbf500 R08: 000000d16e268188 R09: 00000000000000a0 kern :warn : [ 144.631851] R10: 00007ffd3ae8bd80 R11: 0000000000003246 R12: 00007ffd3ae8bdc0 kern :warn : [ 144.639064] R13: 000000d16e2676f0 R14: 000000d16e267688 R15: 0000000000000001 user :notice: [ 158.502568] PTS_SILENT_MODE=1
user :notice: [ 159.138131] 2017-09-29 07:05:50 phoronix-test-suite default-run unvanquished-1.5.0
kern :err : [ 159.534525] BUG: sleeping function called from invalid context at kernel/locking/mutex.c:238 kern :err : [ 159.544927] in_atomic(): 1, irqs_disabled(): 0, pid: 1357, name: glxinfo kern :warn : [ 159.551758] CPU: 2 PID: 1357 Comm: glxinfo Tainted: G W 4.13.0-rc5-01130-g05eadb8 #1 kern :warn : [ 159.560702] Hardware name: System manufacturer System Product Name/P8H67-M PRO, BIOS 1002 04/01/2011 kern :warn : [ 159.569930] Call Trace: kern :warn : [ 159.572377] dump_stack+0x63/0x86 kern :warn : [ 159.575755] ___might_sleep+0xf1/0x110 kern :warn : [ 159.579526] __might_sleep+0x4a/0x80 kern :warn : [ 159.583175] mutex_lock+0x20/0x50 kern :warn : [ 159.586505] drm_gem_object_release_handle+0x6d/0x90 [drm] kern :warn : [ 159.592061] drm_gem_handle_delete+0x42/0x70 [drm] kern :warn : [ 159.596898] ? drm_gem_handle_create+0x40/0x40 [drm] kern :warn : [ 159.601915] drm_gem_close_ioctl+0x20/0x30 [drm] kern :warn : [ 159.606612] drm_ioctl_kernel+0x69/0xb0 [drm] kern :warn : [ 159.610977] ? __might_fault+0x2f/0x40 kern :warn : [ 159.614785] drm_ioctl+0x2f9/0x3d0 [drm] kern :warn : [ 159.618783] ? drm_gem_handle_create+0x40/0x40 [drm] kern :warn : [ 159.623785] ? __might_sleep+0x4a/0x80 kern :warn : [ 159.627634] ? selinux_file_ioctl+0x109/0x1b0 kern :warn : [ 159.632054] do_vfs_ioctl+0x95/0x670 kern :warn : [ 159.635687] ? security_file_ioctl+0x43/0x60 kern :warn : [ 159.639992] SyS_ioctl+0x79/0x90 kern :warn : [ 159.643261] entry_SYSCALL_64_fastpath+0x1a/0xa5 kern :warn : [ 159.647888] RIP: 0033:0x7f1a77a10ca7 kern :warn : [ 159.651501] RSP: 002b:00007ffc99e109f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010 kern :warn : [ 159.659259] RAX: ffffffffffffffda RBX: 00000000017cb890 RCX: 00007f1a77a10ca7 kern :warn : [ 159.666469] RDX: 00007ffc99e10a30 RSI: 0000000040086409 RDI: 0000000000000005 kern :warn : [ 159.673635] RBP: 0000000000000005 R08: 00000000017cb898 R09: 00000000000000a0 kern :warn : [ 159.680846] R10: 00007ffc99e10a40 R11: 0000000000000246 R12: 00000000017cad98 kern :warn : [ 159.688075] R13: 0000000000000000 R14: 00000000017cad98 R15: 0000000000000001
user :notice: [ 199.785643] No Internet Connectivity After Wait
user :notice: [ 203.792063] IPMI BMC is not supported on this machine, skip bmc-watchdog setup!
kern :info : [ 245.457087] perf: interrupt took too long (2535 > 2500), lowering kernel.perf_event_max_sample_rate to 78000 kern :info : [ 246.470090] perf: interrupt took too long (3182 > 3168), lowering kernel.perf_event_max_sample_rate to 62000 kern :info : [ 248.263087] perf: interrupt took too long (3980 > 3977), lowering kernel.perf_event_max_sample_rate to 50000
user :notice: [ 249.796984] No Internet Connectivity After Wait
user :notice: [ 250.900465] Phoronix Test Suite v5.8.1
user :notice: [ 250.906065] System Information
user :notice: [ 250.915926] Hardware:
user :notice: [ 250.925333] Processor: Intel Core i5-2500K @ 3.70GHz (4 Cores), Motherboard: ASUS P8H67-M PRO, Chipset: Intel 2nd Generation Core Family DRAM, Memory: 2048 MB + 1024 MB + 2048 MB + 1024 MB DDR3-1067MHz, Disk: 2000GB Western Digital WD20EZRX-00D, Graphics: Intel HD 3000 (1100MHz), Audio: Realtek ALC892, Network: Realtek RTL8111/8168/8411
user :notice: [ 251.129764] Software:
user :notice: [ 251.137772] OS: Debian GNU/Linux 9, Kernel: 4.13.0-rc5-01130-g05eadb8 (x86_64) 20170928, Display Server: X Server 1.16.4, Display Driver: intel 2.21.15, OpenGL: 3.3 Mesa 17.1.4, Compiler: GCC 6.4.0 20170704, File-System: UNKNOWN (0x794c7630), Screen Resolution: 1024x768
user :notice: [ 251.165486] Would you like to save these test results (Y/n):
user :notice: [ 251.174880] Unvanquished 0.26.0:
user :notice: [ 251.180651] pts/unvanquished-1.5.0 [Resolution: 800 x 600 - Effects Quality: Ultra]
user :notice: [ 251.190390] Test 1 of 2
user :notice: [ 251.195185] Estimated Trial Run Count: 3
user :notice: [ 251.201771] Estimated Test Run-Time: 6 Minutes
user :notice: [ 251.209174] Estimated Time To Completion: 12 Minutes
user :notice: [ 251.713270] Started Run 1 @ 18:07:22
user :notice: [ 253.713900] The test run did not produce a result.
user :notice: [ 253.722412] Started Run 2 @ 18:07:25
user :notice: [ 255.722956] The test run did not produce a result.
user :notice: [ 255.731454] Started Run 3 @ 18:07:27
user :notice: [ 255.737830] The test run did not produce a result.
user :notice: [ 255.746612] Test Results:
user :notice: [ 255.753047] Average: 0 Frames Per Second
user :notice: [ 255.759340] This test failed to run properly.
user :notice: [ 261.736310] Unvanquished 0.26.0:
user :notice: [ 261.742172] pts/unvanquished-1.5.0 [Resolution: 1024 x 768 - Effects Quality: Ultra]
user :notice: [ 261.751953] Test 2 of 2
To reproduce:
git clone https://github.com/intel/lkp-tests.git cd lkp-tests bin/lkp install job.yaml # job file is attached in this email bin/lkp run job.yaml
Thanks, lkp
dri-devel@lists.freedesktop.org
-
Aishwarya Pant -
Chris Wilson -
Daniel Vetter -
Julia Lawall -
kernel test robot