buglwIP - A Lightweight TCP/IP stack - Bugs: bug #50838, mem.c needs SYS_ARCH_PROTECTION...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #50838: mem.c needs SYS_ARCH_PROTECTION around MEM_STATS

Submitted by:  preet <preetpal>
Submitted on:  Thu 20 Apr 2017 07:16:14 PM UTC  
 
Category:  pbufs Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  Simon Goldschmidt <goldsimon>
Open/Closed:  Closed Planned Release:  None
lwIP version:  2.0.0

( Jump to the original submission)

Wed 08 Nov 2017 04:41:13 PM UTC, comment #6: 

After updating to 2.0.3 I noticed that my peek memory usage was reading 0. It looks like the original fix for this issue, which was reverted and replaced by this, was cherry picked into 2.0.3.

Billy Bednar <bb1>
Mon 24 Apr 2017 10:24:15 AM UTC, comment #5: 

Daniel, you're right. I've reverted the commit and pushed a better fix (that will hopefully work for everyone now :-)

Thanks for checking.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Fri 21 Apr 2017 08:49:47 PM UTC, comment #4: 

I just had a look at the code in mem.c which uses these macros and I noticed that these increments/decrements are already protected by a mutex -- at least if MEM_LIBC_MALLOC is 0.

So I think it might be preferable to have the extra locks only where needed, and change the MEM_STATS macros back to unlocked.

Daniel Elstner <danielk>
Fri 21 Apr 2017 08:03:53 PM UTC, comment #3: 

Seems like this fix broke stats counting. STATS_INC_USED is defined as thus:

#define STATS_INC_USED(x, y) do { lwip_stats.x.used += y; \
                                if (lwip_stats.x.max < lwip_stats.x.used) { \
                                    lwip_stats.x.max = lwip_stats.x.used; \
                                } \
                             } while(0)

The commit replaces this with:

#define MEM_STATS_INC_USED(x, y) SYS_ARCH_INC(lwip_stats.mem.x, y)

Which is clearly not the same thing, and explains why my memory usage is shown as zero.

Daniel Elstner <danielk>
Thu 20 Apr 2017 07:48:09 PM UTC, comment #2: 
Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 20 Apr 2017 07:47:46 PM UTC, comment #1: 

Fixed in stats.h, thanks for reporting.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Thu 20 Apr 2017 07:16:14 PM UTC, original submission:  

After running regression tests against LWIP, it was noticed that the Memory stats seemed to roll-over.  The tests should have had the numbers in the range of 0-20K, but instead it was close to the overflow value of uint16_t

lwip_stats.mem.used, lwip_stats.mem.max

We made the fixes locally and proved that the code fix worked.
At mem.c: we need:

  SYS_ARCH_PROTECT(lev);
  MEM_STATS_INC_USED(used, element->size);
  SYS_ARCH_UNPROTECT(lev);

  SYS_ARCH_PROTECT(lev);
  MEM_STATS_DEC_USED(used, hmem->size);
  SYS_ARCH_UNPROTECT(lev);

preet <preetpal>

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by bb1 (Posted a comment)
  • -email is unavailable- added by danielk (Posted a comment)
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by preetpal (Submitted the item)
  • -email is unavailable- added by preetpal
  •  

    Do you think this task is very important?
    If so, you can add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

     

     

    Follow 4 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2017-04-20 goldsimon StatusNone => Fixed
        Assigned toNone => goldsimon
        Open/ClosedOpen => Closed
    2017-04-20 preetpal Carbon-Copy- => Added preetpal

    Back to the top


    Powered by Savane 3.5