trace: module: Maintain a valid user count

PATCH Superseded Romain Izard 3 participants 8 messages Mar 3, 2014 – May 8, 2014 Tracing

Share & tools

Romain Izard

author

2014-03-03 18:18 UTC #0 lore raw

PATCH 1/1 trace: module: Maintain a valid user count

The replacement of the 'count' variable by two variables 'incs' and 'decs' to resolve some race conditions during module unloading was done in parallel with some cleanup in the trace subsystem, and was integrated as a merge.

Unfortunately, the formula for this replacement was wrong in the tracing code, and the refcount in the traces was not usable as a result.

Use 'count = incs - decs' to compute the user count.

Signed-off-by Romain Izard <[email protected]>

---
 include/trace/events/module.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

include/trace/events/module.h +2 -1

index 161932737416..1228d963b6d1 100644
@@ -78,7 +78,8 @@ DECLARE_EVENT_CLASS(module_refcnt,
 
 	TP_fast_assign(
 		__entry->ip	= ip;
-		__entry->refcnt	= __this_cpu_read(mod->refptr->incs) + __this_cpu_read(mod->refptr->decs);
+		__entry->refcnt	= __this_cpu_read(mod->refptr->incs) -
+			__this_cpu_read(mod->refptr->decs);
 		__assign_str(name, mod->name);
 	),

Signature · 1 line

1.8.3.2

Steven Rostedt

maintainer

2014-03-03 18:33 UTC #1 lore raw

Quoted 20 lines · On Mon, 3 Mar 2014 19:17:32 +0100 Romain Izard <[email protected]>…

The replacement of the 'count' variable by two variables 'incs' and 'decs' to resolve some race conditions during module unloading was done in parallel with some cleanup in the trace subsystem, and was integrated as a merge.
Unfortunately, the formula for this replacement was wrong in the tracing code, and the refcount in the traces was not usable as a result.
Use 'count = incs - decs' to compute the user count.
Signed-off-by Romain Izard <[email protected]>
---
 include/trace/events/module.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
include/trace/events/module.h +0 -0
index 161932737416..1228d963b6d1 100644

include/trace/events/module.h DECLARE_EVENT_CLASS(module_refcnt, · line 82

Quoted 6 lines

include/trace/events/module.h +2 -1

@@ -78,7 +78,8 @@ DECLARE_EVENT_CLASS(module_refcnt,
 
 	TP_fast_assign(
 		__entry->ip	= ip;
-		__entry->refcnt	= __this_cpu_read(mod->refptr->incs) + __this_cpu_read(mod->refptr->decs);
+		__entry->refcnt	= __this_cpu_read(mod->refptr->incs) -
+			__this_cpu_read(mod->refptr->decs);

I know this breaks the 80 column rule, but please keep it on one line. Look at the above. The line you are removing is much easier to read than the one you replaced it with.

Or if it is too big, what about:

		__entry->refcnt	=
		  __this_cpu_read(mod->refptr->incs) - __this_cpu_read(mod->refptr->decs);

Thanks,

-- Steve

Quoted 3 lines

include/trace/events/module.h +0 -0
 		__assign_str(name, mod->name);
 	),
 

Romain Izard

author

2014-03-04 09:10 UTC #2 lore raw

PATCH v2 1/1 trace: module: Maintain a valid user count

Unfortunately, the formula for this replacement was wrong in the tracing code, and the refcount in the traces was not usable as a result.

Use 'count = incs - decs' to compute the user count.

Signed-off-by Romain Izard <[email protected]>

---
 include/trace/events/module.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

include/trace/events/module.h +1 -1

index 161932737416..ca298c7157ae 100644
@@ -78,7 +78,7 @@ DECLARE_EVENT_CLASS(module_refcnt,
 
 	TP_fast_assign(
 		__entry->ip	= ip;
-		__entry->refcnt	= __this_cpu_read(mod->refptr->incs) + __this_cpu_read(mod->refptr->decs);
+		__entry->refcnt	= __this_cpu_read(mod->refptr->incs) - __this_cpu_read(mod->refptr->decs);
 		__assign_str(name, mod->name);
 	),

Signature · 1 line

1.8.3.2

Steven Rostedt

maintainer Reviewed-by on v2 1/1

2014-03-06 17:34 UTC #3 lore raw

Ingo,

You want to push this to Linus?

Reviewed-by Steven Rostedt <[email protected]>

-- Steve

Quoted 28 lines · On Tue, 4 Mar 2014 10:09:39 +0100 Romain Izard <[email protected]>…

The replacement of the 'count' variable by two variables 'incs' and 'decs' to resolve some race conditions during module unloading was done in parallel with some cleanup in the trace subsystem, and was integrated as a merge.
Unfortunately, the formula for this replacement was wrong in the tracing code, and the refcount in the traces was not usable as a result.
Use 'count = incs - decs' to compute the user count.
Signed-off-by Romain Izard <[email protected]>
---
 include/trace/events/module.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/trace/events/module.h +1 -1
index 161932737416..ca298c7157ae 100644
@@ -78,7 +78,7 @@ DECLARE_EVENT_CLASS(module_refcnt,
 
 	TP_fast_assign(
 		__entry->ip	= ip;
-		__entry->refcnt	= __this_cpu_read(mod->refptr->incs) + __this_cpu_read(mod->refptr->decs);
+		__entry->refcnt	= __this_cpu_read(mod->refptr->incs) - __this_cpu_read(mod->refptr->decs);
 		__assign_str(name, mod->name);
 	),
 

Romain Izard

author

2014-05-07 10:19 UTC #4 lore raw

2014-03-06 18:34 GMT+01:00 Steven Rostedt <[email protected]>:

Quoted 33 lines

Ingo,
You want to push this to Linus?
Reviewed-by Steven Rostedt <[email protected]>
-- Steve
Quoted 21 lines · On Tue, 4 Mar 2014 10:09:39 +0100 Romain Izard <[email protected]>…
The replacement of the 'count' variable by two variables 'incs' and 'decs' to resolve some race conditions during module unloading was done in parallel with some cleanup in the trace subsystem, and was integrated as a merge.
Unfortunately, the formula for this replacement was wrong in the tracing code, and the refcount in the traces was not usable as a result.
Use 'count = incs - decs' to compute the user count.
Signed-off-by Romain Izard <[email protected]>
---
 include/trace/events/module.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/trace/events/module.h +0 -0
index 161932737416..ca298c7157ae 100644
@@ -78,7 +78,7 @@ DECLARE_EVENT_CLASS(module_refcnt,
 

Quoted 8 lines

include/trace/events/module.h DECLARE_EVENT_CLASS(module_refcnt, · line 84

Quoted 7 lines

include/trace/events/module.h +1 -1

      TP_fast_assign(
              __entry->ip     = ip;
-             __entry->refcnt = __this_cpu_read(mod->refptr->incs) + __this_cpu_read(mod->refptr->decs);
+             __entry->refcnt = __this_cpu_read(mod->refptr->incs) - __this_cpu_read(mod->refptr->decs);
              __assign_str(name, mod->name);
      ),

Ping ?

Ingo Molnar

2014-05-07 19:23 UTC #5 lore raw

In reply to Steven Rostedt

Quoted 5 lines · Steven Rostedt

Ingo,
You want to push this to Linus?
Reviewed-by Steven Rostedt <[email protected]>

I think it would be more natural through the tracing tree, through which most tracepoint fixes go, agreed?

Thanks,

Ingo

Steven Rostedt

maintainer

2014-05-07 19:41 UTC #6 lore raw

Quoted 12 lines · On Wed, 7 May 2014 21:23:36 +0200 Ingo Molnar <[email protected]> wrote:

Quoted 5 lines · Steven Rostedt
Ingo,
You want to push this to Linus?
Reviewed-by Steven Rostedt <[email protected]>
I think it would be more natural through the tracing tree, through which most tracepoint fixes go, agreed?

Hmm, yeah. Not sure why I pushed it off to you :-/

Looks like this should be marked as stable and incorporated in 3.15 as well.

Thanks,

-- Steve

Ingo Molnar

Acked-by

2014-05-08 06:14 UTC #7 lore raw

Quoted 10 lines · Steven Rostedt

Quoted 7 lines · On Wed, 7 May 2014 21:23:36 +0200 Ingo Molnar <[email protected]> wrote:
Quoted 4 lines · Steven Rostedt
Ingo,
You want to push this to Linus?

Quoted 7 lines

Quoted 5 lines
Quoted 1 line
Reviewed-by Steven Rostedt <[email protected]>
I think it would be more natural through the tracing tree, through which most tracepoint fixes go, agreed?
Hmm, yeah. Not sure why I pushed it off to you :-/

To check whether I'm awake, I'm sure!

Quoted 2 lines

Looks like this should be marked as stable and incorporated in 3.15 as well.

Yeah.

Acked-by Ingo Molnar <[email protected]>

Thanks,

Ingo