• State Verified Answer
  • +2 person also asked this people also asked this
  • Replies 18 replies
  • Subscribers 65 subscribers
  • Views 10525 views
  • Users 0 members are here
Attachments (2) Download All
Nordic Case Info
  • Case ID: 129091
Options
This post is older than 2 years and might not be relevant anymore
More Info: Consider searching for newer posts

nrf_log fixes in SDK14.1.0

Keton

I've noticed following line in SDK14.1 release notes:

`

  • Fixed a bug where nrf_log could crash when heavily loaded. `

I'm using SDK13.1 in my current project and I'm experiencing nrf_log related crashes under load as described here.

Please elaborate on what exactly was fixed and if it's related to problems I'm experiencing. Also if possible please provide some pointers on how to backport this fix into SDK13.1. Unfortunately I can't upgrade to SDK14.1 at this time.

Parents
  • 0

    It seems that fix posted above is only valid when NRF_LOG_DEFERRED_BUFSIZE is smaller than logged data amount. For large NRF_LOG_DEFERRED_BUFSIZE patch above leads to a hardfault while original buffer size estimate works.

    I know that Nordic devs are busy preparing SDK15 release but for all users like me who are stuck with SDK13 I'd really appreciate that someone who knows internals of nrf_log module takes a look if this approach is correct.

    diff --git a/components/libraries/log/src/nrf_log_frontend.c b/components/libraries/log/src/nrf_log_frontend.c
index e8f80c10..e6f5dfd7 100644
--- a/components/libraries/log/src/nrf_log_frontend.c
+++ b/components/libraries/log/src/nrf_log_frontend.c
@@ -379,8 +379,25 @@ static inline uint32_t * cont_buf_prealloc(uint32_t len32,

     CRITICAL_REGION_ENTER();
     *p_wr_idx = m_log_data.wr_idx;
+
     uint32_t available_words = (m_log_data.mask + 1) -
                                (m_log_data.wr_idx & m_log_data.mask);
+
+       /*
+               This is more robust attempt of fixing NRF_LOG_DEFFERED=1 hardfault.
+               Nordic posted fix here: devzone.nordicsemi.com/.../
+               The change works only when NRF_LOG_DEFERRED_BUFSIZE isn't large enough to fit all log data before log processing happens.
+               When NRF_LOG_DEFERRED_BUFSIZE is large i.e. 4096 words proposed fix leads to hardfault by overflowing subtraction result
+
+               This is an attempt of more robust fix which assumes that smaller free space estimate is correct.
+       */
+
+       //this is how Nordic proposes to fix it
+       uint32_t available_words_alt=(m_log_data.mask + 1) - (m_log_data.wr_idx - m_log_data.rd_idx);
+
+       //assume that smaller value is correct
+       if(available_words_alt<available_words) available_words=available_words_alt;
+
     if (len32 <= available_words)
     {
         // buffer will fit as is
--
Reply
  • 0

    It seems that fix posted above is only valid when NRF_LOG_DEFERRED_BUFSIZE is smaller than logged data amount. For large NRF_LOG_DEFERRED_BUFSIZE patch above leads to a hardfault while original buffer size estimate works.

    I know that Nordic devs are busy preparing SDK15 release but for all users like me who are stuck with SDK13 I'd really appreciate that someone who knows internals of nrf_log module takes a look if this approach is correct.

    diff --git a/components/libraries/log/src/nrf_log_frontend.c b/components/libraries/log/src/nrf_log_frontend.c
index e8f80c10..e6f5dfd7 100644
--- a/components/libraries/log/src/nrf_log_frontend.c
+++ b/components/libraries/log/src/nrf_log_frontend.c
@@ -379,8 +379,25 @@ static inline uint32_t * cont_buf_prealloc(uint32_t len32,

     CRITICAL_REGION_ENTER();
     *p_wr_idx = m_log_data.wr_idx;
+
     uint32_t available_words = (m_log_data.mask + 1) -
                                (m_log_data.wr_idx & m_log_data.mask);
+
+       /*
+               This is more robust attempt of fixing NRF_LOG_DEFFERED=1 hardfault.
+               Nordic posted fix here: devzone.nordicsemi.com/.../
+               The change works only when NRF_LOG_DEFERRED_BUFSIZE isn't large enough to fit all log data before log processing happens.
+               When NRF_LOG_DEFERRED_BUFSIZE is large i.e. 4096 words proposed fix leads to hardfault by overflowing subtraction result
+
+               This is an attempt of more robust fix which assumes that smaller free space estimate is correct.
+       */
+
+       //this is how Nordic proposes to fix it
+       uint32_t available_words_alt=(m_log_data.mask + 1) - (m_log_data.wr_idx - m_log_data.rd_idx);
+
+       //assume that smaller value is correct
+       if(available_words_alt<available_words) available_words=available_words_alt;
+
     if (len32 <= available_words)
     {
         // buffer will fit as is
--
Children
  • 0 in reply to Keton

    Hi,

    we have made significant changes in nrf_log in SDK 14.0 and then bunch of stability fixes in SDK 14.1 It is hard to provide the patch for them. You can try to port latest nrf_log to SDK 13. Here is the migration guide which might be helpful:

  • 0 in reply to Keton

    Since Nordic provided the 'fixed' snippet at the top of this post I was hoping that someone who knows how this is supposed to work may come up with correct available_words estimate equation. Original doesn't handle overflows correctly, your proposed fix doesn't handle no overflow situations. Please at least advise is there situation where they both fail? I'm not that familiar with nrf_log internals to be able to judge that.

    As for migrating to more recent SDK/backporting parts of SDK14 this is exactly what I'm trying to avoid. The product I'm currently developing is near release so we cannot afford any significant changes.

    BTW it may be a good idea to consider having long term support version of the SDK with focus on no bugs and stability every once in a while. IMHO some developers (including me) would really appreciate codebase that maybe is not feature complete but is thoroughly tested both by Nordic and by the community and it's known to contain (almost) no bugs.

Related