Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1640314 > unrolled thread
| Started by | Chris Wilson <chris@chris-wilson.co.uk> |
|---|---|
| First post | 2017-05-12 10:40 +0200 |
| Last post | 2017-05-16 19:20 +0200 |
| Articles | 2 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [kernel-locking] question about structure field initialization Chris Wilson <chris@chris-wilson.co.uk> - 2017-05-12 10:40 +0200
Re: [kernel-locking] question about structure field initialization "Gustavo A. R. Silva" <garsilva@embeddedor.com> - 2017-05-16 19:20 +0200
| From | Chris Wilson <chris@chris-wilson.co.uk> |
|---|---|
| Date | 2017-05-12 10:40 +0200 |
| Subject | Re: [kernel-locking] question about structure field initialization |
| Message-ID | <tGehb-1Nr-5@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | "Gustavo A. R. Silva" <garsilva@embeddedor.com> |
|---|---|
| Date | 2017-05-16 19:20 +0200 |
| Message-ID | <tHOiB-8rM-3@gated-at.bofh.it> |
| In reply to | #1640314 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web