xfs_check_ondisk_structs() verifies that the sizes of the data types
used by xfs are correct via the XFS_CHECK_STRUCT_SIZE() macro.
xfs_dir2_sf_entry_t size is set erroneously to 3 which breaks the
compilation with the assertion below:
In file included from linux/include/linux/string.h:6,
from linux/include/linux/uuid.h:12,
from linux/fs/xfs/xfs_linux.h:10,
from linux/fs/xfs/xfs.h:22,
from linux/fs/xfs/xfs_super.c:7:
In function ‘xfs_check_ondisk_structs’,
inlined from ‘init_xfs_fs’ at linux/fs/xfs/xfs_super.c:2025:2:
linux/include/linux/compiler.h:350:38:
error: call to ‘__compiletime_assert_107’ declared with attribute
error: XFS: sizeof(xfs_dir2_sf_entry_t) is wrong, expected 3
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
Restore the correct behavior defining the correct size.
Cc: "Darrick J. Wong" <[email protected]>
Signed-off-by: Vincenzo Frascino <[email protected]>
---
fs/xfs/xfs_ondisk.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
index b6701b4f59a9..ee487ddc60c7 100644
--- a/fs/xfs/xfs_ondisk.h
+++ b/fs/xfs/xfs_ondisk.h
@@ -104,7 +104,7 @@ xfs_check_ondisk_structs(void)
XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_hdr_t, 16);
XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_t, 16);
XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_tail_t, 4);
- XFS_CHECK_STRUCT_SIZE(xfs_dir2_sf_entry_t, 3);
+ XFS_CHECK_STRUCT_SIZE(xfs_dir2_sf_entry_t, 4);
XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, namelen, 0);
XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, offset, 1);
XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, name, 3);
--
2.24.1
On 1/9/20 8:14 AM, Vincenzo Frascino wrote:
> xfs_check_ondisk_structs() verifies that the sizes of the data types
> used by xfs are correct via the XFS_CHECK_STRUCT_SIZE() macro.
>
> xfs_dir2_sf_entry_t size is set erroneously to 3 which breaks the
> compilation with the assertion below:
>
> In file included from linux/include/linux/string.h:6,
> from linux/include/linux/uuid.h:12,
> from linux/fs/xfs/xfs_linux.h:10,
> from linux/fs/xfs/xfs.h:22,
> from linux/fs/xfs/xfs_super.c:7:
> In function ‘xfs_check_ondisk_structs’,
> inlined from ‘init_xfs_fs’ at linux/fs/xfs/xfs_super.c:2025:2:
> linux/include/linux/compiler.h:350:38:
> error: call to ‘__compiletime_assert_107’ declared with attribute
> error: XFS: sizeof(xfs_dir2_sf_entry_t) is wrong, expected 3
> _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
>
> Restore the correct behavior defining the correct size.
# pahole -C xfs_dir2_sf_entry fs/xfs/xfs.o
struct xfs_dir2_sf_entry {
__u8 namelen; /* 0 1 */
__u8 offset[2]; /* 1 2 */
__u8 name[0]; /* 3 0 */
/* size: 3, cachelines: 1, members: 3 */
/* last cacheline: 3 bytes */
};
Can you please the same command on your machine, along with which arm abi is
in use etc just for clarity?
-Eric
> Cc: "Darrick J. Wong" <[email protected]>
> Signed-off-by: Vincenzo Frascino <[email protected]>
> ---
> fs/xfs/xfs_ondisk.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> index b6701b4f59a9..ee487ddc60c7 100644
> --- a/fs/xfs/xfs_ondisk.h
> +++ b/fs/xfs/xfs_ondisk.h
> @@ -104,7 +104,7 @@ xfs_check_ondisk_structs(void)
> XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_hdr_t, 16);
> XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_t, 16);
> XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_tail_t, 4);
> - XFS_CHECK_STRUCT_SIZE(xfs_dir2_sf_entry_t, 3);
> + XFS_CHECK_STRUCT_SIZE(xfs_dir2_sf_entry_t, 4);
> XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, namelen, 0);
> XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, offset, 1);
> XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, name, 3);
>
Hi Eric,
On 09/01/2020 15:01, Eric Sandeen wrote:
> On 1/9/20 8:14 AM, Vincenzo Frascino wrote:
>> xfs_check_ondisk_structs() verifies that the sizes of the data types
>> used by xfs are correct via the XFS_CHECK_STRUCT_SIZE() macro.
>>
>> xfs_dir2_sf_entry_t size is set erroneously to 3 which breaks the
>> compilation with the assertion below:
>>
>> In file included from linux/include/linux/string.h:6,
>> from linux/include/linux/uuid.h:12,
>> from linux/fs/xfs/xfs_linux.h:10,
>> from linux/fs/xfs/xfs.h:22,
>> from linux/fs/xfs/xfs_super.c:7:
>> In function ‘xfs_check_ondisk_structs’,
>> inlined from ‘init_xfs_fs’ at linux/fs/xfs/xfs_super.c:2025:2:
>> linux/include/linux/compiler.h:350:38:
>> error: call to ‘__compiletime_assert_107’ declared with attribute
>> error: XFS: sizeof(xfs_dir2_sf_entry_t) is wrong, expected 3
>> _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
>>
>> Restore the correct behavior defining the correct size.
>
> # pahole -C xfs_dir2_sf_entry fs/xfs/xfs.o
>
> struct xfs_dir2_sf_entry {
> __u8 namelen; /* 0 1 */
> __u8 offset[2]; /* 1 2 */
> __u8 name[0]; /* 3 0 */
>
> /* size: 3, cachelines: 1, members: 3 */
> /* last cacheline: 3 bytes */
> };
>
> Can you please the same command on your machine, along with which arm abi is
> in use etc just for clarity?
>
The abi is arm32 eabihf. You can reproduce my scenario using randconfig with
seed 0x72F68201.
In this case I get size 4, hence my patch.
If I enable xfs on the defconfig though size is 3 accordingly to what you have
reported. I will continue the investigation.
Vincenzo
> -Eric
>
>> Cc: "Darrick J. Wong" <[email protected]>
>> Signed-off-by: Vincenzo Frascino <[email protected]>
>> ---
>> fs/xfs/xfs_ondisk.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
>> index b6701b4f59a9..ee487ddc60c7 100644
>> --- a/fs/xfs/xfs_ondisk.h
>> +++ b/fs/xfs/xfs_ondisk.h
>> @@ -104,7 +104,7 @@ xfs_check_ondisk_structs(void)
>> XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_hdr_t, 16);
>> XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_t, 16);
>> XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_tail_t, 4);
>> - XFS_CHECK_STRUCT_SIZE(xfs_dir2_sf_entry_t, 3);
>> + XFS_CHECK_STRUCT_SIZE(xfs_dir2_sf_entry_t, 4);
>> XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, namelen, 0);
>> XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, offset, 1);
>> XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, name, 3);
>>
--
Regards,
Vincenzo
On Thu, Jan 09, 2020 at 03:35:46PM +0000, Vincenzo Frascino wrote:
> Hi Eric,
>
> On 09/01/2020 15:01, Eric Sandeen wrote:
> > On 1/9/20 8:14 AM, Vincenzo Frascino wrote:
> >> xfs_check_ondisk_structs() verifies that the sizes of the data types
> >> used by xfs are correct via the XFS_CHECK_STRUCT_SIZE() macro.
> >>
> >> xfs_dir2_sf_entry_t size is set erroneously to 3 which breaks the
> >> compilation with the assertion below:
> >>
> >> In file included from linux/include/linux/string.h:6,
> >> from linux/include/linux/uuid.h:12,
> >> from linux/fs/xfs/xfs_linux.h:10,
> >> from linux/fs/xfs/xfs.h:22,
> >> from linux/fs/xfs/xfs_super.c:7:
> >> In function ‘xfs_check_ondisk_structs’,
> >> inlined from ‘init_xfs_fs’ at linux/fs/xfs/xfs_super.c:2025:2:
> >> linux/include/linux/compiler.h:350:38:
> >> error: call to ‘__compiletime_assert_107’ declared with attribute
> >> error: XFS: sizeof(xfs_dir2_sf_entry_t) is wrong, expected 3
So, working as expected -- with size == 4 the directory metadata block
pointer calculations will be incorrect, and you'll end up with a corrupt
filesystem.
> >> _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
> >>
> >> Restore the correct behavior defining the correct size.
> >
> > # pahole -C xfs_dir2_sf_entry fs/xfs/xfs.o
> >
> > struct xfs_dir2_sf_entry {
> > __u8 namelen; /* 0 1 */
> > __u8 offset[2]; /* 1 2 */
> > __u8 name[0]; /* 3 0 */
This sounds like gcc getting confused by the zero length array. Though
it's odd that randconfig breaks, but defconfig doesn't? This sounds
like one of the kernel gcc options causing problems.
> >
> > /* size: 3, cachelines: 1, members: 3 */
> > /* last cacheline: 3 bytes */
> > };
> >
> > Can you please the same command on your machine, along with which arm abi is
> > in use etc just for clarity?
> >
>
> The abi is arm32 eabihf. You can reproduce my scenario using randconfig with
> seed 0x72F68201.
Please send the actual .config file produced by randconfig 72f68201...
> In this case I get size 4, hence my patch.
>
> If I enable xfs on the defconfig though size is 3 accordingly to what you have
> reported. I will continue the investigation.
...and the .config file produced by defconfig, in the hopes that someone
will spot the culprit using differential analysis. Assuming you haven't
done that already.
--D
> Vincenzo
>
> > -Eric
> >
> >> Cc: "Darrick J. Wong" <[email protected]>
> >> Signed-off-by: Vincenzo Frascino <[email protected]>
> >> ---
> >> fs/xfs/xfs_ondisk.h | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/fs/xfs/xfs_ondisk.h b/fs/xfs/xfs_ondisk.h
> >> index b6701b4f59a9..ee487ddc60c7 100644
> >> --- a/fs/xfs/xfs_ondisk.h
> >> +++ b/fs/xfs/xfs_ondisk.h
> >> @@ -104,7 +104,7 @@ xfs_check_ondisk_structs(void)
> >> XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_hdr_t, 16);
> >> XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_t, 16);
> >> XFS_CHECK_STRUCT_SIZE(xfs_dir2_leaf_tail_t, 4);
> >> - XFS_CHECK_STRUCT_SIZE(xfs_dir2_sf_entry_t, 3);
> >> + XFS_CHECK_STRUCT_SIZE(xfs_dir2_sf_entry_t, 4);
> >> XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, namelen, 0);
> >> XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, offset, 1);
> >> XFS_CHECK_OFFSET(xfs_dir2_sf_entry_t, name, 3);
> >>
>
> --
> Regards,
> Vincenzo
pub RSA 4096/072FD436 2019-09-02 Vincenzo Frascino <[email protected]>
> sub RSA 2048/4205BF15 2019-09-02
> sub RSA 2048/296522AA 2019-09-02
> sub RSA 2048/7CAB726B 2019-09-02
>
Hi Darrick,
On 09/01/2020 16:50, Darrick J. Wong wrote:
> This sounds like gcc getting confused by the zero length array. Though
> it's odd that randconfig breaks, but defconfig doesn't? This sounds
> like one of the kernel gcc options causing problems.
>
This is what I started suspecting as well.
>>> /* size: 3, cachelines: 1, members: 3 */
>>> /* last cacheline: 3 bytes */
>>> };
>>>
>>> Can you please the same command on your machine, along with which arm abi is
>>> in use etc just for clarity?
>>>
>> The abi is arm32 eabihf. You can reproduce my scenario using randconfig with
>> seed 0x72F68201.
> Please send the actual .config file produced by randconfig 72f68201...
>
>> In this case I get size 4, hence my patch.
>>
>> If I enable xfs on the defconfig though size is 3 accordingly to what you have
>> reported. I will continue the investigation.
> ...and the .config file produced by defconfig, in the hopes that someone
> will spot the culprit using differential analysis. Assuming you haven't
> done that already.
I did not spot anything unusual yet. I am attaching the files so that someone
else can have a look in the meantime.
--
Regards,
Vincenzo
Hi Vincenzo,
I love your patch! Yet something to improve:
[auto build test ERROR on xfs-linux/for-next]
[also build test ERROR on djwong-xfs/djwong-devel v5.5-rc5 next-20200110]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Vincenzo-Frascino/xfs-Fix-xfs_dir2_sf_entry_t-size-check/20200110-144608
base: https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git for-next
config: x86_64-lkp (attached as .config)
compiler: gcc-7 (Debian 7.5.0-3) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=x86_64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <[email protected]>
All errors (new ones prefixed by >>):
In file included from include/linux/string.h:6:0,
from include/linux/uuid.h:12,
from fs/xfs/xfs_linux.h:10,
from fs/xfs/xfs.h:22,
from fs/xfs/xfs_super.c:7:
In function 'xfs_check_ondisk_structs',
inlined from 'init_xfs_fs' at fs/xfs/xfs_super.c:2025:2:
>> include/linux/compiler.h:350:38: error: call to '__compiletime_assert_107' declared with attribute error: XFS: sizeof(xfs_dir2_sf_entry_t) is wrong, expected 4
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^
include/linux/compiler.h:331:4: note: in definition of macro '__compiletime_assert'
prefix ## suffix(); \
^~~~~~
include/linux/compiler.h:350:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
fs/xfs/xfs_ondisk.h:10:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(sizeof(structname) != (size), "XFS: sizeof(" \
^~~~~~~~~~~~~~~~
fs/xfs/xfs_ondisk.h:107:2: note: in expansion of macro 'XFS_CHECK_STRUCT_SIZE'
XFS_CHECK_STRUCT_SIZE(xfs_dir2_sf_entry_t, 4);
^~~~~~~~~~~~~~~~~~~~~
--
In file included from include/linux/string.h:6:0,
from include/linux/uuid.h:12,
from fs//xfs/xfs_linux.h:10,
from fs//xfs/xfs.h:22,
from fs//xfs/xfs_super.c:7:
In function 'xfs_check_ondisk_structs',
inlined from 'init_xfs_fs' at fs//xfs/xfs_super.c:2025:2:
>> include/linux/compiler.h:350:38: error: call to '__compiletime_assert_107' declared with attribute error: XFS: sizeof(xfs_dir2_sf_entry_t) is wrong, expected 4
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^
include/linux/compiler.h:331:4: note: in definition of macro '__compiletime_assert'
prefix ## suffix(); \
^~~~~~
include/linux/compiler.h:350:2: note: in expansion of macro '_compiletime_assert'
_compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
^~~~~~~~~~~~~~~~~~~
include/linux/build_bug.h:39:37: note: in expansion of macro 'compiletime_assert'
#define BUILD_BUG_ON_MSG(cond, msg) compiletime_assert(!(cond), msg)
^~~~~~~~~~~~~~~~~~
fs//xfs/xfs_ondisk.h:10:2: note: in expansion of macro 'BUILD_BUG_ON_MSG'
BUILD_BUG_ON_MSG(sizeof(structname) != (size), "XFS: sizeof(" \
^~~~~~~~~~~~~~~~
fs//xfs/xfs_ondisk.h:107:2: note: in expansion of macro 'XFS_CHECK_STRUCT_SIZE'
XFS_CHECK_STRUCT_SIZE(xfs_dir2_sf_entry_t, 4);
^~~~~~~~~~~~~~~~~~~~~
vim +/__compiletime_assert_107 +350 include/linux/compiler.h
9a8ab1c39970a4 Daniel Santos 2013-02-21 336
9a8ab1c39970a4 Daniel Santos 2013-02-21 337 #define _compiletime_assert(condition, msg, prefix, suffix) \
9a8ab1c39970a4 Daniel Santos 2013-02-21 338 __compiletime_assert(condition, msg, prefix, suffix)
9a8ab1c39970a4 Daniel Santos 2013-02-21 339
9a8ab1c39970a4 Daniel Santos 2013-02-21 340 /**
9a8ab1c39970a4 Daniel Santos 2013-02-21 341 * compiletime_assert - break build and emit msg if condition is false
9a8ab1c39970a4 Daniel Santos 2013-02-21 342 * @condition: a compile-time constant condition to check
9a8ab1c39970a4 Daniel Santos 2013-02-21 343 * @msg: a message to emit if condition is false
9a8ab1c39970a4 Daniel Santos 2013-02-21 344 *
9a8ab1c39970a4 Daniel Santos 2013-02-21 345 * In tradition of POSIX assert, this macro will break the build if the
9a8ab1c39970a4 Daniel Santos 2013-02-21 346 * supplied condition is *false*, emitting the supplied error message if the
9a8ab1c39970a4 Daniel Santos 2013-02-21 347 * compiler has support to do so.
9a8ab1c39970a4 Daniel Santos 2013-02-21 348 */
9a8ab1c39970a4 Daniel Santos 2013-02-21 349 #define compiletime_assert(condition, msg) \
9a8ab1c39970a4 Daniel Santos 2013-02-21 @350 _compiletime_assert(condition, msg, __compiletime_assert_, __LINE__)
9a8ab1c39970a4 Daniel Santos 2013-02-21 351
:::::: The code at line 350 was first introduced by commit
:::::: 9a8ab1c39970a4938a72d94e6fd13be88a797590 bug.h, compiler.h: introduce compiletime_assert & BUILD_BUG_ON_MSG
:::::: TO: Daniel Santos <[email protected]>
:::::: CC: Linus Torvalds <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation
On Thu, Jan 9, 2020 at 10:01 PM Vincenzo Frascino
<[email protected]> wrote:
>
> Hi Darrick,
>
> On 09/01/2020 16:50, Darrick J. Wong wrote:
> > This sounds like gcc getting confused by the zero length array. Though
> > it's odd that randconfig breaks, but defconfig doesn't? This sounds
> > like one of the kernel gcc options causing problems.
> >
>
> This is what I started suspecting as well.
The important bit into the configuration is
# CONFIG_AEABI is not set
With ARM OABI (which you get when EABI is disabled), structures are padded
to multiples of 32 bits. See commits 8353a649f577 ("xfs: kill
xfs_dir2_sf_off_t")
and aa2dd0ad4d6d ("xfs: remove __arch_pack"). Those could be partially
reverted to fix it again, but it doesn't seem worth it as there is
probably nobody
running XFS on OABI machines (actually with the build failure we can
be fairly sure there isn't ;-).
I am testing randconfig builds with OABI and a few other things like ARCH_RPC
disabled because of random issues like this.
Arnd
On Mon, Jan 13, 2020 at 02:55:15PM +0100, Arnd Bergmann wrote:
> With ARM OABI (which you get when EABI is disabled), structures are padded
> to multiples of 32 bits. See commits 8353a649f577 ("xfs: kill
> xfs_dir2_sf_off_t")
> and aa2dd0ad4d6d ("xfs: remove __arch_pack"). Those could be partially
> reverted to fix it again, but it doesn't seem worth it as there is
> probably nobody
> running XFS on OABI machines (actually with the build failure we can
> be fairly sure there isn't ;-).
Or just try adding a __packed to the xfs_dir2_sf_entry definition?
Hi Arnd,
On 1/13/20 1:55 PM, Arnd Bergmann wrote:
> On Thu, Jan 9, 2020 at 10:01 PM Vincenzo Frascino
> <[email protected]> wrote:
>>
>> Hi Darrick,
>>
>> On 09/01/2020 16:50, Darrick J. Wong wrote:
>>> This sounds like gcc getting confused by the zero length array. Though
>>> it's odd that randconfig breaks, but defconfig doesn't? This sounds
>>> like one of the kernel gcc options causing problems.
>>>
>>
>> This is what I started suspecting as well.
>
> The important bit into the configuration is
>
> # CONFIG_AEABI is not set
>
> With ARM OABI (which you get when EABI is disabled), structures are padded
> to multiples of 32 bits. See commits 8353a649f577 ("xfs: kill
> xfs_dir2_sf_off_t")
> and aa2dd0ad4d6d ("xfs: remove __arch_pack"). Those could be partially
> reverted to fix it again, but it doesn't seem worth it as there is
> probably nobody
> running XFS on OABI machines (actually with the build failure we can
> be fairly sure there isn't ;-).
>
Thanks for this, for some reasons I was convinced that CONFIG_AEABI was set in
this configuration file as I reported as well in my previous email.
Since it is OABI makes sense disabling xfs for randconfig purposes.
--
Regards,
Vincenzo
On Mon, Jan 13, 2020 at 2:58 PM Christoph Hellwig <[email protected]> wrote:
>
> On Mon, Jan 13, 2020 at 02:55:15PM +0100, Arnd Bergmann wrote:
> > With ARM OABI (which you get when EABI is disabled), structures are padded
> > to multiples of 32 bits. See commits 8353a649f577 ("xfs: kill
> > xfs_dir2_sf_off_t")
> > and aa2dd0ad4d6d ("xfs: remove __arch_pack"). Those could be partially
> > reverted to fix it again, but it doesn't seem worth it as there is
> > probably nobody
> > running XFS on OABI machines (actually with the build failure we can
> > be fairly sure there isn't ;-).
>
> Or just try adding a __packed to the xfs_dir2_sf_entry definition?
Yes, that should be correct on all architectures, and I just noticed
that this is what we already have on xfs_dir2_sf_hdr_t directly
above it for the same reason.
Arnd
On Mon, Jan 13, 2020 at 03:06:50PM +0100, Arnd Bergmann wrote:
> On Mon, Jan 13, 2020 at 2:58 PM Christoph Hellwig <[email protected]> wrote:
> >
> > On Mon, Jan 13, 2020 at 02:55:15PM +0100, Arnd Bergmann wrote:
> > > With ARM OABI (which you get when EABI is disabled), structures are padded
> > > to multiples of 32 bits. See commits 8353a649f577 ("xfs: kill
> > > xfs_dir2_sf_off_t")
> > > and aa2dd0ad4d6d ("xfs: remove __arch_pack"). Those could be partially
> > > reverted to fix it again, but it doesn't seem worth it as there is
> > > probably nobody
> > > running XFS on OABI machines (actually with the build failure we can
> > > be fairly sure there isn't ;-).
> >
> > Or just try adding a __packed to the xfs_dir2_sf_entry definition?
>
> Yes, that should be correct on all architectures, and I just noticed
> that this is what we already have on xfs_dir2_sf_hdr_t directly
> above it for the same reason.
Yeah, that sounds like a reasonable way forward, short of cleaning out
all the array[0] cr^Hode... ;)
To the original submitter: can you add __packed to the structure
definition and (assuming it passes oabi compilation) send that to the
list, please?
--D
>
> Arnd
On 1/13/20 11:01 AM, Darrick J. Wong wrote:
> On Mon, Jan 13, 2020 at 03:06:50PM +0100, Arnd Bergmann wrote:
>> On Mon, Jan 13, 2020 at 2:58 PM Christoph Hellwig <[email protected]> wrote:
>>>
>>> On Mon, Jan 13, 2020 at 02:55:15PM +0100, Arnd Bergmann wrote:
>>>> With ARM OABI (which you get when EABI is disabled), structures are padded
>>>> to multiples of 32 bits. See commits 8353a649f577 ("xfs: kill
>>>> xfs_dir2_sf_off_t")
>>>> and aa2dd0ad4d6d ("xfs: remove __arch_pack"). Those could be partially
>>>> reverted to fix it again, but it doesn't seem worth it as there is
>>>> probably nobody
>>>> running XFS on OABI machines (actually with the build failure we can
>>>> be fairly sure there isn't ;-).
>>>
>>> Or just try adding a __packed to the xfs_dir2_sf_entry definition?
>>
>> Yes, that should be correct on all architectures, and I just noticed
>> that this is what we already have on xfs_dir2_sf_hdr_t directly
>> above it for the same reason.
>
> Yeah, that sounds like a reasonable way forward, short of cleaning out
> all the array[0] cr^Hode... ;)
>
> To the original submitter: can you add __packed to the structure
> definition and (assuming it passes oabi compilation) send that to the
> list, please?
Probably worth doing this iteratively until all the build-time size checks
pass on OABI - just to be sure there are no more lurking?
-Eric
Hi Darrick,
On 1/13/20 5:01 PM, Darrick J. Wong wrote:
> On Mon, Jan 13, 2020 at 03:06:50PM +0100, Arnd Bergmann wrote:
>> On Mon, Jan 13, 2020 at 2:58 PM Christoph Hellwig <[email protected]> wrote:
>>>
>>> On Mon, Jan 13, 2020 at 02:55:15PM +0100, Arnd Bergmann wrote:
>>>> With ARM OABI (which you get when EABI is disabled), structures are padded
>>>> to multiples of 32 bits. See commits 8353a649f577 ("xfs: kill
>>>> xfs_dir2_sf_off_t")
>>>> and aa2dd0ad4d6d ("xfs: remove __arch_pack"). Those could be partially
>>>> reverted to fix it again, but it doesn't seem worth it as there is
>>>> probably nobody
>>>> running XFS on OABI machines (actually with the build failure we can
>>>> be fairly sure there isn't ;-).
>>>
>>> Or just try adding a __packed to the xfs_dir2_sf_entry definition?
>>
>> Yes, that should be correct on all architectures, and I just noticed
>> that this is what we already have on xfs_dir2_sf_hdr_t directly
>> above it for the same reason.
>
> Yeah, that sounds like a reasonable way forward, short of cleaning out
> all the array[0] cr^Hode... ;)
>
> To the original submitter: can you add __packed to the structure
> definition and (assuming it passes oabi compilation) send that to the
> list, please?
>
I will test it tomorrow morning as first thing and will send a patch out.
Thank you all for your help!
> --D
>
>>
>> Arnd
--
Regards,
Vincenzo