Re: [patch] Staging: hv: Fix vmbus load hang caused by wrong datapacking

[Greg KH]

On Mon, Oct 12, 2009 at 05:03:42PM +0000, Haiyang Zhang wrote:
> >  	ChannelMessageCount
> > -} __attribute__((packed));
> 
> Why the change here?  Isn't this doing the same thing?
> 
> And I'm guessing that not all of these structures are needing to be
> packed, right?  Are they all shared across the HV boundry?
> 
> And is this fixing the problem that Hank and users have reported with
> the current code?  Does this need to get into the 2.6.32 release?
> 
> 
> thanks,
> 
> greg k-h
> ================================================
> Hi Greg,

Odd quoting style :(

> Based on our testing, the #pragma pack(push,1) can pack the data
> correctly for the HyperV to use, but __attribute__((packed)) couldn't
> do this right.

Why?  What does gcc generate differently?  This should be identical.

> These data structures are moved by someone from the original file,
> ChannelMessages.h, which contains structures used for messaging to
> host.

I moved them as they did not need to be in that file, right?

And are all of these structures needing to be packed?

Ideally, we don't deal with packed structures at all, but with offsets
in memory and pick out the proper fields and put them into new
structures if you want to use them that way.  How hard would that be to
do here instead?

> Yes, it's fixing the problem that Hank and users have reported with
> the current code. And yes, 2.6.32 needs this fix.

I still want to figure out what the real difference here is.  Especially
as I removed a lot of the #pragma pack(push,1) lines from the hv code.
If it really is different, all of those patches should be reverted,
right?

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/