Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753847Ab0LORme (ORCPT ); Wed, 15 Dec 2010 12:42:34 -0500 Received: from e28smtp06.in.ibm.com ([122.248.162.6]:57224 "EHLO e28smtp06.in.ibm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752777Ab0LORmb (ORCPT ); Wed, 15 Dec 2010 12:42:31 -0500 Date: Wed, 15 Dec 2010 12:40:18 +0530 From: Balbir Singh To: Andrew Morton Cc: Dan Carpenter , Brian Rogers , Jeff Mahoney , linux-kernel@vger.kernel.org, Guillaume Chazarain Subject: Re: [patch] delayacct: fix iotop on x86_64 Message-ID: <20101215071018.GD2657@balbir.in.ibm.com> Reply-To: balbir@linux.vnet.ibm.com References: <20101213113745.GE1620@bicker> <20101213125709.GB14178@balbir.in.ibm.com> <4D0639B2.4060903@suse.com> <4D068869.7040305@xyzw.org> <20101213212253.GI1620@bicker> <20101214070243.GJ1620@bicker> <20101214080239.GC14178@balbir.in.ibm.com> <20101214121641.7c337509.akpm@linux-foundation.org> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <20101214121641.7c337509.akpm@linux-foundation.org> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2757 Lines: 75 * Andrew Morton [2010-12-14 12:16:41]: > On Tue, 14 Dec 2010 13:32:39 +0530 > Balbir Singh wrote: > > > * Dan Carpenter [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 > > > Signed-off-by: Dan Carpenter > > > > > > 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 > > Well. Abstracting something tends to make it permanent. When you have > an ugly, special-case temporary hack, there is merit to having it > sitting there in the middle of the code staring you in the face. It's > very explicit and we won't forget about it. > OK, agreed and learnt > > 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. > > Nah, let's not knowingly break a userspace app. > Fair enough! > > This is a versioned interface, is it not? How is that supposed > to work? Should we have upped the version number when making this > change? -- Three Cheers, Balbir -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/