On 2018-12-12, Brian Foster <bfoster@xxxxxxxxxx> wrote: > On Tue, Dec 11, 2018 at 03:20:23PM -0500, Nick Bowler wrote: >> Right after I write this, I realize that it's almost certainly >> the case that architectures which _don't_ define BROKEN_X86_ALIGNMENT >> will have matching ioctl numbers between e.g., XFS_IOC_ALLOCSP and >> XFS_IOC_ALLOCSP_32. [...] > So the two struct [compat_]xfs_flock64 variants are essentially the same > with the exception of internal alignment padding that leaves a hole > after the first 4 bytes in the 64-bit variant. So on x86_64, these are > essentially two different (sized) structures and ioctl commands. On some > non-broken alignment arch, the packing may be implied and thus these > definitions evaluate down to the same ioctl command. If so, then having > those separate but equivalent value commands in the same switch > statement results in a compilation error. Am I following that correctly? > > If so, then it does seem we need an ifdef to exlude those definitions so > long as we follow the one huge switch statement implementation. But is > there anything that fundamentally prevents a multiple switch statement > implementation to avoid such syntax errors? I think nothing fundamentally prevents this approach and it is an option. The only point I see is that, without the configuration ifdefs, the impact of the change is expanded; there are (presumably benign) functional changes on more architectures, most of which I don't have the ability to test. Making it dependent on the arch configuration means nothing should change anywhere except on amd64 kernels that have x32 binary support turned on. > Note that in the end I don't care too much about whether we have an > ifdef or not. I'm more interested in the most simple and elegant > implementation and it just seemed that trying to dilute the existing > ifdef may push us in the opposite direction (as opposed to something > that for example called into the "native" ioctl and fell back to the _32 > variants on failure before returning an error). There may be technical > complications to that or using some form of the ifdef may just end up > being the more simple approach after all. OK, since I have some test results I'll submit the current one for RFC; afterwards I'll try to follow up with a less ifdeffy variant for comparison purposes, then we can decide which version is better. :) Cheers, Nick
