Re: [PATCH] taskstats: Use better ifdef for alignment

Florian Mickler
2011-01-01T16:19:53Z

On Thu, 30 Dec 2010 10:52:01 -0500
Jeff Mahoney <jeffm@suse.com> wrote:

> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> On 12/30/2010 12:32 AM, Andrew Morton wrote:
> > On Thu, 30 Dec 2010 00:26:34 -0500 Jeff Mahoney <jeffm@suse.com> wrote:
> >
> >> On 12/29/2010 07:14 PM, Andrew Morton wrote:
> >>> On Wed, 29 Dec 2010 19:12:08 -0500 Jeff Mahoney <jeffm@suse.com> wrote:
> >>>
> >>>> Commit 4be2c95d added a null field to align the taskstats structure but
> >>>> the discussion centered around ia64. The issue exists on other platforms
> >>>> with inefficient unaligned access and adding them piecemeal would be
> >>>> an unmaintainable mess.
> >>>>
> >>>> This patch uses Dave Miller's suggestion of using a combination of
> >>>> CONFIG_64BIT && !CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS to determine
> >>>> whether alignment is needed.
> >>>>
> >>>> Note that this will cause breakage on those platforms with applications
> >>>> like iotop which had hard-coded offsets into the packet to access the
> >>>> taskstats structure.
> >>>
> >>> That seems a very good reason to not apply the patch.
> >>>
> >>> Tell us more, please...
> >>
> >> I don't want to rehash the same discussion
> >
> > Please do so. That discussion went on for a long time over many emails
> > and multiple iterations of the patch. I personally have forgotten the
> > reasoning and if I could remember it, I wouldn't remember which version
> > of the patch it applied to.
> >
> > Applying a patch which is *known* to break *known* userspace
> > applications is a quite extraordinary thing to do. We owe it to people
> > to fully explain the reasoning.
>
> Ok, so the gist is that iotop makes what I'd call unreasonable
> assumptions about the contents of a netlink genetlink packet containing
> generic attributes.

Is there already a patch available or integrated into iotop which
fixes this? I'd think that if the kernel could wait on fixed iotop's to
be distributed it would be easier to sell the breakage on bugzilla &
co...

Just my 2 cents, but I don't know enough to weigh the different aspects
of this... iotop shurely is not a showstopper if the upsides of this
patch are big enough?

Regards,
Flo