2008-11-15 10:00:40

by Markus Metzger

Subject: [patch] x86, bts: fix unlock problem in ds.c

Fix a problem where ds_request() returned an error without releasing the
ds lock.

Reported-by: Stephane Eranian <[email protected]>
Signed-off-by: Markus Metzger <[email protected]>
---

Index: gits/arch/x86/kernel/ds.c
===================================================================
--- gits.orig/arch/x86/kernel/ds.c 2008-11-15 10:51:51.000000000 +0100
+++ gits/arch/x86/kernel/ds.c 2008-11-15 10:53:43.000000000 +0100
@@ -384,8 +384,9 @@

spin_lock(&ds_lock);

+ error = -EPERM;
if (!check_tracer(task))
- return -EPERM;
+ goto out_unlock;

error = -ENOMEM;
context = ds_alloc_context(task);

2008-11-16 07:26:37

by Ingo Molnar

Subject: Re: [patch] x86, bts: fix unlock problem in ds.c

* Markus Metzger <[email protected]> wrote:

> Fix a problem where ds_request() returned an error without releasing
> the ds lock.
>
> Reported-by: Stephane Eranian <[email protected]>
> Signed-off-by: Markus Metzger <[email protected]>

applied to tip/x86/urgent, thanks Markus!

i also reviewed the other locking branches there and they seem to be
fine.

Ingo

2008-11-20 21:14:53

by Stephane Eranian

Subject: Re: [patch] x86, bts: fix unlock problem in ds.c

Markus,

I think this patch is not quite right. You don't want to go out via out_unlock
because you're going to call ds_put_context)() when you did not invoke the
matching ds_get_context() (hidden in ds_alloc_context). It happens later in
the ds_request() function.


On Sat, Nov 15, 2008 at 11:00 AM, Markus Metzger
<[email protected]> wrote:
> Fix a problem where ds_request() returned an error without releasing the
> ds lock.
>
> Reported-by: Stephane Eranian <[email protected]>
> Signed-off-by: Markus Metzger <[email protected]>
> ---
>
> Index: gits/arch/x86/kernel/ds.c
> ===================================================================
> --- gits.orig/arch/x86/kernel/ds.c 2008-11-15 10:51:51.000000000 +0100
> +++ gits/arch/x86/kernel/ds.c 2008-11-15 10:53:43.000000000 +0100
> @@ -384,8 +384,9 @@
>
> spin_lock(&ds_lock);
>
> + error = -EPERM;
> if (!check_tracer(task))
> - return -EPERM;
> + goto out_unlock;
>
> error = -ENOMEM;
> context = ds_alloc_context(task);
>
>
>

2008-11-20 21:48:29

by Ingo Molnar

Subject: Re: [patch] x86, bts: fix unlock problem in ds.c

* stephane eranian <[email protected]> wrote:

> Markus,
>
> I think this patch is not quite right. You don't want to go out via
> out_unlock because you're going to call ds_put_context)() when you
> did not invoke the matching ds_get_context() (hidden in
> ds_alloc_context). It happens later in the ds_request() function.

i noticed that, and fixed it via:

10db4ef: x86, PEBS/DS: fix code flow in ds_request()

can you still see problems with it?

Ingo

2008-11-20 22:06:35

by Stephane Eranian

Subject: Re: [patch] x86, bts: fix unlock problem in ds.c

On Thu, Nov 20, 2008 at 10:48 PM, Ingo Molnar <[email protected]> wrote:
>
> * stephane eranian <[email protected]> wrote:
>
>> Markus,
>>
>> I think this patch is not quite right. You don't want to go out via
>> out_unlock because you're going to call ds_put_context)() when you
>> did not invoke the matching ds_get_context() (hidden in
>> ds_alloc_context). It happens later in the ds_request() function.
>
> i noticed that, and fixed it via:
>
> 10db4ef: x86, PEBS/DS: fix code flow in ds_request()
>
> can you still see problems with it?
>
Well, I have lots of problems with ds.c so I have heavily modified the
code therefore I have not pulled from
any public tree in several days.