Re: linux-next: Tree for Feb 2 (trace/events/sunrpc.h)

[Randy Dunlap <rdunlap@xenotime.net>]

On 02/08/2012 07:14 PM, Myklebust, Trond wrote:
> On Wed, 2012-02-08 at 21:37 -0500, Steven Rostedt wrote:
>> [ Added the person responsible for this ]
>>
>> On Thu, 2012-02-02 at 10:10 -0800, Randy Dunlap wrote:
>>> On 02/01/2012 07:45 PM, Stephen Rothwell wrote:
>>>> Hi all,
>>>>
>>>> Changes since 20120201:
>>>
>>>
>>>
>>> include/trace/events/sunrpc.h:69:1: error: implicit declaration of function 'rpc_qname'
>>> include/trace/events/sunrpc.h:69:1: warning: format '%s' expects type 'char *', but argument 9 has type 'int'
>>
>> This has actually nothing to do with the tracepoint itself. The bug is
>> with the rpc_qname().
>>
>> The tracepoint references rpc_qname() and in
>> include/linux/sunrpc/sched.h:
>>
>>
>> #ifdef RPC_DEBUG
>> static inline const char * rpc_qname(const struct rpc_wait_queue *q)
>> {
>> 	return ((q && q->name) ? q->name : "unknown");
>> }
>> #endif
>>
>> Your config had RPC_DEBUG not set, thus the function was not defined.
>>
>>
>> The below patch fixes the problem with the side effect that the trace
>> data will contain "unknown" for all references to rcu_qname().
>>
>> -- Steve
>>
>> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
>>
>>
>> diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h
>> index f7b2df5..c89ba95 100644
>> --- a/include/linux/sunrpc/sched.h
>> +++ b/include/linux/sunrpc/sched.h
>> @@ -275,6 +275,11 @@ static inline const char * rpc_qname(const struct rpc_wait_queue *q)
>>  {
>>  	return ((q && q->name) ? q->name : "unknown");
>>  }
>> +#else
>> +static inline const char * rpc_qname(const struct rpc_wait_queue *q)
>> +{
>> +	return "unknown";
>> +}
>>  #endif
>>  
>>  #endif /* _LINUX_SUNRPC_SCHED_H_ */
> 
> Hmm.... How about if we rather take that out of the RPC_DEBUG condition?
> I'm assuming that if someone compiles in the tracepoint code, then they
> want to be able to do a full trace independently of whether or not they
> set CONFIG_SYSCTL.
> 
> 8<---------------------------------------------------------------------
> From d051b60dcc3032b71cf8d9b96ac4bf24f12b6dcb Mon Sep 17 00:00:00 2001
> From: Trond Myklebust <Trond.Myklebust@netapp.com>
> Date: Wed, 8 Feb 2012 22:01:15 -0500
> Subject: [PATCH] SUNRPC: Ensure that we can trace waitqueues when
>  !defined(CONFIG_SYSCTL)
> 
> The tracepoint code relies on the queue->name being defined in order to
> be able to display the name of the waitqueue on which an RPC task is
> sleeping.
> 
> Reported-by: Randy Dunlap <rdunlap@xenotime.net>
> Reported-by: Steven Rostedt <rostedt@goodmis.org>
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>

Acked-by: Randy Dunlap <rdunlap@xenotime.net>

Thanks.

> ---
>  include/linux/sunrpc/debug.h |    3 +++
>  include/linux/sunrpc/sched.h |   15 +++++++++++++--
>  net/sunrpc/sched.c           |    4 +---
>  3 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/sunrpc/debug.h b/include/linux/sunrpc/debug.h
> index c2786f2..2a11eb2 100644
> --- a/include/linux/sunrpc/debug.h
> +++ b/include/linux/sunrpc/debug.h
> @@ -34,6 +34,9 @@
>  #ifdef CONFIG_SYSCTL
>  #define  RPC_DEBUG
>  #endif
> +#ifdef CONFIG_TRACEPOINTS
> +#define RPC_TRACEPOINTS
> +#endif
>  /* #define  RPC_PROFILE */
>  
>  /*
> diff --git a/include/linux/sunrpc/sched.h b/include/linux/sunrpc/sched.h
> index f7b2df5..22dfc24 100644
> --- a/include/linux/sunrpc/sched.h
> +++ b/include/linux/sunrpc/sched.h
> @@ -195,7 +195,7 @@ struct rpc_wait_queue {
>  	unsigned char		nr;			/* # tasks remaining for cookie */
>  	unsigned short		qlen;			/* total # tasks waiting in queue */
>  	struct rpc_timer	timer_list;
> -#ifdef RPC_DEBUG
> +#if defined(RPC_DEBUG) || defined(RPC_TRACEPOINTS)
>  	const char *		name;
>  #endif
>  };
> @@ -270,11 +270,22 @@ static inline int rpc_task_has_priority(struct rpc_task *task, unsigned char pri
>  	return (task->tk_priority + RPC_PRIORITY_LOW == prio);
>  }
>  
> -#ifdef RPC_DEBUG
> +#if defined(RPC_DEBUG) || defined (RPC_TRACEPOINTS)
>  static inline const char * rpc_qname(const struct rpc_wait_queue *q)
>  {
>  	return ((q && q->name) ? q->name : "unknown");
>  }
> +
> +static inline void rpc_assign_waitqueue_name(struct rpc_wait_queue *q,
> +		const char *name)
> +{
> +	q->name = name;
> +}
> +#else
> +static inline void rpc_assign_waitqueue_name(struct rpc_wait_queue *q,
> +		const char *name)
> +{
> +}
>  #endif
>  
>  #endif /* _LINUX_SUNRPC_SCHED_H_ */
> diff --git a/net/sunrpc/sched.c b/net/sunrpc/sched.c
> index d79c63d..1c570a8 100644
> --- a/net/sunrpc/sched.c
> +++ b/net/sunrpc/sched.c
> @@ -208,9 +208,7 @@ static void __rpc_init_priority_wait_queue(struct rpc_wait_queue *queue, const c
>  	queue->qlen = 0;
>  	setup_timer(&queue->timer_list.timer, __rpc_queue_timer_fn, (unsigned long)queue);
>  	INIT_LIST_HEAD(&queue->timer_list.list);
> -#ifdef RPC_DEBUG
> -	queue->name = qname;
> -#endif
> +	rpc_assign_waitqueue_name(queue, qname);
>  }
>  
>  void rpc_init_priority_wait_queue(struct rpc_wait_queue *queue, const char *qname)


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***