* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).