Re: [patch] delayacct: fix iotop on x86_64

[Balbir Singh <balbir@linux.vnet.ibm.com>]

* Dan Carpenter <error27@gmail.com> [2010-12-14 10:02:43]:

> We changed how the taskstats was exported to user space in:
> 85893120699 "delayacct: align to 8 byte boundary on 64-bit systems"
> This was important because it fixes a run time warning on IA64.  In
> theory it shouldn't have broken anything, if you just assume that user
> space programmers don't smoke crack all day long.
> 
> But actually it breaks iotop on x86_64.
> 
> Reported-by: Brian Rogers <brian@xyzw.org>
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> diff --git a/kernel/taskstats.c b/kernel/taskstats.c
> index c8231fb..a0758de 100644
> --- a/kernel/taskstats.c
> +++ b/kernel/taskstats.c
> @@ -358,7 +358,19 @@ static struct taskstats *mk_reply(struct sk_buff *skb, int type, u32 pid)
>  	 * This causes lots of runtime warnings on systems requiring 8 byte
>  	 * alignment */
>  	u32 pids[2] = { pid, 0 };
> -	int pid_size = ALIGN(sizeof(pid), sizeof(long));
> +	int pid_size;
> +
> +	/*
> +	 * IA64 can't be aligned on a 4 byte boundary.  But iotop on x86_64
> +	 * depends on the current struct layout.  The next version of iotop
> +	 * will fix this so maybe we can move everything to the new code in
> +	 * a couple years.
> +	 */
> +#if defined(CONFIG_IA64)
> +	pid_size = ALIGN(sizeof(pid), sizeof(long));
> +#else
> +	pid_size = sizeof(u32);
> +#endif

I would rather abstract this better and I'd be apprehensive about the
fix if iotop was at fault to begin with, I would rather fix iotop.
IOW, are we fixing what iotop got wrong? Isn't it easier to backport
the correct behaviour in iotop. I understand we broke the ABI, but
user space can still live.

> 
>  	aggr = (type == TASKSTATS_TYPE_PID)
>  			? TASKSTATS_TYPE_AGGR_PID

-- 
	Three Cheers,
	Balbir