On Mon, Jan 21, 2013 at 06:58:38PM +0000, Wyborny, Carolyn wrote:
> > -----Original Message-----
> > From: Dan Carpenter [mailto:[email protected]]
> > Sent: Monday, January 21, 2013 10:36 AM
> > To: Wyborny, Carolyn
> > Cc: [email protected]
> > Subject: re: igb: Add i2c interface to igb.
> > 
> > Hello Carolyn Wyborny,
> > 
> > The patch 441fc6fdb47a: "igb: Add i2c interface to igb." from Dec 7, 2012, 
> > leads
> > to the following warning:
> > drivers/net/ethernet/intel/igb/igb_main.c:7641 igb_get_i2c_client()
> >      error: scheduling with locks held: 'spin_lock:i2c_clients_lock'
> > 
> >   7622          spin_lock_irqsave(&i2c_clients_lock, flags);
> >                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> > We take the spin_lock here.
> > 
> >   7623          client_list = adapter->i2c_clients;
> >   7624
> >   7625          /* See if we already have an i2c_client */
> >   7626          while (client_list) {
> >   7627                  if (client_list->client->addr == (dev_addr >> 1)) {
> >   7628                          client = client_list->client;
> >   7629                          goto exit;
> >   7630                  } else {
> >   7631                          client_list = client_list->next;
> >   7632                  }
> >   7633          }
> >   7634
> >   7635          /* no client_list found, create a new one as long as
> >   7636           * irqs are not disabled
> >   7637           */
> >   7638          if (unlikely(irqs_disabled()))
> >                     ^^^^^^^^^^^^^^^^^^^^^^^^^ IRQs are always disabled 
> > here, so we
> > never get to the kzalloc() actually.
> > 
> >   7639                  goto exit;
> >   7640
> >   7641          client_list = kzalloc(sizeof(*client_list), GFP_KERNEL);
> >                                                             ^^^^^^^^^^ We 
> > can sleep here.
> > 
> >   7642          if (client_list == NULL)
> >   7643                  goto exit;
> > 
> > 
> > regards,
> > dan carpenter
> 
> Hello Dan,
> 
> We'll take a look at it.  This was tested here and has gone
> through multiple revisions internally but I'm pretty sure we
> didn't see this.   Can you provide some info so we can try and
> repro this?  Can you provide a copy of your .config so we can see
> what modules are enabled.  Also an output of lspci -vvv would be
> useful as well.

It's just static analysis and reading the code (today's linux-next).

We're not allowed to call kzalloc() with GFP_KERNEL while holding
a spinlock.  It will sometimes sleep and can lead to a lockup.  One
solution would be to use GFP_ATOMIC.  These bugs will generate a
warning if you have CONFIG_DEBUG_ATOMIC_SLEEP enabled.

But as I mentioned, we never hit the GFP_KERNEL bug because we
always take the irqs_disabled() goto exit path since we disabled
IRQs earlier in the function by calling spin_lock_irqsave().

regards,
dan carpenter


------------------------------------------------------------------------------
Master Visual Studio, SharePoint, SQL, ASP.NET, C# 2012, HTML5, CSS,
MVC, Windows 8 Apps, JavaScript and much more. Keep your skills current
with LearnDevNow - 3,200 step-by-step video tutorials by Microsoft
MVPs and experts. SALE $99.99 this month only -- learn more at:
http://p.sf.net/sfu/learnmore_122412
_______________________________________________
E1000-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/e1000-devel
To learn more about Intel® Ethernet, visit 
http://communities.intel.com/community/wired

Reply via email to