[Intel-gfx] [PATCH v4 2/5] drm: Add private data field to trace control block

[Patrik Jakobsson]

On Wed, Aug 26, 2015 at 03:26:08PM +0200, Patrik Jakobsson wrote:
> On Tue, Aug 25, 2015 at 11:12 PM, Mike Frysinger <vapier at gentoo.org> wrote:
> > On 24 Aug 2015 14:42, Patrik Jakobsson wrote:
> >> We need to be able to store private data in the tcb across it's
> >> lifetime. To ensure proper destruction of the data a free_priv_data
> >> callback must be provided if an allocation is stored in priv_data. The
> >> callback is executed automatically when the life of the tcb ends.
> >>
> >> * defs.h: Add extern declaration of free_tcb_priv_data.
> >>  (struct tcb): Add priv_data and free_priv_data.
> >> * strace.c (free_tcb_priv_data): New function
> >> (drop_tcb): Execute free_tcb_priv_data callback
> >> * syscall.c (trace_syscall_exiting): Execute free_tcb_priv_data callback
> >>
> >> Signed-off-by: Patrik Jakobsson <patrik.jakobsson at linux.intel.com>
> >> ---
> >>  defs.h    |  6 ++++++
> >>  strace.c  | 14 ++++++++++++++
> >>  syscall.c |  1 +
> >>  3 files changed, 21 insertions(+)
> >>
> >> diff --git a/defs.h b/defs.h
> >> index 9059026..bc3bd83 100644
> >> --- a/defs.h
> >> +++ b/defs.h
> >> @@ -266,6 +266,10 @@ struct tcb {
> >>       int u_error;            /* Error code */
> >>       long scno;              /* System call number */
> >>       long u_arg[MAX_ARGS];   /* System call arguments */
> >> +
> >> +     void *priv_data;        /* Private data for syscall decoding functions */
> >> +     void (*free_priv_data)(void *); /* Callback for freeing priv_data */
> >
> > should we name these _priv_data and _free_priv_data and provides accessor
> > functions ?  i worry that code paths might stomp on each other by accident
> > and we don't end up noticing.
> >
> > static void set_tcb_priv_data(struct tcb *tcp, void *data, void (*free_data)(void *))
> > {
> >         assert(tcp->_priv_data == NULL && tcp->_free_priv_data == NULL);
> >         ...
> > }
> > -mike
> 
> Yes, that's a good idea. My use case is pretty simple but usage can easliy grow.
> 
> I'll resend this patch and take it out of the drm/i915 series.
> 
> -Patrik

Here's my take on it (I assume it needs some discussion):

int
set_tcb_priv_data(struct tcb *tcp, void *priv_data)
{
	/* A free callback is required before setting private data and private
	 * data must be set back to NULL before being set again.
	 */
	if (tcp->_free_priv_data == NULL ||
	    (tcp->_priv_data && priv_data != NULL))
		return -1;

	tcp->_priv_data = priv_data;
	return 0;
}

void *
get_tcb_priv_data(struct tcb *tcp)
{
	return tcp->_priv_data;
}

int
set_tcb_free_priv_data(struct tcb *tcp, void (*free_priv_data)(void *))
{
 	/* _free_priv_data must be set back to NULL before being set again. */
	if (tcp->_free_priv_data && free_priv_data != NULL)
		return -1;

	tcp->_free_priv_data = free_priv_data;
	return 0;
}

void
free_tcb_priv_data(struct tcb *tcp)
{
	if (tcp->_priv_data) {
		if (tcp->_free_priv_data) {
			tcp->_free_priv_data(tcp->_priv_data);
			tcp->_free_priv_data = NULL;
		}
		tcp->_priv_data = NULL;
	}
}

Cheers
Patrik