* [kernel-locking] question about structure field initialization
@ 2017-05-11 20:00 Gustavo A. R. Silva
2017-05-12 8:38 ` Chris Wilson
0 siblings, 1 reply; 3+ messages in thread
From: Gustavo A. R. Silva @ 2017-05-11 20:00 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar; +Cc: linux-kernel
Hello everybody,
While looking into Coverity ID 1402035 I ran into the following piece
of code at kernel/locking/test-ww_mutex.c:197:
197static int test_abba(bool resolve)
198{
199 struct test_abba abba;
200 struct ww_acquire_ctx ctx;
201 int err, ret;
202
203 ww_mutex_init(&abba.a_mutex, &ww_class);
204 ww_mutex_init(&abba.b_mutex, &ww_class);
205 INIT_WORK_ONSTACK(&abba.work, test_abba_work);
206 init_completion(&abba.a_ready);
207 init_completion(&abba.b_ready);
208 abba.resolve = resolve;
209
210 schedule_work(&abba.work);
211
212 ww_acquire_init(&ctx, &ww_class);
213 ww_mutex_lock(&abba.a_mutex, &ctx);
214
215 complete(&abba.a_ready);
216 wait_for_completion(&abba.b_ready);
217
218 err = ww_mutex_lock(&abba.b_mutex, &ctx);
219 if (resolve && err == -EDEADLK) {
220 ww_mutex_unlock(&abba.a_mutex);
221 ww_mutex_lock_slow(&abba.b_mutex, &ctx);
222 err = ww_mutex_lock(&abba.a_mutex, &ctx);
223 }
224
225 if (!err)
226 ww_mutex_unlock(&abba.b_mutex);
227 ww_mutex_unlock(&abba.a_mutex);
228 ww_acquire_fini(&ctx);
229
230 flush_work(&abba.work);
231 destroy_work_on_stack(&abba.work);
232
233 ret = 0;
234 if (resolve) {
235 if (err || abba.result) {
236 pr_err("%s: failed to resolve ABBA
deadlock, A err=%d, B err=%d\n",
237 __func__, err, abba.result);
238 ret = -EINVAL;
239 }
240 } else {
241 if (err != -EDEADLK && abba.result != -EDEADLK) {
242 pr_err("%s: missed ABBA deadlock, A err=%d,
B err=%d\n",
243 __func__, err, abba.result);
244 ret = -EINVAL;
245 }
246 }
247 return ret;
248}
The issue here is that apparently abba.result is being used at lines
235, 237 and 241 without previous initialization.
It seems to me that this is an issue, but I may be overlooking something.
Can someone help me to spot where exactly abba.result is being
initialized, if at all?
I'm trying to figure out if this is a false positive or something that
needs to be fixed.
I'd really appreciate any comment on this.
Thank you!
--
Gustavo A. R. Silva
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [kernel-locking] question about structure field initialization
2017-05-11 20:00 [kernel-locking] question about structure field initialization Gustavo A. R. Silva
@ 2017-05-12 8:38 ` Chris Wilson
2017-05-16 17:16 ` Gustavo A. R. Silva
0 siblings, 1 reply; 3+ messages in thread
From: Chris Wilson @ 2017-05-12 8:38 UTC (permalink / raw)
To: Gustavo A. R. Silva; +Cc: Peter Zijlstra, Ingo Molnar, linux-kernel
On Thu, May 11, 2017 at 03:00:02PM -0500, Gustavo A. R. Silva wrote:
>
> Hello everybody,
>
> While looking into Coverity ID 1402035 I ran into the following
> piece of code at kernel/locking/test-ww_mutex.c:197:
>
> 197static int test_abba(bool resolve)
> 198{
> 199 struct test_abba abba;
> 200 struct ww_acquire_ctx ctx;
> 201 int err, ret;
> 202
> 203 ww_mutex_init(&abba.a_mutex, &ww_class);
> 204 ww_mutex_init(&abba.b_mutex, &ww_class);
> 205 INIT_WORK_ONSTACK(&abba.work, test_abba_work);
> 206 init_completion(&abba.a_ready);
> 207 init_completion(&abba.b_ready);
> 208 abba.resolve = resolve;
> 209
> 210 schedule_work(&abba.work);
> 211
> 212 ww_acquire_init(&ctx, &ww_class);
> 213 ww_mutex_lock(&abba.a_mutex, &ctx);
> 214
> 215 complete(&abba.a_ready);
> 216 wait_for_completion(&abba.b_ready);
> 217
> 218 err = ww_mutex_lock(&abba.b_mutex, &ctx);
> 219 if (resolve && err == -EDEADLK) {
> 220 ww_mutex_unlock(&abba.a_mutex);
> 221 ww_mutex_lock_slow(&abba.b_mutex, &ctx);
> 222 err = ww_mutex_lock(&abba.a_mutex, &ctx);
> 223 }
> 224
> 225 if (!err)
> 226 ww_mutex_unlock(&abba.b_mutex);
> 227 ww_mutex_unlock(&abba.a_mutex);
> 228 ww_acquire_fini(&ctx);
> 229
> 230 flush_work(&abba.work);
> 231 destroy_work_on_stack(&abba.work);
> 232
> 233 ret = 0;
> 234 if (resolve) {
> 235 if (err || abba.result) {
> 236 pr_err("%s: failed to resolve ABBA
> deadlock, A err=%d, B err=%d\n",
> 237 __func__, err, abba.result);
> 238 ret = -EINVAL;
> 239 }
> 240 } else {
> 241 if (err != -EDEADLK && abba.result != -EDEADLK) {
> 242 pr_err("%s: missed ABBA deadlock, A
> err=%d, B err=%d\n",
> 243 __func__, err, abba.result);
> 244 ret = -EINVAL;
> 245 }
> 246 }
> 247 return ret;
> 248}
>
> The issue here is that apparently abba.result is being used at lines
> 235, 237 and 241 without previous initialization.
>
> It seems to me that this is an issue, but I may be overlooking something.
> Can someone help me to spot where exactly abba.result is being
> initialized, if at all?
You are only looking at half the code. Though the schedule/flush it is
indirectly executing test_abba_work().
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [kernel-locking] question about structure field initialization
2017-05-12 8:38 ` Chris Wilson
@ 2017-05-16 17:16 ` Gustavo A. R. Silva
0 siblings, 0 replies; 3+ messages in thread
From: Gustavo A. R. Silva @ 2017-05-16 17:16 UTC (permalink / raw)
To: Chris Wilson; +Cc: Peter Zijlstra, Ingo Molnar, linux-kernel
Hi Chris,
Quoting Chris Wilson <chris@chris-wilson.co.uk>:
> On Thu, May 11, 2017 at 03:00:02PM -0500, Gustavo A. R. Silva wrote:
>>
>> Hello everybody,
>>
>> While looking into Coverity ID 1402035 I ran into the following
>> piece of code at kernel/locking/test-ww_mutex.c:197:
>>
>> 197static int test_abba(bool resolve)
>> 198{
>> 199 struct test_abba abba;
>> 200 struct ww_acquire_ctx ctx;
>> 201 int err, ret;
>> 202
>> 203 ww_mutex_init(&abba.a_mutex, &ww_class);
>> 204 ww_mutex_init(&abba.b_mutex, &ww_class);
>> 205 INIT_WORK_ONSTACK(&abba.work, test_abba_work);
>> 206 init_completion(&abba.a_ready);
>> 207 init_completion(&abba.b_ready);
>> 208 abba.resolve = resolve;
>> 209
>> 210 schedule_work(&abba.work);
>> 211
>> 212 ww_acquire_init(&ctx, &ww_class);
>> 213 ww_mutex_lock(&abba.a_mutex, &ctx);
>> 214
>> 215 complete(&abba.a_ready);
>> 216 wait_for_completion(&abba.b_ready);
>> 217
>> 218 err = ww_mutex_lock(&abba.b_mutex, &ctx);
>> 219 if (resolve && err == -EDEADLK) {
>> 220 ww_mutex_unlock(&abba.a_mutex);
>> 221 ww_mutex_lock_slow(&abba.b_mutex, &ctx);
>> 222 err = ww_mutex_lock(&abba.a_mutex, &ctx);
>> 223 }
>> 224
>> 225 if (!err)
>> 226 ww_mutex_unlock(&abba.b_mutex);
>> 227 ww_mutex_unlock(&abba.a_mutex);
>> 228 ww_acquire_fini(&ctx);
>> 229
>> 230 flush_work(&abba.work);
>> 231 destroy_work_on_stack(&abba.work);
>> 232
>> 233 ret = 0;
>> 234 if (resolve) {
>> 235 if (err || abba.result) {
>> 236 pr_err("%s: failed to resolve ABBA
>> deadlock, A err=%d, B err=%d\n",
>> 237 __func__, err, abba.result);
>> 238 ret = -EINVAL;
>> 239 }
>> 240 } else {
>> 241 if (err != -EDEADLK && abba.result != -EDEADLK) {
>> 242 pr_err("%s: missed ABBA deadlock, A
>> err=%d, B err=%d\n",
>> 243 __func__, err, abba.result);
>> 244 ret = -EINVAL;
>> 245 }
>> 246 }
>> 247 return ret;
>> 248}
>>
>> The issue here is that apparently abba.result is being used at lines
>> 235, 237 and 241 without previous initialization.
>>
>> It seems to me that this is an issue, but I may be overlooking something.
>> Can someone help me to spot where exactly abba.result is being
>> initialized, if at all?
>
> You are only looking at half the code. Though the schedule/flush it is
> indirectly executing test_abba_work().
> -Chris
>
I get it.
Thanks for clarifying!
--
Gustavo A. R. Silva
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-05-16 17:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-11 20:00 [kernel-locking] question about structure field initialization Gustavo A. R. Silva
2017-05-12 8:38 ` Chris Wilson
2017-05-16 17:16 ` Gustavo A. R. Silva
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.