linux-trace-devel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Steven Rostedt <rostedt@goodmis.org>
To: "Yordan Karadzhov (VMware)" <y.karadz@gmail.com>
Cc: linux-trace-devel@vger.kernel.org
Subject: Re: [PATCH 4/4] libtracefs: Propagate the return value of the callback function
Date: Wed, 9 Jun 2021 13:00:18 -0400	[thread overview]
Message-ID: <20210609130018.2dd877ef@oasis.local.home> (raw)
In-Reply-To: <20210608135503.12135-4-y.karadz@gmail.com>

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

-- Steve

  reply	other threads:[~2021-06-09 17:00 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 [this message]
2021-06-11 11:03     ` Yordan Karadzhov (VMware)

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=20210609130018.2dd877ef@oasis.local.home \
    --to=rostedt@goodmis.org \
    --cc=linux-trace-devel@vger.kernel.org \
    --cc=y.karadz@gmail.com \
    --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).