RE: [lwip-users] about alignment issues.

[Curt McDowell]

Hi Pedro,

Unions are really ugly, and I don't think they're necessary in pbuf.c.  But
your idea to use an embedded structure is good.  How about the following?

struct {
  struct pbuf pbuf;
  u8_t payload[MEM_ALIGN_SIZE(PBUF_POOL_BUFSIZE)];
} pbuf_pool_memory[PBUF_POOL_SIZE];

The alignment requirement of a structure is the maximum of the alignment
requirements of any member of the structure.  struct pbuf is intrinsically
aligned.  This alignment is inherited by the outer structure and the payload is
also suitably aligned.  The MEM_ALIGN_SIZE() above is not necessary, but helps
to clarify what is actually happening.  Here is a corresponding pbuf_init()
similar to yours (and tested in my app).

void
pbuf_init(void)
{
  u16_t i;
  struct pbuf *p;

#if PBUF_STATS
  lwip_stats.pbuf.avail = PBUF_POOL_SIZE;
#endif /* PBUF_STATS */

  pbuf_pool = NULL;

  for (i = 0; i < PBUF_POOL_SIZE; i++) {
    p = (struct pbuf *)&pbuf_pool_memory[i];
    p->payload = pbuf_pool_memory[i].payload;
    p->len = p->tot_len = MEM_ALIGN_SIZE(PBUF_POOL_BUFSIZE);
    p->flags = PBUF_FLAG_POOL;
    p->next = pbuf_pool;
    pbuf_pool = p;
  }

#if !SYS_LIGHTWEIGHT_PROT
  pbuf_pool_alloc_lock = 0;
  pbuf_pool_free_lock = 0;
  pbuf_pool_free_sem = sys_sem_new(1);
#endif
}

Regards,
Curt McDowell
Broadcom Corp.

> -----Original Message-----
> From: address@hidden 
> [mailto:address@hidden On 
> Behalf Of Pedro Alves
> Sent: Thursday, April 13, 2006 11:05 AM
> To: Mailing list for lwIP users
> Subject: [lwip-users] about alignment issues.
> 
> Hi all,
> 
> Is there a reason the structures that need a forced alignment 
> in lwip aren't declared something like this:
> 
> take pbuf.c for example:
> 
> typedef unsigned long LWIP_ALIGN_TYPE;
> 
> struct pool_pbuf
> {
>   struct pbuf pbuf; // "inherit" a pbuf. C guaranties the 
> first member 
> takes the same address of the container.      pool_pbuf b; &b 
> == &b.pbuf 
> -> (struct pbuf* )&b is ok.
>   union
>   {
>     LWIP_ALIGN_TYPE force_align;
>     u8_t payload_buf[MEM_ALIGN_SIZE(PBUF_POOL_BUFSIZE)];
>   };
> };
> 
> static struct pool_pbuf mem[PBUF_POOL_SIZE];
> 
> Then in pbuf_init:
> 
> void
> pbuf_init(void)
> {
>   u16_t i;
> 
>   pbuf_pool = (struct pbuf* )mem; // already aligned
> 
> #if PBUF_STATS
>   lwip_stats.pbuf.avail = PBUF_POOL_SIZE; #endif /* PBUF_STATS */
> 
>   /* Set up ->next pointers to link the pbufs of the pool together */
>   for(i = 0; i < PBUF_POOL_SIZE; ++i) {
>     p = (struct pbuf* )mem[i];
>     p->next = (struct pbuf*)mem[i+1];
>     p->len = p->tot_len = MEM_ALIGN_SIZE(PBUF_POOL_BUFSIZE);
>     p->payload = mem[i].payload_buf;
>     p->flags = PBUF_FLAG_POOL;
>   }
> 
>   /* The ->next pointer of last pbuf is NULL to indicate that there
>      are no more pbufs in the pool */
>    mem[PBUF_POOL_SIZE-1]->next = NULL;
> 
> #if !SYS_LIGHTWEIGHT_PROT
>   pbuf_pool_alloc_lock = 0;
>   pbuf_pool_free_lock = 0;
>   pbuf_pool_free_sem = sys_sem_new(1);
> #endif
> }
> 
> and where an offset if requested:
> p->payload = MEM_ALIGN(p->payload_buf + offset);
> 
> If there are compilers that don't support anonymouse unions 
> we can have simple macros and named unions.
> 
> I never tested this, but I am sure that this would shave a 
> few ram bytes in many arquitectures. Right now we allocate 
> more than needed.
> For example:
> u8_t mem[MEM_ALIGNMENT - 1 + PBUF_POOL_SIZE * 
> MEM_ALIGN_SIZE(PBUF_POOL_BUFSIZE + sizeof(struct pbuf))];
> 
> allocates MEM_ALIGNMENT - 1 bytes too much if mem ends up 
> already aligned.
> 
> What do you think?
> 
> Cheers,
> Pedro Alves
> 
> 
> 
> _______________________________________________
> lwip-users mailing list
> address@hidden
> http://lists.nongnu.org/mailman/listinfo/lwip-users
>