* drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59)
@ 2020-11-16 10:08 ` Dan Carpenter
0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2020-11-16 10:08 UTC (permalink / raw)
To: kbuild, Chris Wilson
Cc: lkp, kbuild-all, linux-kernel, Joonas Lahtinen, Tvrtko Ursulin,
Rodrigo Vivi
[-- Attachment #1: Type: text/plain, Size: 4019 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 0062442ecfef0d82cd69e3e600d5006357f8d8e4
commit: 27a5dcfe73f4b696b3de8c23a560199bb1c193a4 drm/i915/gem: Remove disordered per-file request list for throttling
config: i386-randconfig-m021-20201115 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59)
vim +59 drivers/gpu/drm/i915/gem/i915_gem_throttle.c
35 int
36 i915_gem_throttle_ioctl(struct drm_device *dev, void *data,
37 struct drm_file *file)
38 {
39 const unsigned long recent_enough = jiffies - DRM_I915_THROTTLE_JIFFIES;
40 struct drm_i915_file_private *file_priv = file->driver_priv;
41 struct i915_gem_context *ctx;
42 unsigned long idx;
43 long ret;
44
45 /* ABI: return -EIO if already wedged */
46 ret = intel_gt_terminally_wedged(&to_i915(dev)->gt);
47 if (ret)
48 return ret;
49
50 rcu_read_lock();
51 xa_for_each(&file_priv->context_xa, idx, ctx) {
52 struct i915_gem_engines_iter it;
53 struct intel_context *ce;
54
55 if (!kref_get_unless_zero(&ctx->ref))
56 continue;
57 rcu_read_unlock();
58
59 for_each_gem_engine(ce,
60 i915_gem_context_lock_engines(ctx),
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
I don't understand why this takes the lock every iteration through the
loop
61 it) {
62 struct i915_request *rq, *target = NULL;
63
64 if (!ce->timeline)
65 continue;
66
67 mutex_lock(&ce->timeline->mutex);
68 list_for_each_entry_reverse(rq,
69 &ce->timeline->requests,
70 link) {
71 if (i915_request_completed(rq))
72 break;
73
74 if (time_after(rq->emitted_jiffies,
75 recent_enough))
76 continue;
77
78 target = i915_request_get(rq);
79 break;
80 }
81 mutex_unlock(&ce->timeline->mutex);
82 if (!target)
83 continue;
84
85 ret = i915_request_wait(target,
86 I915_WAIT_INTERRUPTIBLE,
87 MAX_SCHEDULE_TIMEOUT);
88 i915_request_put(target);
89 if (ret < 0)
90 break;
91 }
92 i915_gem_context_unlock_engines(ctx);
But only unlocks the last element
93 i915_gem_context_put(ctx);
94
95 rcu_read_lock();
96 }
97 rcu_read_unlock();
98
99 return ret < 0 ? ret : 0;
100 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 45384 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59)
@ 2020-11-16 10:08 ` Dan Carpenter
0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2020-11-16 10:08 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 4111 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 0062442ecfef0d82cd69e3e600d5006357f8d8e4
commit: 27a5dcfe73f4b696b3de8c23a560199bb1c193a4 drm/i915/gem: Remove disordered per-file request list for throttling
config: i386-randconfig-m021-20201115 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59)
vim +59 drivers/gpu/drm/i915/gem/i915_gem_throttle.c
35 int
36 i915_gem_throttle_ioctl(struct drm_device *dev, void *data,
37 struct drm_file *file)
38 {
39 const unsigned long recent_enough = jiffies - DRM_I915_THROTTLE_JIFFIES;
40 struct drm_i915_file_private *file_priv = file->driver_priv;
41 struct i915_gem_context *ctx;
42 unsigned long idx;
43 long ret;
44
45 /* ABI: return -EIO if already wedged */
46 ret = intel_gt_terminally_wedged(&to_i915(dev)->gt);
47 if (ret)
48 return ret;
49
50 rcu_read_lock();
51 xa_for_each(&file_priv->context_xa, idx, ctx) {
52 struct i915_gem_engines_iter it;
53 struct intel_context *ce;
54
55 if (!kref_get_unless_zero(&ctx->ref))
56 continue;
57 rcu_read_unlock();
58
59 for_each_gem_engine(ce,
60 i915_gem_context_lock_engines(ctx),
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
I don't understand why this takes the lock every iteration through the
loop
61 it) {
62 struct i915_request *rq, *target = NULL;
63
64 if (!ce->timeline)
65 continue;
66
67 mutex_lock(&ce->timeline->mutex);
68 list_for_each_entry_reverse(rq,
69 &ce->timeline->requests,
70 link) {
71 if (i915_request_completed(rq))
72 break;
73
74 if (time_after(rq->emitted_jiffies,
75 recent_enough))
76 continue;
77
78 target = i915_request_get(rq);
79 break;
80 }
81 mutex_unlock(&ce->timeline->mutex);
82 if (!target)
83 continue;
84
85 ret = i915_request_wait(target,
86 I915_WAIT_INTERRUPTIBLE,
87 MAX_SCHEDULE_TIMEOUT);
88 i915_request_put(target);
89 if (ret < 0)
90 break;
91 }
92 i915_gem_context_unlock_engines(ctx);
But only unlocks the last element
93 i915_gem_context_put(ctx);
94
95 rcu_read_lock();
96 }
97 rcu_read_unlock();
98
99 return ret < 0 ? ret : 0;
100 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 45384 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59)
@ 2020-11-16 10:08 ` Dan Carpenter
0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2020-11-16 10:08 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4111 bytes --]
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 0062442ecfef0d82cd69e3e600d5006357f8d8e4
commit: 27a5dcfe73f4b696b3de8c23a560199bb1c193a4 drm/i915/gem: Remove disordered per-file request list for throttling
config: i386-randconfig-m021-20201115 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59)
vim +59 drivers/gpu/drm/i915/gem/i915_gem_throttle.c
35 int
36 i915_gem_throttle_ioctl(struct drm_device *dev, void *data,
37 struct drm_file *file)
38 {
39 const unsigned long recent_enough = jiffies - DRM_I915_THROTTLE_JIFFIES;
40 struct drm_i915_file_private *file_priv = file->driver_priv;
41 struct i915_gem_context *ctx;
42 unsigned long idx;
43 long ret;
44
45 /* ABI: return -EIO if already wedged */
46 ret = intel_gt_terminally_wedged(&to_i915(dev)->gt);
47 if (ret)
48 return ret;
49
50 rcu_read_lock();
51 xa_for_each(&file_priv->context_xa, idx, ctx) {
52 struct i915_gem_engines_iter it;
53 struct intel_context *ce;
54
55 if (!kref_get_unless_zero(&ctx->ref))
56 continue;
57 rcu_read_unlock();
58
59 for_each_gem_engine(ce,
60 i915_gem_context_lock_engines(ctx),
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
I don't understand why this takes the lock every iteration through the
loop
61 it) {
62 struct i915_request *rq, *target = NULL;
63
64 if (!ce->timeline)
65 continue;
66
67 mutex_lock(&ce->timeline->mutex);
68 list_for_each_entry_reverse(rq,
69 &ce->timeline->requests,
70 link) {
71 if (i915_request_completed(rq))
72 break;
73
74 if (time_after(rq->emitted_jiffies,
75 recent_enough))
76 continue;
77
78 target = i915_request_get(rq);
79 break;
80 }
81 mutex_unlock(&ce->timeline->mutex);
82 if (!target)
83 continue;
84
85 ret = i915_request_wait(target,
86 I915_WAIT_INTERRUPTIBLE,
87 MAX_SCHEDULE_TIMEOUT);
88 i915_request_put(target);
89 if (ret < 0)
90 break;
91 }
92 i915_gem_context_unlock_engines(ctx);
But only unlocks the last element
93 i915_gem_context_put(ctx);
94
95 rcu_read_lock();
96 }
97 rcu_read_unlock();
98
99 return ret < 0 ? ret : 0;
100 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 45384 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59)
2020-11-16 10:08 ` Dan Carpenter
(?)
(?)
@ 2020-11-16 10:15 ` Chris Wilson
2020-11-16 10:44 ` Dan Carpenter
-1 siblings, 1 reply; 8+ messages in thread
From: Chris Wilson @ 2020-11-16 10:15 UTC (permalink / raw)
To: Dan Carpenter, kbuild
Cc: lkp, kbuild-all, linux-kernel, Joonas Lahtinen, Tvrtko Ursulin,
Rodrigo Vivi
Quoting Dan Carpenter (2020-11-16 10:08:38)
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 0062442ecfef0d82cd69e3e600d5006357f8d8e4
> commit: 27a5dcfe73f4b696b3de8c23a560199bb1c193a4 drm/i915/gem: Remove disordered per-file request list for throttling
> config: i386-randconfig-m021-20201115 (attached as .config)
> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
>
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> smatch warnings:
> drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59)
>
> vim +59 drivers/gpu/drm/i915/gem/i915_gem_throttle.c
>
> 35 int
> 36 i915_gem_throttle_ioctl(struct drm_device *dev, void *data,
> 37 struct drm_file *file)
> 38 {
> 39 const unsigned long recent_enough = jiffies - DRM_I915_THROTTLE_JIFFIES;
> 40 struct drm_i915_file_private *file_priv = file->driver_priv;
> 41 struct i915_gem_context *ctx;
> 42 unsigned long idx;
> 43 long ret;
> 44
> 45 /* ABI: return -EIO if already wedged */
> 46 ret = intel_gt_terminally_wedged(&to_i915(dev)->gt);
> 47 if (ret)
> 48 return ret;
> 49
> 50 rcu_read_lock();
> 51 xa_for_each(&file_priv->context_xa, idx, ctx) {
> 52 struct i915_gem_engines_iter it;
> 53 struct intel_context *ce;
> 54
> 55 if (!kref_get_unless_zero(&ctx->ref))
> 56 continue;
> 57 rcu_read_unlock();
> 58
> 59 for_each_gem_engine(ce,
> 60 i915_gem_context_lock_engines(ctx),
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> I don't understand why this takes the lock every iteration through the
> loop
It doesn't.
static inline struct i915_gem_engines *
i915_gem_context_lock_engines(struct i915_gem_context *ctx)
__acquires(&ctx->engines_mutex)
{
mutex_lock(&ctx->engines_mutex);
return i915_gem_context_engines(ctx);
}
static inline void
i915_gem_context_unlock_engines(struct i915_gem_context *ctx)
__releases(&ctx->engines_mutex)
{
mutex_unlock(&ctx->engines_mutex);
}
with the i915_gem_engines stored as a local in the iterator at the start
of the for loop.
-Chris
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59)
2020-11-16 10:15 ` Chris Wilson
2020-11-16 10:44 ` Dan Carpenter
@ 2020-11-16 10:44 ` Dan Carpenter
0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2020-11-16 10:44 UTC (permalink / raw)
To: Chris Wilson
Cc: kbuild, lkp, kbuild-all, linux-kernel, Joonas Lahtinen,
Tvrtko Ursulin, Rodrigo Vivi
On Mon, Nov 16, 2020 at 10:15:04AM +0000, Chris Wilson wrote:
> Quoting Dan Carpenter (2020-11-16 10:08:38)
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head: 0062442ecfef0d82cd69e3e600d5006357f8d8e4
> > commit: 27a5dcfe73f4b696b3de8c23a560199bb1c193a4 drm/i915/gem: Remove disordered per-file request list for throttling
> > config: i386-randconfig-m021-20201115 (attached as .config)
> > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > smatch warnings:
> > drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59)
> >
> > vim +59 drivers/gpu/drm/i915/gem/i915_gem_throttle.c
> >
> > 35 int
> > 36 i915_gem_throttle_ioctl(struct drm_device *dev, void *data,
> > 37 struct drm_file *file)
> > 38 {
> > 39 const unsigned long recent_enough = jiffies - DRM_I915_THROTTLE_JIFFIES;
> > 40 struct drm_i915_file_private *file_priv = file->driver_priv;
> > 41 struct i915_gem_context *ctx;
> > 42 unsigned long idx;
> > 43 long ret;
> > 44
> > 45 /* ABI: return -EIO if already wedged */
> > 46 ret = intel_gt_terminally_wedged(&to_i915(dev)->gt);
> > 47 if (ret)
> > 48 return ret;
> > 49
> > 50 rcu_read_lock();
> > 51 xa_for_each(&file_priv->context_xa, idx, ctx) {
> > 52 struct i915_gem_engines_iter it;
> > 53 struct intel_context *ce;
> > 54
> > 55 if (!kref_get_unless_zero(&ctx->ref))
> > 56 continue;
> > 57 rcu_read_unlock();
> > 58
> > 59 for_each_gem_engine(ce,
> > 60 i915_gem_context_lock_engines(ctx),
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > I don't understand why this takes the lock every iteration through the
> > loop
>
> It doesn't.
>
> static inline struct i915_gem_engines *
> i915_gem_context_lock_engines(struct i915_gem_context *ctx)
> __acquires(&ctx->engines_mutex)
> {
> mutex_lock(&ctx->engines_mutex);
> return i915_gem_context_engines(ctx);
> }
>
> static inline void
> i915_gem_context_unlock_engines(struct i915_gem_context *ctx)
> __releases(&ctx->engines_mutex)
> {
> mutex_unlock(&ctx->engines_mutex);
> }
>
> with the i915_gem_engines stored as a local in the iterator at the start
> of the for loop.
Yeah... But that's true enough. But what I think is actually causing
the static checker warning are the continues.
52 xa_for_each(&file_priv->context_xa, idx, ctx) {
53 struct i915_gem_engines_iter it;
54 struct intel_context *ce;
55
56 if (!kref_get_unless_zero(&ctx->ref))
57 continue;
58 rcu_read_unlock();
59
60 for_each_gem_engine(ce,
61 i915_gem_context_lock_engines(ctx),
62 it) {
63 struct i915_request *rq, *target = NULL;
64
65 if (!ce->timeline)
66 continue;
^^^^^^^^^
This continue is for the inside loop, so "ctx" isn't iterated. There
is another continue as well later in the loop. Potentially they could
be replaced with breaks?
67
68 mutex_lock(&ce->timeline->mutex);
69 list_for_each_entry_reverse(rq,
70 &ce->timeline->requests,
71 link) {
72 if (i915_request_completed(rq))
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59)
@ 2020-11-16 10:44 ` Dan Carpenter
0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2020-11-16 10:44 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 4340 bytes --]
On Mon, Nov 16, 2020 at 10:15:04AM +0000, Chris Wilson wrote:
> Quoting Dan Carpenter (2020-11-16 10:08:38)
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head: 0062442ecfef0d82cd69e3e600d5006357f8d8e4
> > commit: 27a5dcfe73f4b696b3de8c23a560199bb1c193a4 drm/i915/gem: Remove disordered per-file request list for throttling
> > config: i386-randconfig-m021-20201115 (attached as .config)
> > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > smatch warnings:
> > drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59)
> >
> > vim +59 drivers/gpu/drm/i915/gem/i915_gem_throttle.c
> >
> > 35 int
> > 36 i915_gem_throttle_ioctl(struct drm_device *dev, void *data,
> > 37 struct drm_file *file)
> > 38 {
> > 39 const unsigned long recent_enough = jiffies - DRM_I915_THROTTLE_JIFFIES;
> > 40 struct drm_i915_file_private *file_priv = file->driver_priv;
> > 41 struct i915_gem_context *ctx;
> > 42 unsigned long idx;
> > 43 long ret;
> > 44
> > 45 /* ABI: return -EIO if already wedged */
> > 46 ret = intel_gt_terminally_wedged(&to_i915(dev)->gt);
> > 47 if (ret)
> > 48 return ret;
> > 49
> > 50 rcu_read_lock();
> > 51 xa_for_each(&file_priv->context_xa, idx, ctx) {
> > 52 struct i915_gem_engines_iter it;
> > 53 struct intel_context *ce;
> > 54
> > 55 if (!kref_get_unless_zero(&ctx->ref))
> > 56 continue;
> > 57 rcu_read_unlock();
> > 58
> > 59 for_each_gem_engine(ce,
> > 60 i915_gem_context_lock_engines(ctx),
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > I don't understand why this takes the lock every iteration through the
> > loop
>
> It doesn't.
>
> static inline struct i915_gem_engines *
> i915_gem_context_lock_engines(struct i915_gem_context *ctx)
> __acquires(&ctx->engines_mutex)
> {
> mutex_lock(&ctx->engines_mutex);
> return i915_gem_context_engines(ctx);
> }
>
> static inline void
> i915_gem_context_unlock_engines(struct i915_gem_context *ctx)
> __releases(&ctx->engines_mutex)
> {
> mutex_unlock(&ctx->engines_mutex);
> }
>
> with the i915_gem_engines stored as a local in the iterator at the start
> of the for loop.
Yeah... But that's true enough. But what I think is actually causing
the static checker warning are the continues.
52 xa_for_each(&file_priv->context_xa, idx, ctx) {
53 struct i915_gem_engines_iter it;
54 struct intel_context *ce;
55
56 if (!kref_get_unless_zero(&ctx->ref))
57 continue;
58 rcu_read_unlock();
59
60 for_each_gem_engine(ce,
61 i915_gem_context_lock_engines(ctx),
62 it) {
63 struct i915_request *rq, *target = NULL;
64
65 if (!ce->timeline)
66 continue;
^^^^^^^^^
This continue is for the inside loop, so "ctx" isn't iterated. There
is another continue as well later in the loop. Potentially they could
be replaced with breaks?
67
68 mutex_lock(&ce->timeline->mutex);
69 list_for_each_entry_reverse(rq,
70 &ce->timeline->requests,
71 link) {
72 if (i915_request_completed(rq))
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59)
@ 2020-11-16 10:44 ` Dan Carpenter
0 siblings, 0 replies; 8+ messages in thread
From: Dan Carpenter @ 2020-11-16 10:44 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4340 bytes --]
On Mon, Nov 16, 2020 at 10:15:04AM +0000, Chris Wilson wrote:
> Quoting Dan Carpenter (2020-11-16 10:08:38)
> > tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> > head: 0062442ecfef0d82cd69e3e600d5006357f8d8e4
> > commit: 27a5dcfe73f4b696b3de8c23a560199bb1c193a4 drm/i915/gem: Remove disordered per-file request list for throttling
> > config: i386-randconfig-m021-20201115 (attached as .config)
> > compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
> >
> > If you fix the issue, kindly add following tag as appropriate
> > Reported-by: kernel test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > smatch warnings:
> > drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59)
> >
> > vim +59 drivers/gpu/drm/i915/gem/i915_gem_throttle.c
> >
> > 35 int
> > 36 i915_gem_throttle_ioctl(struct drm_device *dev, void *data,
> > 37 struct drm_file *file)
> > 38 {
> > 39 const unsigned long recent_enough = jiffies - DRM_I915_THROTTLE_JIFFIES;
> > 40 struct drm_i915_file_private *file_priv = file->driver_priv;
> > 41 struct i915_gem_context *ctx;
> > 42 unsigned long idx;
> > 43 long ret;
> > 44
> > 45 /* ABI: return -EIO if already wedged */
> > 46 ret = intel_gt_terminally_wedged(&to_i915(dev)->gt);
> > 47 if (ret)
> > 48 return ret;
> > 49
> > 50 rcu_read_lock();
> > 51 xa_for_each(&file_priv->context_xa, idx, ctx) {
> > 52 struct i915_gem_engines_iter it;
> > 53 struct intel_context *ce;
> > 54
> > 55 if (!kref_get_unless_zero(&ctx->ref))
> > 56 continue;
> > 57 rcu_read_unlock();
> > 58
> > 59 for_each_gem_engine(ce,
> > 60 i915_gem_context_lock_engines(ctx),
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > I don't understand why this takes the lock every iteration through the
> > loop
>
> It doesn't.
>
> static inline struct i915_gem_engines *
> i915_gem_context_lock_engines(struct i915_gem_context *ctx)
> __acquires(&ctx->engines_mutex)
> {
> mutex_lock(&ctx->engines_mutex);
> return i915_gem_context_engines(ctx);
> }
>
> static inline void
> i915_gem_context_unlock_engines(struct i915_gem_context *ctx)
> __releases(&ctx->engines_mutex)
> {
> mutex_unlock(&ctx->engines_mutex);
> }
>
> with the i915_gem_engines stored as a local in the iterator at the start
> of the for loop.
Yeah... But that's true enough. But what I think is actually causing
the static checker warning are the continues.
52 xa_for_each(&file_priv->context_xa, idx, ctx) {
53 struct i915_gem_engines_iter it;
54 struct intel_context *ce;
55
56 if (!kref_get_unless_zero(&ctx->ref))
57 continue;
58 rcu_read_unlock();
59
60 for_each_gem_engine(ce,
61 i915_gem_context_lock_engines(ctx),
62 it) {
63 struct i915_request *rq, *target = NULL;
64
65 if (!ce->timeline)
66 continue;
^^^^^^^^^
This continue is for the inside loop, so "ctx" isn't iterated. There
is another continue as well later in the loop. Potentially they could
be replaced with breaks?
67
68 mutex_lock(&ce->timeline->mutex);
69 list_for_each_entry_reverse(rq,
70 &ce->timeline->requests,
71 link) {
72 if (i915_request_completed(rq))
regards,
dan carpenter
^ permalink raw reply [flat|nested] 8+ messages in thread
* drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59)
@ 2020-11-15 20:19 kernel test robot
0 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2020-11-15 20:19 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 4006 bytes --]
CC: kbuild-all(a)lists.01.org
CC: linux-kernel(a)vger.kernel.org
TO: Chris Wilson <chris@chris-wilson.co.uk>
CC: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
CC: Rodrigo Vivi <rodrigo.vivi@intel.com>
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
head: 0062442ecfef0d82cd69e3e600d5006357f8d8e4
commit: 27a5dcfe73f4b696b3de8c23a560199bb1c193a4 drm/i915/gem: Remove disordered per-file request list for throttling
date: 10 weeks ago
:::::: branch date: 2 hours ago
:::::: commit date: 10 weeks ago
config: i386-randconfig-m021-20201115 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59)
vim +59 drivers/gpu/drm/i915/gem/i915_gem_throttle.c
446e2d16a131ed2 Chris Wilson 2019-05-28 23
446e2d16a131ed2 Chris Wilson 2019-05-28 24 /*
446e2d16a131ed2 Chris Wilson 2019-05-28 25 * Throttle our rendering by waiting until the ring has completed our requests
446e2d16a131ed2 Chris Wilson 2019-05-28 26 * emitted over 20 msec ago.
446e2d16a131ed2 Chris Wilson 2019-05-28 27 *
446e2d16a131ed2 Chris Wilson 2019-05-28 28 * Note that if we were to use the current jiffies each time around the loop,
446e2d16a131ed2 Chris Wilson 2019-05-28 29 * we wouldn't escape the function with any frames outstanding if the time to
446e2d16a131ed2 Chris Wilson 2019-05-28 30 * render a frame was over 20ms.
446e2d16a131ed2 Chris Wilson 2019-05-28 31 *
446e2d16a131ed2 Chris Wilson 2019-05-28 32 * This should get us reasonable parallelism between CPU and GPU but also
446e2d16a131ed2 Chris Wilson 2019-05-28 33 * relatively low latency when blocking on a particular request to finish.
446e2d16a131ed2 Chris Wilson 2019-05-28 34 */
446e2d16a131ed2 Chris Wilson 2019-05-28 35 int
446e2d16a131ed2 Chris Wilson 2019-05-28 36 i915_gem_throttle_ioctl(struct drm_device *dev, void *data,
446e2d16a131ed2 Chris Wilson 2019-05-28 37 struct drm_file *file)
446e2d16a131ed2 Chris Wilson 2019-05-28 38 {
27a5dcfe73f4b69 Chris Wilson 2020-07-28 39 const unsigned long recent_enough = jiffies - DRM_I915_THROTTLE_JIFFIES;
446e2d16a131ed2 Chris Wilson 2019-05-28 40 struct drm_i915_file_private *file_priv = file->driver_priv;
27a5dcfe73f4b69 Chris Wilson 2020-07-28 41 struct i915_gem_context *ctx;
27a5dcfe73f4b69 Chris Wilson 2020-07-28 42 unsigned long idx;
446e2d16a131ed2 Chris Wilson 2019-05-28 43 long ret;
446e2d16a131ed2 Chris Wilson 2019-05-28 44
446e2d16a131ed2 Chris Wilson 2019-05-28 45 /* ABI: return -EIO if already wedged */
cb823ed9915b0d4 Chris Wilson 2019-07-12 46 ret = intel_gt_terminally_wedged(&to_i915(dev)->gt);
446e2d16a131ed2 Chris Wilson 2019-05-28 47 if (ret)
446e2d16a131ed2 Chris Wilson 2019-05-28 48 return ret;
446e2d16a131ed2 Chris Wilson 2019-05-28 49
27a5dcfe73f4b69 Chris Wilson 2020-07-28 50 rcu_read_lock();
27a5dcfe73f4b69 Chris Wilson 2020-07-28 51 xa_for_each(&file_priv->context_xa, idx, ctx) {
27a5dcfe73f4b69 Chris Wilson 2020-07-28 52 struct i915_gem_engines_iter it;
27a5dcfe73f4b69 Chris Wilson 2020-07-28 53 struct intel_context *ce;
27a5dcfe73f4b69 Chris Wilson 2020-07-28 54
27a5dcfe73f4b69 Chris Wilson 2020-07-28 55 if (!kref_get_unless_zero(&ctx->ref))
27a5dcfe73f4b69 Chris Wilson 2020-07-28 56 continue;
27a5dcfe73f4b69 Chris Wilson 2020-07-28 57 rcu_read_unlock();
27a5dcfe73f4b69 Chris Wilson 2020-07-28 58
27a5dcfe73f4b69 Chris Wilson 2020-07-28 @59 for_each_gem_engine(ce,
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 45384 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2020-11-16 12:19 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-16 10:08 drivers/gpu/drm/i915/gem/i915_gem_throttle.c:59 i915_gem_throttle_ioctl() error: double locked 'ctx->engines_mutex' (orig line 59) Dan Carpenter
2020-11-16 10:08 ` Dan Carpenter
2020-11-16 10:08 ` Dan Carpenter
2020-11-16 10:15 ` Chris Wilson
2020-11-16 10:44 ` Dan Carpenter
2020-11-16 10:44 ` Dan Carpenter
2020-11-16 10:44 ` Dan Carpenter
-- strict thread matches above, loose matches on Subject: below --
2020-11-15 20:19 kernel test robot
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.