* [PATCH] xfs: Fix xfs_dir2_sf_entry_t size check @ 2020-01-09 14:14 Vincenzo Frascino 2020-01-09 15:01 ` Eric Sandeen 2020-01-12 0:44 ` kbuild test robot 0 siblings, 2 replies; 12+ messages in thread From: Vincenzo Frascino @ 2020-01-09 14:14 UTC (permalink / raw) To: darrick.wong, linux-xfs, linux-kernel; +Cc: vincenzo.frascino 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" <darrick.wong@oracle.com> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com> --- 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] xfs: Fix xfs_dir2_sf_entry_t size check 2020-01-09 14:14 [PATCH] xfs: Fix xfs_dir2_sf_entry_t size check Vincenzo Frascino @ 2020-01-09 15:01 ` Eric Sandeen 2020-01-09 15:35 ` Vincenzo Frascino 2020-01-12 0:44 ` kbuild test robot 1 sibling, 1 reply; 12+ messages in thread From: Eric Sandeen @ 2020-01-09 15:01 UTC (permalink / raw) To: Vincenzo Frascino, darrick.wong, linux-xfs, linux-kernel 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" <darrick.wong@oracle.com> > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com> > --- > 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); > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] xfs: Fix xfs_dir2_sf_entry_t size check 2020-01-09 15:01 ` Eric Sandeen @ 2020-01-09 15:35 ` Vincenzo Frascino 2020-01-09 16:50 ` Darrick J. Wong 0 siblings, 1 reply; 12+ messages in thread From: Vincenzo Frascino @ 2020-01-09 15:35 UTC (permalink / raw) To: Eric Sandeen, darrick.wong, linux-xfs, linux-kernel [-- Attachment #1: Type: text/plain, Size: 2794 bytes --] 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" <darrick.wong@oracle.com> >> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com> >> --- >> 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 [-- Attachment #2: pEpkey.asc --] [-- Type: application/pgp-keys, Size: 14291 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] xfs: Fix xfs_dir2_sf_entry_t size check 2020-01-09 15:35 ` Vincenzo Frascino @ 2020-01-09 16:50 ` Darrick J. Wong [not found] ` <435bcb71-9126-b1f1-3803-4977754b36ff@arm.com> 0 siblings, 1 reply; 12+ messages in thread From: Darrick J. Wong @ 2020-01-09 16:50 UTC (permalink / raw) To: Vincenzo Frascino; +Cc: Eric Sandeen, linux-xfs, linux-kernel 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" <darrick.wong@oracle.com> > >> Signed-off-by: Vincenzo Frascino <vincenzo.frascino@arm.com> > >> --- > >> 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 <vincenzo.frascino@arm.com> > sub RSA 2048/4205BF15 2019-09-02 > sub RSA 2048/296522AA 2019-09-02 > sub RSA 2048/7CAB726B 2019-09-02 > ^ permalink raw reply [flat|nested] 12+ messages in thread
[parent not found: <435bcb71-9126-b1f1-3803-4977754b36ff@arm.com>]
* Re: [PATCH] xfs: Fix xfs_dir2_sf_entry_t size check [not found] ` <435bcb71-9126-b1f1-3803-4977754b36ff@arm.com> @ 2020-01-13 13:55 ` Arnd Bergmann 2020-01-13 13:58 ` Christoph Hellwig 2020-01-13 14:05 ` Vincenzo Frascino 0 siblings, 2 replies; 12+ messages in thread From: Arnd Bergmann @ 2020-01-13 13:55 UTC (permalink / raw) To: Vincenzo Frascino Cc: Darrick J. Wong, Eric Sandeen, linux-xfs, linux-kernel, Christoph Hellwig On Thu, Jan 9, 2020 at 10:01 PM Vincenzo Frascino <vincenzo.frascino@arm.com> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] xfs: Fix xfs_dir2_sf_entry_t size check 2020-01-13 13:55 ` Arnd Bergmann @ 2020-01-13 13:58 ` Christoph Hellwig 2020-01-13 14:06 ` Arnd Bergmann 2020-01-13 14:05 ` Vincenzo Frascino 1 sibling, 1 reply; 12+ messages in thread From: Christoph Hellwig @ 2020-01-13 13:58 UTC (permalink / raw) To: Arnd Bergmann Cc: Vincenzo Frascino, Darrick J. Wong, Eric Sandeen, linux-xfs, linux-kernel, Christoph Hellwig 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? ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] xfs: Fix xfs_dir2_sf_entry_t size check 2020-01-13 13:58 ` Christoph Hellwig @ 2020-01-13 14:06 ` Arnd Bergmann 2020-01-13 17:01 ` Darrick J. Wong 0 siblings, 1 reply; 12+ messages in thread From: Arnd Bergmann @ 2020-01-13 14:06 UTC (permalink / raw) To: Christoph Hellwig Cc: Vincenzo Frascino, Darrick J. Wong, Eric Sandeen, linux-xfs, linux-kernel On Mon, Jan 13, 2020 at 2:58 PM Christoph Hellwig <hch@lst.de> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] xfs: Fix xfs_dir2_sf_entry_t size check 2020-01-13 14:06 ` Arnd Bergmann @ 2020-01-13 17:01 ` Darrick J. Wong 2020-01-13 17:04 ` Eric Sandeen 2020-01-13 17:26 ` Vincenzo Frascino 0 siblings, 2 replies; 12+ messages in thread From: Darrick J. Wong @ 2020-01-13 17:01 UTC (permalink / raw) To: Arnd Bergmann Cc: Christoph Hellwig, Vincenzo Frascino, Eric Sandeen, linux-xfs, linux-kernel On Mon, Jan 13, 2020 at 03:06:50PM +0100, Arnd Bergmann wrote: > On Mon, Jan 13, 2020 at 2:58 PM Christoph Hellwig <hch@lst.de> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] xfs: Fix xfs_dir2_sf_entry_t size check 2020-01-13 17:01 ` Darrick J. Wong @ 2020-01-13 17:04 ` Eric Sandeen 2020-01-13 17:26 ` Vincenzo Frascino 1 sibling, 0 replies; 12+ messages in thread From: Eric Sandeen @ 2020-01-13 17:04 UTC (permalink / raw) To: Darrick J. Wong, Arnd Bergmann Cc: Christoph Hellwig, Vincenzo Frascino, linux-xfs, linux-kernel 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 <hch@lst.de> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] xfs: Fix xfs_dir2_sf_entry_t size check 2020-01-13 17:01 ` Darrick J. Wong 2020-01-13 17:04 ` Eric Sandeen @ 2020-01-13 17:26 ` Vincenzo Frascino 1 sibling, 0 replies; 12+ messages in thread From: Vincenzo Frascino @ 2020-01-13 17:26 UTC (permalink / raw) To: Darrick J. Wong, Arnd Bergmann Cc: Christoph Hellwig, Eric Sandeen, linux-xfs, linux-kernel 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 <hch@lst.de> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] xfs: Fix xfs_dir2_sf_entry_t size check 2020-01-13 13:55 ` Arnd Bergmann 2020-01-13 13:58 ` Christoph Hellwig @ 2020-01-13 14:05 ` Vincenzo Frascino 1 sibling, 0 replies; 12+ messages in thread From: Vincenzo Frascino @ 2020-01-13 14:05 UTC (permalink / raw) To: Arnd Bergmann Cc: Darrick J. Wong, Eric Sandeen, linux-xfs, linux-kernel, Christoph Hellwig Hi Arnd, On 1/13/20 1:55 PM, Arnd Bergmann wrote: > On Thu, Jan 9, 2020 at 10:01 PM Vincenzo Frascino > <vincenzo.frascino@arm.com> 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 ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] xfs: Fix xfs_dir2_sf_entry_t size check 2020-01-09 14:14 [PATCH] xfs: Fix xfs_dir2_sf_entry_t size check Vincenzo Frascino 2020-01-09 15:01 ` Eric Sandeen @ 2020-01-12 0:44 ` kbuild test robot 1 sibling, 0 replies; 12+ messages in thread From: kbuild test robot @ 2020-01-12 0:44 UTC (permalink / raw) To: Vincenzo Frascino Cc: kbuild-all, darrick.wong, linux-xfs, linux-kernel, vincenzo.frascino [-- Attachment #1: Type: text/plain, Size: 5766 bytes --] 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 <lkp@intel.com> 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 <daniel.santos@pobox.com> :::::: CC: Linus Torvalds <torvalds@linux-foundation.org> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org Intel Corporation [-- Attachment #2: .config.gz --] [-- Type: application/gzip, Size: 28789 bytes --] ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2020-01-13 17:23 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-09 14:14 [PATCH] xfs: Fix xfs_dir2_sf_entry_t size check Vincenzo Frascino 2020-01-09 15:01 ` Eric Sandeen 2020-01-09 15:35 ` Vincenzo Frascino 2020-01-09 16:50 ` Darrick J. Wong [not found] ` <435bcb71-9126-b1f1-3803-4977754b36ff@arm.com> 2020-01-13 13:55 ` Arnd Bergmann 2020-01-13 13:58 ` Christoph Hellwig 2020-01-13 14:06 ` Arnd Bergmann 2020-01-13 17:01 ` Darrick J. Wong 2020-01-13 17:04 ` Eric Sandeen 2020-01-13 17:26 ` Vincenzo Frascino 2020-01-13 14:05 ` Vincenzo Frascino 2020-01-12 0:44 ` kbuild test robot
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).