linux-kernel - Re: [PATCH 2/3] tracing/profile: add ref count for registering profile events

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Fri, 11 Sep 2009 10:33:26 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Peter Zijlstra <peterz@...radead.org>
Cc:	linux-kernel@...r.kernel.org, Ingo Molnar <mingo@...e.hu>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Li Zefan <lizf@...fujitsu.com>,
	Mathieu Desnoyers <compudj@...stal.dyndns.org>
Subject: Re: [PATCH 2/3] tracing/profile: add ref count for registering
 profile events

On Fri, 2009-09-11 at 16:09 +0200, Peter Zijlstra wrote:
> On Fri, 2009-09-11 at 16:04 +0200, Peter Zijlstra wrote:
> > On Fri, 2009-09-11 at 09:54 -0400, Steven Rostedt wrote:
> > 
> > > +#ifdef MODULE
> > > +# define event_trace_up_ref()					\
> > > +	do {							\
> > > +		if (!try_module_get(THIS_MODULE)) {		\
> > > +			atomic_dec(&event_call->profile_count);	\
> > > +			return -ENOENT;				\
> > > +		}						\
> > > +	} while (0)
> > > +# define event_trace_down_ref() module_put(THIS_MODULE)
> > > +#else
> > > +# define event_trace_up_ref() do { } while (0)
> > > +# define event_trace_down_ref() do { } while (0)
> > > +#endif
> > 
> > That's like truely gruesomely ugly.
> > 
> > At the very least write it like:
> > 
> > int event_trace_up_ref(struct ftrace_event_call *call)
> > {
> > 	if (!try_module_get(THIS_MODULE)) {
> > 		atomic_dev(&call->profile_count);
> > 		return -ENOENT;
> > 	}
> > 	return 0;
> > }
> 
> Or we can go with Li's original patch, that was less ugly.

I can go back to Li's original patch, but the talk on that was
"fragile". If you no longer feel that way, then I'll use his instead.

For now, I'll pull out this patch altogether, and resubmit the pull
request without it. I'd like the other changes to not be held up by
this.

> 
> I still think tracepoints/markers should sort this out, because we now
> have a sematic difference between the two wrt modules.

I originally tried to do it in the tracepoint logic, but that broke a
lot of assumptions about tracepoints that Mathieu pointed out. This is
not a normal use of tracepoints. It is expected that if you register a
probe in a module, you will unregister it before exiting.

I can't remember all the details, but at the end, it seemed that the fix
belonged at the ftrace level.

-- Steve


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/