From: Felix Janda <felix.janda@posteo.de>
To: Dave Chinner <david@fromorbit.com>
Cc: Christoph Hellwig <hch@infradead.org>, xfs@oss.sgi.com
Subject: Re: [PATCH xfsprogs 2/2] linux.h: Define xfs_off_t as int64_t
Date: Sun, 7 Aug 2016 09:09:49 +0200 [thread overview]
Message-ID: <20160807070949.GA24018@nyan> (raw)
In-Reply-To: <20160806231802.GN16044@dastard>
Dave Chinner wrote:
> On Sat, Aug 06, 2016 at 10:38:52AM +0200, Felix Janda wrote:
> > Dave Chinner wrote:
[snip]
> > > If you want to clean this up, then remove the dependence on
> > > _LARGEFILE64_SOURCE in the entire xfsprogs code base (e.g. it uses
> > > lseek64 everywhere which requires off64_t to be defined) and instead
> > > make it dependent on _FILEOFFSET_BITS=64. Then you can get rid of
> > > all the uses of off64_t completely, and we can break the build if
> > > _FILEOFFSET_BITS != 64 on inclusion of xfs.h.
> >
> > Yes, I'd like to clean this up.
> >
> > But first note that you can have both _FILE_OFFSET_BITS=64 and
> > _LARGEFILE64_SOURCE. Then everything (off64_t, lseek64, ...) is
> > defined and everything (off_t, lseek, ...) is 64bit.
> >
> > So to clean up I would first get _FILE_OFFSET_BITS=64 defined and then
> > start "removing 64" from functions/types in any order. *Before
> > modifying the public headers* the sizeof(off_t)=8 check needs to
> > be put into xfs.h.
> >
> > Also note that there are 3 different (but equivalent) off_t types
> > currently used in the code base: off64_t, loff_t and xfs_off_t.
> > Should these be converted to xfs_off_t or off_t?
>
> Not that simple. loff_t has to remain for the copy_file_range()
> syscall in xfs_io. That syscall requires _GNU_SOURCE and loff_t to
> be defined from the system headers, so it can't really go away.
loff_t is the kernel name for the 64 bit offset type. (The kernel
needs to distinguish between off_t and loff_t because it needs to
export both 32bit and 64bit syscalls.) Since we are in userspace, we
can use the user space off_t, which coincides with kernel loff_t.
> xfs_off_t is an internal XFS file offset definition, used by the
> code in libxfs/ and shared with the kernel code, so it can't go
> away, either.
>
> So, essentially, the only code that should change is all
> the code that uses off64_t - that can use off_t as that's what
> all the systems that use those variables require...
ok.
> > Still, doing these type conversions is going to be pretty invasive
> > and is not unlikely to conflict with outstanding patches. Is now
> > a good time for this? (How about the __uint -> uint, __int -> int
> > conversion?)
>
> off64_t -> off_t affects very little of the new code we have
> outstanding. It mostly affects xfs_io, so there's little to worry
> about in terms of merge conflicts here.
will send a patch series soon.
> The __*int conversions are a different matter. They affect the
> entire code base - they are widespread through the libxfs code so we
> need to do a kernel code conversion first. Then we can propagate
> that back into the libxfs code in xfsprogs, and then the rest of
> xfsprogs can be done.
ok, I looked into the kernel. The only user of __uint*_t and __int*_t
are inside fs/xfs. I can prepare a patch for that.
Thanks,
Felix
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
prev parent reply other threads:[~2016-08-07 7:10 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-30 13:37 [PATCH xfsprogs 2/2] linux.h: Define xfs_off_t as int64_t Felix Janda
2016-07-30 16:36 ` Eric Sandeen
2016-08-01 6:24 ` Christoph Hellwig
2016-08-01 6:54 ` Felix Janda
2016-08-04 0:47 ` Dave Chinner
2016-08-05 8:02 ` Felix Janda
2016-08-05 11:52 ` Dave Chinner
2016-08-05 13:09 ` Felix Janda
2016-08-05 22:44 ` Dave Chinner
2016-08-06 8:38 ` Felix Janda
2016-08-06 9:13 ` Felix Janda
2016-08-06 23:18 ` Dave Chinner
2016-08-07 7:09 ` Felix Janda [this message]
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=20160807070949.GA24018@nyan \
--to=felix.janda@posteo.de \
--cc=david@fromorbit.com \
--cc=hch@infradead.org \
--cc=xfs@oss.sgi.com \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).