Re: types for storing instruction pointers

[Christoph Hellwig <hch@lst.de>]

On Sun, Jun 07, 2009 at 03:25:16PM +0200, Ingo Molnar wrote:
> 
> * Christoph Hellwig <hch@lst.de> wrote:
> 
> > Currently the _THIS_IP_ and _RET_IP_ macros aded for lockdep but 
> > now available from kernel.org case the instruction pointer to an 
> > unsigned long.  But the %pf/%pF format for printing them want a 
> > pointer of some sort.  That's a pretty nasty situation for tracers 
> > - can we standardize on one type for it?
> 
> Yes but what's the practical problem exactly? Could you cite an 
> example?

The simplest tracer in xfs showing this is the following:

/*
 * ilock/iolock tracer
 *
 * Reports the inode, operation, flags and caller for each operation
 * on the inode locks.
 */
TRACE_EVENT(xfs_ilock,
	TP_PROTO(struct xfs_inode *ip, const char *op, unsigned lockflags,
		 unsigned long caller_ip),
	TP_ARGS(ip, op, lockflags, caller_ip),

	TP_STRUCT__entry(
		__field(dev_t, dev)
		__field(xfs_ino_t, ino)
		__field(const char *, op)
		__field(int, lockflags)
		__field(unsigned long, caller_ip)
	),

	TP_fast_assign(
		__entry->dev = VFS_I(ip)->i_sb->s_dev;
		__entry->ino = ip->i_ino;
		__entry->op = op;
		__entry->lockflags = lockflags;
		__entry->caller_ip = caller_ip;
	),

	TP_printk("dev %d:%d ino 0x%lld %s %s by %pF",
		  MAJOR(__entry->dev), MINOR(__entry->dev),
		  __entry->ino,
		  __entry->op,
		  __print_flags(__entry->lockflags, "|", XFS_LOCK_FLAGS),
		  (void *)__entry->caller_ip)
);

It has the following callers:

	trace_xfs_ilock(ip, "lock", lock_flags, _RET_IP_);
	trace_xfs_ilock(ip, "lock_nowait", lock_flags, _RET_IP_);
	trace_xfs_ilock(ip, "unlock", lock_flags, _RET_IP_);
	trace_xfs_ilock(ip, "demote", lock_flags, _RET_IP_);

Basically everything obtained via _RET_IP_/_THIS_IP_ needs to be
casted.  Given that both need to case the their return value to a
pointer that's rather unfortunately.  Life would be much easier
if _RET_IP_/_THIS_IP_ just returned a pointer (probably just a void
pointer, maybe with a fancy typedef to make it clear we're dealing
with an instruction pointer here).