diff mbox

[API-NEXT] linux-gen: pktio ipc: fix clang build

Message ID 1481898455-31282-1-git-send-email-maxim.uvarov@linaro.org
State Accepted
Commit ba203281cfd10b88a5d5b8f143ea34d14d373b58
Headers show

Commit Message

Maxim Uvarov Dec. 16, 2016, 2:27 p.m. UTC 
clang is more clever on setting and not using variables,
so it traps compilation. Also buffers header almost everywhere
reference by pointer so size of it should not impact on
performance.

Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>

---
 platform/linux-generic/include/odp_buffer_internal.h | 7 +++----
 platform/linux-generic/pktio/ipc.c                   | 7 -------
 2 files changed, 3 insertions(+), 11 deletions(-)

-- 
2.7.1.250.gff4ea60

Comments

Bill Fischofer Dec. 16, 2016, 6:11 p.m. UTC | #1 
On Fri, Dec 16, 2016 at 8:27 AM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> clang is more clever on setting and not using variables,

> so it traps compilation. Also buffers header almost everywhere

> reference by pointer so size of it should not impact on

> performance.

>

> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>


Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org>

> ---

>  platform/linux-generic/include/odp_buffer_internal.h | 7 +++----

>  platform/linux-generic/pktio/ipc.c                   | 7 -------

>  2 files changed, 3 insertions(+), 11 deletions(-)

>

> diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h

> index 903f0a7..4cc51d3 100644

> --- a/platform/linux-generic/include/odp_buffer_internal.h

> +++ b/platform/linux-generic/include/odp_buffer_internal.h

> @@ -64,11 +64,10 @@ struct odp_buffer_hdr_t {

>         struct {

>                 void     *hdr;

>                 uint8_t  *data;

> -#ifdef _ODP_PKTIO_IPC

> -       /* ipc mapped process can not walk over pointers,

> -        * offset has to be used */

> +               /* Used only if _ODP_PKTIO_IPC is set.

> +                * ipc mapped process can not walk over pointers,

> +                * offset has to be used */

>                 uint64_t ipc_data_offset;

> -#endif

>                 uint32_t  len;

>         } seg[CONFIG_PACKET_MAX_SEGS];

>

> diff --git a/platform/linux-generic/pktio/ipc.c b/platform/linux-generic/pktio/ipc.c

> index 5f26b56..c9df043 100644

> --- a/platform/linux-generic/pktio/ipc.c

> +++ b/platform/linux-generic/pktio/ipc.c

> @@ -459,12 +459,7 @@ static int ipc_pktio_recv_lockless(pktio_entry_t *pktio_entry,

>                 if (odp_unlikely(pool == ODP_POOL_INVALID))

>                         ODP_ABORT("invalid pool");

>

> -#ifdef _ODP_PKTIO_IPC

>                 data_pool_off = phdr->buf_hdr.seg[0].ipc_data_offset;

> -#else

> -               /* compile all function code even if ipc disabled with config */

> -               data_pool_off = 0;

> -#endif

>

>                 pkt = odp_packet_alloc(pool, phdr->frame_len);

>                 if (odp_unlikely(pkt == ODP_PACKET_INVALID)) {

> @@ -590,7 +585,6 @@ static int ipc_pktio_send_lockless(pktio_entry_t *pktio_entry,

>                 data_pool_off = (uint8_t *)pkt_hdr->buf_hdr.seg[0].data -

>                                 (uint8_t *)odp_shm_addr(pool->shm);

>

> -#ifdef _ODP_PKTIO_IPC

>                 /* compile all function code even if ipc disabled with config */

>                 pkt_hdr->buf_hdr.seg[0].ipc_data_offset = data_pool_off;

>                 IPC_ODP_DBG("%d/%d send packet %llx, pool %llx,"

> @@ -598,7 +592,6 @@ static int ipc_pktio_send_lockless(pktio_entry_t *pktio_entry,

>                             i, len,

>                             odp_packet_to_u64(pkt), odp_pool_to_u64(pool_hdl),

>                             pkt_hdr, pkt_hdr->buf_hdr.seg[0].ipc_data_offset);

> -#endif

>         }

>

>         /* Put packets to ring to be processed by other process. */

> --

> 2.7.1.250.gff4ea60

>
Maxim Uvarov Dec. 16, 2016, 6:47 p.m. UTC | #2 
Thanks, merged.

I think after some time of stable pass we can remove enable/disable
option. A lot of options makes it nightmare to test all combinations.

Maxim.

On 12/16/16 21:11, Bill Fischofer wrote:
> On Fri, Dec 16, 2016 at 8:27 AM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:

>> clang is more clever on setting and not using variables,

>> so it traps compilation. Also buffers header almost everywhere

>> reference by pointer so size of it should not impact on

>> performance.

>>

>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>

> 

> Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org>

> 

>> ---

>>  platform/linux-generic/include/odp_buffer_internal.h | 7 +++----

>>  platform/linux-generic/pktio/ipc.c                   | 7 -------

>>  2 files changed, 3 insertions(+), 11 deletions(-)

>>

>> diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h

>> index 903f0a7..4cc51d3 100644

>> --- a/platform/linux-generic/include/odp_buffer_internal.h

>> +++ b/platform/linux-generic/include/odp_buffer_internal.h

>> @@ -64,11 +64,10 @@ struct odp_buffer_hdr_t {

>>         struct {

>>                 void     *hdr;

>>                 uint8_t  *data;

>> -#ifdef _ODP_PKTIO_IPC

>> -       /* ipc mapped process can not walk over pointers,

>> -        * offset has to be used */

>> +               /* Used only if _ODP_PKTIO_IPC is set.

>> +                * ipc mapped process can not walk over pointers,

>> +                * offset has to be used */

>>                 uint64_t ipc_data_offset;

>> -#endif

>>                 uint32_t  len;

>>         } seg[CONFIG_PACKET_MAX_SEGS];

>>

>> diff --git a/platform/linux-generic/pktio/ipc.c b/platform/linux-generic/pktio/ipc.c

>> index 5f26b56..c9df043 100644

>> --- a/platform/linux-generic/pktio/ipc.c

>> +++ b/platform/linux-generic/pktio/ipc.c

>> @@ -459,12 +459,7 @@ static int ipc_pktio_recv_lockless(pktio_entry_t *pktio_entry,

>>                 if (odp_unlikely(pool == ODP_POOL_INVALID))

>>                         ODP_ABORT("invalid pool");

>>

>> -#ifdef _ODP_PKTIO_IPC

>>                 data_pool_off = phdr->buf_hdr.seg[0].ipc_data_offset;

>> -#else

>> -               /* compile all function code even if ipc disabled with config */

>> -               data_pool_off = 0;

>> -#endif

>>

>>                 pkt = odp_packet_alloc(pool, phdr->frame_len);

>>                 if (odp_unlikely(pkt == ODP_PACKET_INVALID)) {

>> @@ -590,7 +585,6 @@ static int ipc_pktio_send_lockless(pktio_entry_t *pktio_entry,

>>                 data_pool_off = (uint8_t *)pkt_hdr->buf_hdr.seg[0].data -

>>                                 (uint8_t *)odp_shm_addr(pool->shm);

>>

>> -#ifdef _ODP_PKTIO_IPC

>>                 /* compile all function code even if ipc disabled with config */

>>                 pkt_hdr->buf_hdr.seg[0].ipc_data_offset = data_pool_off;

>>                 IPC_ODP_DBG("%d/%d send packet %llx, pool %llx,"

>> @@ -598,7 +592,6 @@ static int ipc_pktio_send_lockless(pktio_entry_t *pktio_entry,

>>                             i, len,

>>                             odp_packet_to_u64(pkt), odp_pool_to_u64(pool_hdl),

>>                             pkt_hdr, pkt_hdr->buf_hdr.seg[0].ipc_data_offset);

>> -#endif

>>         }

>>

>>         /* Put packets to ring to be processed by other process. */

>> --

>> 2.7.1.250.gff4ea60

>>
Bill Fischofer Dec. 16, 2016, 7:05 p.m. UTC | #3 
On Fri, Dec 16, 2016 at 12:47 PM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:
> Thanks, merged.

>

> I think after some time of stable pass we can remove enable/disable

> option. A lot of options makes it nightmare to test all combinations.


Agreed, but let's discuss that at a future ARCH call post MS-1.

>

> Maxim.

>

> On 12/16/16 21:11, Bill Fischofer wrote:

>> On Fri, Dec 16, 2016 at 8:27 AM, Maxim Uvarov <maxim.uvarov@linaro.org> wrote:

>>> clang is more clever on setting and not using variables,

>>> so it traps compilation. Also buffers header almost everywhere

>>> reference by pointer so size of it should not impact on

>>> performance.

>>>

>>> Signed-off-by: Maxim Uvarov <maxim.uvarov@linaro.org>

>>

>> Reviewed-and-tested-by: Bill Fischofer <bill.fischofer@linaro.org>

>>

>>> ---

>>>  platform/linux-generic/include/odp_buffer_internal.h | 7 +++----

>>>  platform/linux-generic/pktio/ipc.c                   | 7 -------

>>>  2 files changed, 3 insertions(+), 11 deletions(-)

>>>

>>> diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h

>>> index 903f0a7..4cc51d3 100644

>>> --- a/platform/linux-generic/include/odp_buffer_internal.h

>>> +++ b/platform/linux-generic/include/odp_buffer_internal.h

>>> @@ -64,11 +64,10 @@ struct odp_buffer_hdr_t {

>>>         struct {

>>>                 void     *hdr;

>>>                 uint8_t  *data;

>>> -#ifdef _ODP_PKTIO_IPC

>>> -       /* ipc mapped process can not walk over pointers,

>>> -        * offset has to be used */

>>> +               /* Used only if _ODP_PKTIO_IPC is set.

>>> +                * ipc mapped process can not walk over pointers,

>>> +                * offset has to be used */

>>>                 uint64_t ipc_data_offset;

>>> -#endif

>>>                 uint32_t  len;

>>>         } seg[CONFIG_PACKET_MAX_SEGS];

>>>

>>> diff --git a/platform/linux-generic/pktio/ipc.c b/platform/linux-generic/pktio/ipc.c

>>> index 5f26b56..c9df043 100644

>>> --- a/platform/linux-generic/pktio/ipc.c

>>> +++ b/platform/linux-generic/pktio/ipc.c

>>> @@ -459,12 +459,7 @@ static int ipc_pktio_recv_lockless(pktio_entry_t *pktio_entry,

>>>                 if (odp_unlikely(pool == ODP_POOL_INVALID))

>>>                         ODP_ABORT("invalid pool");

>>>

>>> -#ifdef _ODP_PKTIO_IPC

>>>                 data_pool_off = phdr->buf_hdr.seg[0].ipc_data_offset;

>>> -#else

>>> -               /* compile all function code even if ipc disabled with config */

>>> -               data_pool_off = 0;

>>> -#endif

>>>

>>>                 pkt = odp_packet_alloc(pool, phdr->frame_len);

>>>                 if (odp_unlikely(pkt == ODP_PACKET_INVALID)) {

>>> @@ -590,7 +585,6 @@ static int ipc_pktio_send_lockless(pktio_entry_t *pktio_entry,

>>>                 data_pool_off = (uint8_t *)pkt_hdr->buf_hdr.seg[0].data -

>>>                                 (uint8_t *)odp_shm_addr(pool->shm);

>>>

>>> -#ifdef _ODP_PKTIO_IPC

>>>                 /* compile all function code even if ipc disabled with config */

>>>                 pkt_hdr->buf_hdr.seg[0].ipc_data_offset = data_pool_off;

>>>                 IPC_ODP_DBG("%d/%d send packet %llx, pool %llx,"

>>> @@ -598,7 +592,6 @@ static int ipc_pktio_send_lockless(pktio_entry_t *pktio_entry,

>>>                             i, len,

>>>                             odp_packet_to_u64(pkt), odp_pool_to_u64(pool_hdl),

>>>                             pkt_hdr, pkt_hdr->buf_hdr.seg[0].ipc_data_offset);

>>> -#endif

>>>         }

>>>

>>>         /* Put packets to ring to be processed by other process. */

>>> --

>>> 2.7.1.250.gff4ea60

>>>

>
diff mbox

Patch

diff --git a/platform/linux-generic/include/odp_buffer_internal.h b/platform/linux-generic/include/odp_buffer_internal.h
index 903f0a7..4cc51d3 100644
--- a/platform/linux-generic/include/odp_buffer_internal.h
+++ b/platform/linux-generic/include/odp_buffer_internal.h
@@ -64,11 +64,10 @@  struct odp_buffer_hdr_t {
 	struct {
 		void     *hdr;
 		uint8_t  *data;
-#ifdef _ODP_PKTIO_IPC
-	/* ipc mapped process can not walk over pointers,
-	 * offset has to be used */
+		/* Used only if _ODP_PKTIO_IPC is set.
+		 * ipc mapped process can not walk over pointers,
+		 * offset has to be used */
 		uint64_t ipc_data_offset;
-#endif
 		uint32_t  len;
 	} seg[CONFIG_PACKET_MAX_SEGS];
 
diff --git a/platform/linux-generic/pktio/ipc.c b/platform/linux-generic/pktio/ipc.c
index 5f26b56..c9df043 100644
--- a/platform/linux-generic/pktio/ipc.c
+++ b/platform/linux-generic/pktio/ipc.c
@@ -459,12 +459,7 @@  static int ipc_pktio_recv_lockless(pktio_entry_t *pktio_entry,
 		if (odp_unlikely(pool == ODP_POOL_INVALID))
 			ODP_ABORT("invalid pool");
 
-#ifdef _ODP_PKTIO_IPC
 		data_pool_off = phdr->buf_hdr.seg[0].ipc_data_offset;
-#else
-		/* compile all function code even if ipc disabled with config */
-		data_pool_off = 0;
-#endif
 
 		pkt = odp_packet_alloc(pool, phdr->frame_len);
 		if (odp_unlikely(pkt == ODP_PACKET_INVALID)) {
@@ -590,7 +585,6 @@  static int ipc_pktio_send_lockless(pktio_entry_t *pktio_entry,
 		data_pool_off = (uint8_t *)pkt_hdr->buf_hdr.seg[0].data -
 				(uint8_t *)odp_shm_addr(pool->shm);
 
-#ifdef _ODP_PKTIO_IPC
 		/* compile all function code even if ipc disabled with config */
 		pkt_hdr->buf_hdr.seg[0].ipc_data_offset = data_pool_off;
 		IPC_ODP_DBG("%d/%d send packet %llx, pool %llx,"
@@ -598,7 +592,6 @@  static int ipc_pktio_send_lockless(pktio_entry_t *pktio_entry,
 			    i, len,
 			    odp_packet_to_u64(pkt), odp_pool_to_u64(pool_hdl),
 			    pkt_hdr, pkt_hdr->buf_hdr.seg[0].ipc_data_offset);
-#endif
 	}
 
 	/* Put packets to ring to be processed by other process. */