All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel THOMPSON <daniel.thompson@st.com>
To: fundu_1999@yahoo.com
Cc: Mike Frysinger <vapier.adi@gmail.com>,
	linux embedded <linux-embedded@vger.kernel.org>
Subject: Re: local_save_flags(flags)
Date: Fri, 19 Sep 2008 10:52:29 +0100	[thread overview]
Message-ID: <48D3765D.3080408@st.com> (raw)
In-Reply-To: <545834.68406.qm@web63405.mail.re1.yahoo.com>

Fundu wrote:
> 
>> that book explicitly covers your question.  read chapter 2
>> where it
>> covers these irq functions.
> as i said i'm reading the book and actully i did read that chapter. 
> 
> for me, here's the exact line that needs clarification,
> In chapter 2, page 42 last paragraph(starts with "However, if ..." )
> 
> here's the code snippet he's talking about.
> Point A:
> local_irq_disable();
> /* critical section ...*/
> local_irq_enable();
> 
> 
> Author say, if irg are already disabled at Point A (see snippet above) then local_irq_enable() creates an unpleasant side effect of re-enabling interrupts rather than restoring interrupt state. 
> 
> 1) first what's the difference between re-enabling and restoring interrupt state. 
> 2) so is disable interrupts twice a problem, or just enabling them when after they are diabled (which sounds like how it should be ) a problem.
> 
> hope i have made myself clear enough for you to respond.

I think you are confused by the term 'flags'. In this case 'flags' is
not the interrupts that are pending, there are the interrupts that where
enabled before calling local_irq_save().

Does an example help? Consider driver A that calls 'library' code B.

void driver_A()
{
  local_irq_disable();
  /* do something critical */
  library_code_B();
  /* do something else critical */
  local_irq_enable();

  /* NO BUG - if interrupts are not locked when local_irq_disable()
   * was called.
   */
}

void library_code_B(void)
{
#if BUGGY_CODE_COMES_FIRST
    local_irq_disable();
    /* do something critical */
    local_irq_enable();

    /* BUG HERE - driver A thinks interrupts are still disabled but
     * they are not
     */
#else
   flags = local_irq_save();
   /* do something critical */
   local_irq_restore(flags);

   /* NO BUG - interrupts are still locked (flags is used to remember
    * that interrupts were locked when we called local_irq_save().
    */
#endif
}

In fewer words: use local_irq_save/restore() if you don't know whether
interrupts where locked or not when your function was called.

PS There are only a small number of drivers that should use the
   local_irq_... family of functions anyway. Normally you should use
   create a spin lock and use spin_lock_irq() and spin_lock_irqsave()
   instead.

-- 
Daniel Thompson (STMicroelectronics) <daniel.thompson@st.com>
1000 Aztec West, Almondsbury, Bristol, BS32 4SQ. 01454 462659

If a car is a horseless carriage then is a motorcycle a horseless horse?

  parent reply	other threads:[~2008-09-19  9:52 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <556445368AFA1C438794ABDA8901891C098AABE6@USA0300MS03.na.xerox.net>
2008-09-18 21:09 ` local_save_flags(flags) Fundu
2008-09-18 21:15   ` local_save_flags(flags) Mike Frysinger
2008-09-18 22:05     ` local_save_flags(flags) Fundu
2008-09-18 22:07       ` local_save_flags(flags) Mike Frysinger
2008-09-18 22:45         ` local_save_flags(flags) Fundu
2008-09-18 22:51           ` local_save_flags(flags) Mike Frysinger
2008-09-23 20:01             ` local_save_flags(flags) Fundu
2008-09-23 23:11               ` local_save_flags(flags) Mike Frysinger
2008-09-24  1:15                 ` local_save_flags(flags) Fundu
2008-09-24  1:31                   ` local_save_flags(flags) Mike Frysinger
2008-09-24 17:06                     ` local_save_flags(flags) Fundu
2008-09-29  3:32                       ` local_save_flags(flags) Robin Getz
2008-09-19  9:52           ` Daniel THOMPSON [this message]
2008-09-18 18:48 local_save_flags(flags) Fundu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=48D3765D.3080408@st.com \
    --to=daniel.thompson@st.com \
    --cc=fundu_1999@yahoo.com \
    --cc=linux-embedded@vger.kernel.org \
    --cc=vapier.adi@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.