All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nick Bowler <nbowler@draconx.ca>
To: Brian Foster <bfoster@redhat.com>
Cc: Dave Chinner <david@fromorbit.com>,
	"Darrick J. Wong" <darrick.wong@oracle.com>,
	linux-xfs@vger.kernel.org
Subject: Re: Enlarging w/ xfs_growfs: XFS_IOC_FSGROWFSDATA xfsctl failed: Inappropriate ioctl for device
Date: Wed, 12 Dec 2018 19:21:58 -0500	[thread overview]
Message-ID: <CADyTPEwXO3OvrvP3wPNT2RGhi4FgkbA75=Dhg6u1uh6vmra8Zg@mail.gmail.com> (raw)
In-Reply-To: <20181212130922.GA23261@bfoster>

On 2018-12-12, Brian Foster <bfoster@redhat.com> 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

  reply	other threads:[~2018-12-13  0:22 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-12-10  4:29 Enlarging w/ xfs_growfs: XFS_IOC_FSGROWFSDATA xfsctl failed: Inappropriate ioctl for device Nick Bowler
2018-12-10 14:33 ` Brian Foster
2018-12-10 15:39   ` Nick Bowler
2018-12-10 16:11     ` Brian Foster
2018-12-10 16:50       ` Darrick J. Wong
2018-12-10 16:55         ` Darrick J. Wong
2018-12-10 17:46         ` Brian Foster
2018-12-10 20:54           ` Nick Bowler
2018-12-10 21:41             ` Dave Chinner
2018-12-11  7:04               ` Nick Bowler
2018-12-11 12:27                 ` Brian Foster
2018-12-11 20:13                   ` Nick Bowler
2018-12-11 20:20                     ` Nick Bowler
2018-12-12 13:09                       ` Brian Foster
2018-12-13  0:21                         ` Nick Bowler [this message]
2018-12-12  4:56                   ` Nick Bowler
2018-12-13  3:53                     ` Dave Chinner
2018-12-13  4:14                       ` Nick Bowler
2018-12-13  4:49                         ` Nick Bowler
2018-12-13 21:39                           ` Dave Chinner
2018-12-13 21:53                             ` Nick Bowler
2018-12-14  1:43                               ` Dave Chinner
2018-12-14  3:35                             ` Nick Bowler
2018-12-14  3:40                               ` [RFC PATCH xfstests] xfs: add tests to validate ioctl structure layout Nick Bowler
2019-01-15 15:55                                 ` Luis Chamberlain
2018-12-13 16:30                       ` Enlarging w/ xfs_growfs: XFS_IOC_FSGROWFSDATA xfsctl failed: Inappropriate ioctl for device Darrick J. Wong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CADyTPEwXO3OvrvP3wPNT2RGhi4FgkbA75=Dhg6u1uh6vmra8Zg@mail.gmail.com' \
    --to=nbowler@draconx.ca \
    --cc=bfoster@redhat.com \
    --cc=darrick.wong@oracle.com \
    --cc=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.