[dpdk-dev] [PATCH] common/dpaax: fix i686 compilation issue

[dpdk-dev] [PATCH] common/dpaax: fix i686 compilation issue

[Shreyansh Jain]

Meson build on i686, part of DPDK CI, reports: (example report [1])
    *Meson Failed Build #1:
    OS: UB1604-32
    Target: i686-native-linuxapp-gcc
    ../drivers/common/dpaax/dpaax_iova_table.c:121:3: note: in expansion of
    macro ‘DPAAX_DEBUG’
       DPAAX_DEBUG("Invalid memory node values or count. (size=%lu)",
       ^

'statbuf.st_size', part of fstat() is a 'off_t' - which should be printable
as '%lu'. But, only a particularly old compiler reports this (5.4.0).
For now, removing the variable being dumped as part of debug.

[1] http://mails.dpdk.org/archives/test-report/2019-May/082622.html

Fixes: 2f3d633aa593 ("common/dpaax: add library for PA/VA translation table")
Cc: shreyansh.jain@nxp.com

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
Note:
 - Another option was to use the PRIu32 macros but, in absence of a proper
   verification environment, I wish to take the safer route. In future, if
   need arises, the field would be added to debug logs.

 drivers/common/dpaax/dpaax_iova_table.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/common/dpaax/dpaax_iova_table.c b/drivers/common/dpaax/dpaax_iova_table.c
index 2dd38a920..4ba38ac00 100644
--- a/drivers/common/dpaax/dpaax_iova_table.c
+++ b/drivers/common/dpaax/dpaax_iova_table.c
@@ -118,8 +118,7 @@ read_memory_node(unsigned int *count)
 	 */
 	*count = (statbuf.st_size / 16);
 	if ((*count) <= 0 || (statbuf.st_size % 16 != 0)) {
-		DPAAX_DEBUG("Invalid memory node values or count. (size=%lu)",
-			    statbuf.st_size);
+		DPAAX_DEBUG("Invalid memory node values or count. ");
 		goto cleanup;
 	}
 
-- 
2.17.1

Re: [dpdk-dev] [PATCH] common/dpaax: fix i686 compilation issue

[Bruce Richardson]

On Wed, May 29, 2019 at 12:17:16PM +0530, Shreyansh Jain wrote:
> Meson build on i686, part of DPDK CI, reports: (example report [1])
>     *Meson Failed Build #1:
>     OS: UB1604-32
>     Target: i686-native-linuxapp-gcc
>     ../drivers/common/dpaax/dpaax_iova_table.c:121:3: note: in expansion of
>     macro ‘DPAAX_DEBUG’
>        DPAAX_DEBUG("Invalid memory node values or count. (size=%lu)",
>        ^
> 
> 'statbuf.st_size', part of fstat() is a 'off_t' - which should be printable
> as '%lu'. But, only a particularly old compiler reports this (5.4.0).
> For now, removing the variable being dumped as part of debug.
> 
> [1] http://mails.dpdk.org/archives/test-report/2019-May/082622.html
> 
> Fixes: 2f3d633aa593 ("common/dpaax: add library for PA/VA translation table")
> Cc: shreyansh.jain@nxp.com
> 
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> ---
> Note:
>  - Another option was to use the PRIu32 macros but, in absence of a proper
>    verification environment, I wish to take the safer route. In future, if
>    need arises, the field would be added to debug logs.
> 
Actually, I think it's a bit more complicated than that, since for meson
and make builds, the type and size of the offset field varies. I've already
posted a fix for this and other 32-bit build issues. Please review that, and
let me know what you think.

Ref: http://patches.dpdk.org/patch/53728/ <Fix for DPAA driver>
Ref: http://patches.dpdk.org/project/dpdk/list/?series=4792 <Full series>

Regards,
/Bruce

Re: [dpdk-dev] [PATCH] common/dpaax: fix i686 compilation issue

[Shreyansh Jain]

Hi Bruce,

> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Wednesday, May 29, 2019 2:48 PM
> To: Shreyansh Jain <shreyansh.jain@nxp.com>
> Cc: ferruh.yigit@intel.com; stephen@networkplumber.org; bluca@debian.org;
> dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH] common/dpaax: fix i686 compilation issue
> 
> On Wed, May 29, 2019 at 12:17:16PM +0530, Shreyansh Jain wrote:
> > Meson build on i686, part of DPDK CI, reports: (example report [1])
> >     *Meson Failed Build #1:
> >     OS: UB1604-32
> >     Target: i686-native-linuxapp-gcc
> >     ../drivers/common/dpaax/dpaax_iova_table.c:121:3: note: in expansion of
> >     macro ‘DPAAX_DEBUG’
> >        DPAAX_DEBUG("Invalid memory node values or count. (size=%lu)",
> >        ^
> >
> > 'statbuf.st_size', part of fstat() is a 'off_t' - which should be printable
> > as '%lu'. But, only a particularly old compiler reports this (5.4.0).
> > For now, removing the variable being dumped as part of debug.
> >
> > [1]
> https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fmails.dpdk.o
> rg%2Farchives%2Ftest-report%2F2019-
> May%2F082622.html&amp;data=02%7C01%7Cshreyansh.jain%40nxp.com%7C3b8f427731c44
> d3f946908d6e4168d1f%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636947182857
> 407108&amp;sdata=Dir66wDBr8YqsKrqcqCstuLBMPVVrYfEYqN3ETZm0LA%3D&amp;reserved=
> 0
> >
> > Fixes: 2f3d633aa593 ("common/dpaax: add library for PA/VA translation
> table")
> > Cc: shreyansh.jain@nxp.com
> >
> > Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> > ---
> > Note:
> >  - Another option was to use the PRIu32 macros but, in absence of a proper
> >    verification environment, I wish to take the safer route. In future, if
> >    need arises, the field would be added to debug logs.
> >
> Actually, I think it's a bit more complicated than that, since for meson
> and make builds, the type and size of the offset field varies. I've already
> posted a fix for this and other 32-bit build issues. Please review that, and
> let me know what you think.
> 
[...]

Thanks for highlighting - I didn't notice this series.
I have acked the DPAAX patch - 32 bit and large file is not really a 'worry' case.
Also, I can't test my patch as I am unable to reproduce this issue. So, anyways yours is better - it continues to keep the variable dumped in debug, and assuming you would have verified it somehow.

I will reject/suspend this patch in Patchwork.

Regards,
Shreyansh