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

[Greg KH <gregkh@suse.de>]

On Mon, Oct 12, 2009 at 08:10:40PM +0000, Hank Janssen wrote:
> 
> >Odd quoting style :(
> 
> We like to keep things lively :)
> 
> >> 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.
> 
> It should, but in practice in this case it does not seem to behave the same
> Way.

Can you figure out why?  What is the output of gcc for both ways?

Can you show what is fixed by this change?

Also note that #pragma packed is not supported by older versions of gcc,
so I don't think that it would work at all on some compiler versions
that are still legal to use for the kernel.  But I'm not quite sure when
it was added, so I might be wrong.

> >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?
> 
> It is something that I want to look at in the future. Our primary focus
> Is to get the bug fixed. We cannot do the offset way in the time we
> Have before 2.6.32 closes and still be comfortable we have gone through
> The extensive testing cycle we do on our side.

I can't take this patch until I see what the root problem is here,
sorry.

> >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?
> 
> Not sure yet if they need to be reverted, after we fixed this bug last week
> We are getting another one, it was masked by the one we just fixed. 
> We are checking into that right now;
> 
> 	BUG: unable to handle kernel NULL pointer dereference at (null)

What is the rest of the oops message?  That's pretty hard to determine
anything from :)

thanks,

greg k-h