linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Yordan Karadzhov (VMware)" <y.karadz@gmail.com>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: linux-trace-devel@vger.kernel.org
Subject: Re: [PATCH 4/4] libtracefs: Propagate the return value of the callback function
Date: Fri, 11 Jun 2021 14:03:00 +0300	[thread overview]
Message-ID: <b218bb7c-6546-ed02-f871-dff3acf79d2f@gmail.com> (raw)
In-Reply-To: <20210609130018.2dd877ef@oasis.local.home>



On 9.06.21 г. 20:00, Steven Rostedt wrote:
> [ Sending again, but this time with "Reply-all" to include the mailing list ]
> 
> On Tue,  8 Jun 2021 16:55:03 +0300
> "Yordan Karadzhov (VMware)" <y.karadz@gmail.com> wrote:
> 
>> Currently the return value of the callback function can be used to
>> stop pulling data from the trace buffer, however this return value
>> is lost and the user has no idea if tracefs_iterate_raw_events()
>> terminated because there was no more data or because this was
>> requested from the callback function. If we propagate the return
>> value of the callback, this can be used in cases like the one below:
>>
>> int callback(struct tep_event *event, struct tep_record *record,
>> 	     int cpu, void *py_func)
>> {
>> 	....
>>
>> 	return (something) ? 0 : 1
>> }
>>
>> int main()
>> {
>> 	int ret;
>>
>> 	....
>>
>> 	while(ret == 0)
>> 		ret = tracefs_iterate_raw_events(tep, instance,
>> 		                                 NULL, 0,
>> 		                                 callback, NULL);
>>
>> 	....
>>
>> Here the user can effectively terminate the pulling the data
>> from inside the callback.
> 
> With this change, the user can't tell if returned -1 due to an error or
> because the callback ended early, (which is not considered an error).
> 
> The proper way for an application to handle this, is to pass in a
> context structure, and have the callback set a value if it exits early
> or not. I've done this already for needing the same information.
> 
> No change to libtracefs is needed. This functionality is already
> available with the current design:
> 
> struct my_struct {
> 	bool	stopped;
> };
> 
> int callback(struct tep_event *event, struct tep_record *record,
> 		    int cpu, void *data)
> {
> 	struct my_struct *my_data = data;
> 
> 	[..]
> 	if (condition) {
> 		my_data->stopped = true;
> 		return 1;
> 	}
> 	return 0;
> }
> 
> int main()
> {
> 	struct my_struct my_data = { .stopped = false };
> 	int ret = 0;
> 
> 	while (!ret && !my_data.stopped)
> 		ret = tracefs_iterate_raw_events(tep, instance, NULL, 0,
> 					callback, &my_data);
> }
> 
> Now, if you are using this within python, and you want the python
> wrapper to pass data as well, you just need to add that to the struct:
> 
> struct my_struct {
> 	bool		stopped;
> 	func		*python_callback;
> 	void		*python_data;
> }
> 
> int callback(struct tep_event *event, struct tep_record *record,
> 		    int cpu, void *data)
> {
> 	struct my_struct *my_data = data;
> 	int ret;
> 
> 	[..]
> 	ret = my_data->python_callback(event, record, cpu, data->python_data);
> 
> 	if (ret) {
> 		my_data->stopped = true;
> 		return 1;
> 	}
> 	return 0;
> }
> 
> int python_iterator(pthyon_callback, python_data)
> {
> 	struct my_struct my_data = { .stopped = false };
> 	int ret = 0;
> 
> 	my_data.python_data = python_data;
> 	my_data.python_callback = python_callback;
> 
> 	while (!ret && !my_data.stopped)
> 		ret = tracefs_iterate_raw_events(tep, instance, NULL, 0,
> 					callback, &my_data);
> }
> 
> So this patch isn't needed.

I agree. Your solution is better.

Thanks!
Yordan

> 
> -- Steve
> 

      reply	other threads:[~2021-06-11 11:04 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-08 13:55 [PATCH 1/4] libtracefs: Fix enable_disable_all() return value Yordan Karadzhov (VMware)
2021-06-08 13:55 ` [PATCH 2/4] libtracefs: Fix event_enable_disable() " Yordan Karadzhov (VMware)
2021-06-08 13:55 ` [PATCH 3/4] libtracefs: Fix typo in function name Yordan Karadzhov (VMware)
2021-06-08 13:55 ` [PATCH 4/4] libtracefs: Propagate the return value of the callback function Yordan Karadzhov (VMware)
2021-06-09 17:00   ` Steven Rostedt
2021-06-11 11:03     ` Yordan Karadzhov (VMware) [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b218bb7c-6546-ed02-f871-dff3acf79d2f@gmail.com \
    --to=y.karadz@gmail.com \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --subject='Re: [PATCH 4/4] libtracefs: Propagate the return value of the callback function' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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).