Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-23 Thread Tim Small
I've been using Antti's fix for a while, it seems to be working fine.  
Any chance of getting it pushed into the main repository?


Cheers,

Tim.



Removing of unnecessary spinlock

Based on the discussion on linux-dvb it seems that ISR:s are already
serialized by the linux kernel and thus drivers won't have to specify
their own facilities for ISR serialization.

Signed-off-by: Antti Seppälä [EMAIL PROTECTED]


--
Antti Seppälä

--- linux/drivers/media/dvb/b2c2/flexcop-pci.c~ 2007-04-07 09:44:50.0 
+0300
+++ linux/drivers/media/dvb/b2c2/flexcop-pci.c  2007-04-07 09:43:27.0 
+0300
@@ -59,8 +59,6 @@
u32 last_dma1_cur_pos; /* position of the pointer last time the 
timer/packet irq occured */
int count;
 
-	spinlock_t irq_lock;

-
unsigned long last_irq;
 
 #if LINUX_VERSION_CODE  KERNEL_VERSION(2,6,20)

@@ -143,12 +141,9 @@
 {
struct flexcop_pci *fc_pci = dev_id;
struct flexcop_device *fc = fc_pci-fc_dev;
-   unsigned long flags;
flexcop_ibi_value v;
irqreturn_t ret = IRQ_HANDLED;
 
-	spin_lock_irqsave(fc_pci-irq_lock,flags);

-
v = fc-read_ibi_reg(fc,irq_20c);
 
/* errors */

@@ -211,8 +206,6 @@
ret = IRQ_NONE;
}
 
-	spin_unlock_irqrestore(fc_pci-irq_lock,flags);

-
return ret;
 }
 
@@ -310,7 +303,6 @@

}
 
 	pci_set_drvdata(fc_pci-pdev, fc_pci);

-   spin_lock_init(fc_pci-irq_lock);
if ((ret = request_irq(fc_pci-pdev-irq, flexcop_pci_isr,
IRQF_SHARED, DRIVER_NAME, fc_pci)) != 0)
goto err_pci_iounmap;
  



___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb



___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-08 Thread Antti Seppälä

mp3geek wrote:


safe to use this directly on the 2.6.20.x source?



The fix is valid but the patch I posted won't apply to 2.6.20. Use this
one instead.


--
Antti Seppälä

--- linux-2.6.20/drivers/media/dvb/b2c2/flexcop-pci.c~	2007-02-04 20:44:54.0 +0200
+++ linux-2.6.20/drivers/media/dvb/b2c2/flexcop-pci.c	2007-04-04 22:10:25.0 +0300
@@ -59,8 +59,6 @@
 	u32 last_dma1_cur_pos; /* position of the pointer last time the timer/packet irq occured */
 	int count;
 
-	spinlock_t irq_lock;
-
 	unsigned long last_irq;
 
 	struct delayed_work irq_check_work;
@@ -130,8 +128,6 @@
 	flexcop_ibi_value v;
 	irqreturn_t ret = IRQ_HANDLED;
 
-	spin_lock_irq(fc_pci-irq_lock);
-
 	v = fc-read_ibi_reg(fc,irq_20c);
 
/* errors */
@@ -194,8 +190,6 @@
 		ret = IRQ_NONE;
 	}
 
-	spin_unlock_irq(fc_pci-irq_lock);
-
 	return ret;
 }
 
@@ -298,8 +292,6 @@
 	IRQF_SHARED, DRIVER_NAME, fc_pci)) != 0)
 		goto err_pci_iounmap;
 
-	spin_lock_init(fc_pci-irq_lock);
-
 	fc_pci-init_state |= FC_PCI_INIT;
 	return ret;
 
___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-08 Thread Tim Small

Antti Seppälä wrote:

Removing of unnecessary spinlock

Based on the discussion on linux-dvb it seems that ISR:s are already
serialized by the linux kernel and thus drivers won't have to specify
their own facilities for ISR serialization.
There is a good summary of concurrency and ISRs on page 268 Linux Device 
Drivers 3rd Edition:


http://lwn.net/images/pdf/LDD3/ch10.pdf

Thanks for the debugging!

Cheers,

Tim.

___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-08 Thread Tim Small

Tim Small wrote:
There is a good summary of concurrency and ISRs on page 268 Linux 
Device Drivers 3rd Edition:


http://lwn.net/images/pdf/LDD3/ch10.pdf


Sorry, I should also have referenced this:

http://lwn.net/Kernel/LDD3/

Cheers,

Tim.

___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-07 Thread Antti Seppälä

Patrick Boettcher wrote:

On Fri, 6 Apr 2007, Antti Seppälä wrote:


Patrick Boettcher wrote:


Someone outthere with a SMP-system and a flexcop who could see if e.g.
dvbscan or dvbtraffic is killing the system/flexcop when removing the
irq_lock?

Patrick.


I've now been torturing my SMP box for over three hours with various dvb
-related activities (dvbtraffic, dvbscan, vdr) without the irq_lock in flexcop
driver. So far I haven't noticed any problems in operation whatsoever.

Maybe the spinlock really should go?


I vote for it, too.

If someone will have a problem with the flexcop we will see during 
development phase of the kernel after integration.


regards,
Patrick.



I've created a patch for removing the spinlock from latest v4l-dvb -tree:


Removing of unnecessary spinlock

Based on the discussion on linux-dvb it seems that ISR:s are already
serialized by the linux kernel and thus drivers won't have to specify
their own facilities for ISR serialization.

Signed-off-by: Antti Seppälä [EMAIL PROTECTED]


--
Antti Seppälä
--- linux/drivers/media/dvb/b2c2/flexcop-pci.c~	2007-04-07 09:44:50.0 +0300
+++ linux/drivers/media/dvb/b2c2/flexcop-pci.c	2007-04-07 09:43:27.0 +0300
@@ -59,8 +59,6 @@
 	u32 last_dma1_cur_pos; /* position of the pointer last time the timer/packet irq occured */
 	int count;
 
-	spinlock_t irq_lock;
-
 	unsigned long last_irq;
 
 #if LINUX_VERSION_CODE  KERNEL_VERSION(2,6,20)
@@ -143,12 +141,9 @@
 {
 	struct flexcop_pci *fc_pci = dev_id;
 	struct flexcop_device *fc = fc_pci-fc_dev;
-	unsigned long flags;
 	flexcop_ibi_value v;
 	irqreturn_t ret = IRQ_HANDLED;
 
-	spin_lock_irqsave(fc_pci-irq_lock,flags);
-
 	v = fc-read_ibi_reg(fc,irq_20c);
 
/* errors */
@@ -211,8 +206,6 @@
 		ret = IRQ_NONE;
 	}
 
-	spin_unlock_irqrestore(fc_pci-irq_lock,flags);
-
 	return ret;
 }
 
@@ -310,7 +303,6 @@
 	}
 
 	pci_set_drvdata(fc_pci-pdev, fc_pci);
-	spin_lock_init(fc_pci-irq_lock);
 	if ((ret = request_irq(fc_pci-pdev-irq, flexcop_pci_isr,
 	IRQF_SHARED, DRIVER_NAME, fc_pci)) != 0)
 		goto err_pci_iounmap;
___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-07 Thread mp3geek

safe to use this directly on the 2.6.20.x source?

I've created a patch for removing the spinlock from latest v4l-dvb -tree:



Removing of unnecessary spinlock

Based on the discussion on linux-dvb it seems that ISR:s are already
serialized by the linux kernel and thus drivers won't have to specify
their own facilities for ISR serialization.

Signed-off-by: Antti Seppälä [EMAIL PROTECTED]


--
Antti Seppälä



___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-06 Thread Patrick Boettcher
On Thu, 5 Apr 2007, Trent Piepho wrote:

 On Thu, 5 Apr 2007, Patrick Boettcher wrote:
  On Thu, 5 Apr 2007, Borgi2008 wrote:
   Am Mittwoch, den 04.04.2007, 23:29 +0300 schrieb Antti Sepp?l?:
   
Actually, looking at the code I cannot figure out why there has to be a
spinlock in the first place.
   
The lock is only taken in the interrupt handler which already runs in
atomic context so there is no use in making the handler protected by a
spinlock. Am I missing something here?
   
I think the spinlock is unnecessary and should be removed entirely.
 
  Even on SMP systems? ISRs are only atomic on one CPU.
 
 I thought ISRs were normally atomic even on SMP systems.  When an interrupt
 occurs, that interrupt is disabled until the ISR returns.  Except in fast
 handlers (which flexcop is not) all interrupts aren't disabled, and so an
 ISR can be interrupted by a _different_ ISR, and a different ISR could be
 running at the same time on another CPU.  But an ISR doesn't have to worry
 about two copies of itself running at the same time.
 
 At least, that's how I think it works.
 
 I don't see how a spin lock that is only used from the ISR could be useful,
 assuming the ISR is only called via an interrupt.  There is no reason you
 couldn't call the isr function from some system call handler, but that
 would be an unusual thing to do.
 
 Normally an ISR does need a spinlock, to access data shared by the
 non-interrupt part of the driver.  I wonder if there is a bug in flexcop in
 that nothing else uses irq_lock?

You can only take a spinlock at IRQ-level and not at normal kernel level. 
So you cannot protect from shared data access without delegating the work 
to a lower level and there to use mutexes to protect.

As there is only the isr which is at IRQ-level you might be right, it can 
be removed. 

Someone outthere with a SMP-system and a flexcop who could see if e.g. 
dvbscan or dvbtraffic is killing the system/flexcop when removing the 
irq_lock?

Patrick.
___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-06 Thread Antti Seppälä

Patrick Boettcher wrote:

Someone outthere with a SMP-system and a flexcop who could see if e.g. 
dvbscan or dvbtraffic is killing the system/flexcop when removing the 
irq_lock?


Patrick.



I've now been torturing my SMP box for over three hours with various dvb 
-related activities (dvbtraffic, dvbscan, vdr) without the irq_lock in 
flexcop driver. So far I haven't noticed any problems in operation 
whatsoever.


Maybe the spinlock really should go?

--
Antti Seppälä


___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-06 Thread Patrick Boettcher
On Fri, 6 Apr 2007, Antti Seppälä wrote:

 Patrick Boettcher wrote:
 
  Someone outthere with a SMP-system and a flexcop who could see if e.g.
  dvbscan or dvbtraffic is killing the system/flexcop when removing the
  irq_lock?
  
  Patrick.
 
 
 I've now been torturing my SMP box for over three hours with various dvb
 -related activities (dvbtraffic, dvbscan, vdr) without the irq_lock in flexcop
 driver. So far I haven't noticed any problems in operation whatsoever.
 
 Maybe the spinlock really should go?

I vote for it, too.

If someone will have a problem with the flexcop we will see during 
development phase of the kernel after integration.

regards,
Patrick.
___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-06 Thread Trent Piepho
On Fri, 6 Apr 2007, Patrick Boettcher wrote:
 On Thu, 5 Apr 2007, Trent Piepho wrote:
  On Thu, 5 Apr 2007, Patrick Boettcher wrote:
   On Thu, 5 Apr 2007, Borgi2008 wrote:
Am Mittwoch, den 04.04.2007, 23:29 +0300 schrieb Antti Sepp?l?:

 Actually, looking at the code I cannot figure out why there has to be 
 a
 spinlock in the first place.

 The lock is only taken in the interrupt handler which already runs in
 atomic context so there is no use in making the handler protected by a
 spinlock. Am I missing something here?

 I think the spinlock is unnecessary and should be removed entirely.
  
   Even on SMP systems? ISRs are only atomic on one CPU.
 
  I thought ISRs were normally atomic even on SMP systems.  When an interrupt
  occurs, that interrupt is disabled until the ISR returns.  Except in fast
  handlers (which flexcop is not) all interrupts aren't disabled, and so an
  ISR can be interrupted by a _different_ ISR, and a different ISR could be
  running at the same time on another CPU.  But an ISR doesn't have to worry
  about two copies of itself running at the same time.
 
  At least, that's how I think it works.
 
  I don't see how a spin lock that is only used from the ISR could be useful,
  assuming the ISR is only called via an interrupt.  There is no reason you
  couldn't call the isr function from some system call handler, but that
  would be an unusual thing to do.
 
  Normally an ISR does need a spinlock, to access data shared by the
  non-interrupt part of the driver.  I wonder if there is a bug in flexcop in
  that nothing else uses irq_lock?

 You can only take a spinlock at IRQ-level and not at normal kernel level.

Sure you can.  There is a ton of code that does this.  It would be pretty
much impossible to create SMP safe ISRs if there was no locking mechanism
that worked across interrupt-level to kernel-level.

Look in Documentation/cli-sti-removal.txt:

irq_handler (...)
{
unsigned long flags;

spin_lock_irqsave(driver_lock, flags);

driver_data.finish = 1;
driver_data.new_work = 0;

spin_unlock_irqrestore(driver_lock, flags);

}
ioctl_func (...)
{
...
spin_lock_irq(driver_lock);
...
driver_data.finish = 0;
driver_data.new_work = 2;
...
spin_unlock_irq(driver_lock);
...
}

___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-05 Thread Borgi2008
Am Mittwoch, den 04.04.2007, 23:29 +0300 schrieb Antti Seppälä:
 Borgi2008 wrote:
  Hello,
  
  i've created a bugfixes. Hope it could helps you.
  
  Hendrik Borghorst
  
  
 
 Actually, looking at the code I cannot figure out why there has to be a
 spinlock in the first place.
 
 The lock is only taken in the interrupt handler which already runs in
 atomic context so there is no use in making the handler protected by a
 spinlock. Am I missing something here?
 
 I think the spinlock is unnecessary and should be removed entirely.
 
 
 --
 Antti Seppälä

Hello,

it could be that the spinlock can be completely  removed. I only saw
that the spinlock is called before initialization. :-) So I fixed
because this crashed my system. I would like to see that the bug is
fixed official so i have not to patch my kernels anymore. 

Best greets

Hendrik Borghorst


___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-05 Thread Patrick Boettcher
On Thu, 5 Apr 2007, Borgi2008 wrote:

 Am Mittwoch, den 04.04.2007, 23:29 +0300 schrieb Antti Seppälä:
  Borgi2008 wrote:
   Hello,
   
   i've created a bugfixes. Hope it could helps you.
   
   Hendrik Borghorst
   
   
  
  Actually, looking at the code I cannot figure out why there has to be a
  spinlock in the first place.
  
  The lock is only taken in the interrupt handler which already runs in
  atomic context so there is no use in making the handler protected by a
  spinlock. Am I missing something here?
  
  I think the spinlock is unnecessary and should be removed entirely.

Even on SMP systems? ISRs are only atomic on one CPU.

Patrick.___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-05 Thread Michael Krufky
Patrick Boettcher wrote:
 On Thu, 5 Apr 2007, Borgi2008 wrote:
 
 Am Mittwoch, den 04.04.2007, 23:29 +0300 schrieb Antti Seppälä:
 Borgi2008 wrote:
 Hello,

 i've created a bugfixes. Hope it could helps you.

 Hendrik Borghorst


 Actually, looking at the code I cannot figure out why there has to be a
 spinlock in the first place.

 The lock is only taken in the interrupt handler which already runs in
 atomic context so there is no use in making the handler protected by a
 spinlock. Am I missing something here?

 I think the spinlock is unnecessary and should be removed entirely.
 
 Even on SMP systems? ISRs are only atomic on one CPU.

Redhat has a bugzilla ticket open about this issue.

Patrick, please take a look at the patch in bugzilla:

https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=234900

Regards,

Michael Krufky


___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-05 Thread Michael Krufky
Michael Krufky wrote:
 Patrick Boettcher wrote:
 On Thu, 5 Apr 2007, Borgi2008 wrote:

 Am Mittwoch, den 04.04.2007, 23:29 +0300 schrieb Antti Seppälä:
 Borgi2008 wrote:
 Hello,

 i've created a bugfixes. Hope it could helps you.

 Hendrik Borghorst


 Actually, looking at the code I cannot figure out why there has to be a
 spinlock in the first place.

 The lock is only taken in the interrupt handler which already runs in
 atomic context so there is no use in making the handler protected by a
 spinlock. Am I missing something here?

 I think the spinlock is unnecessary and should be removed entirely.
 Even on SMP systems? ISRs are only atomic on one CPU.
 
 Redhat has a bugzilla ticket open about this issue.
 
 Patrick, please take a look at the patch in bugzilla:
 
 https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=234900

Actually, the bugzilla patch is also from Hendrik Borghorst ...

Sorry about that... Nothing in the bugzilla report that hasnt already been
said in this thread.

-- 
Michael Krufky


___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-05 Thread Mauro Carvalho Chehab
Em Qua, 2007-04-04 às 15:28 +0200, P. van Gaans escreveu:
 I've got a Technisat Airstar USB that doesn't lock, so I'd like to try 
 the patch whatever it's for. How do I apply it?

I've just commited Hendrik's patch at v4l-dvb tree. So, you can just do:
hg clone http://linuxtv.org/hg/v4l-dvb
or
cd v4l-dvb;hg pull
(if you have already a copy of the tree)


 
 Borgi2008 wrote:
  Hello,
  
  i've created a bugfixes. Hope it could helps you.
  
  Hendrik Borghorst

Hendrik,

You've forgot to sign your patch. This is not really required for such
very simple fixes, but it would be nice if you can provide your SOB for
me to add on it, before I submit it to mainstream. 
 
Cheers,
Mauro


___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-05 Thread Antti Seppälä

Patrick Boettcher wrote:

On Thu, 5 Apr 2007, Borgi2008 wrote:


Am Mittwoch, den 04.04.2007, 23:29 +0300 schrieb Antti Seppälä:

Borgi2008 wrote:

Hello,

i've created a bugfixes. Hope it could helps you.

Hendrik Borghorst



Actually, looking at the code I cannot figure out why there has to be a
spinlock in the first place.

The lock is only taken in the interrupt handler which already runs in
atomic context so there is no use in making the handler protected by a
spinlock. Am I missing something here?

I think the spinlock is unnecessary and should be removed entirely.


Even on SMP systems? ISRs are only atomic on one CPU.

Patrick.


Apparently I've used to thinking too much in the UP world.

It seems that flexcop interrupts are not acked with a special register
write so an interrupt can then occur even while it is being processed on
another CPU.(?)

In that case the patch from Hendrik is correct.


--
Antti Seppälä

___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-05 Thread Trent Piepho
On Thu, 5 Apr 2007, Patrick Boettcher wrote:
 On Thu, 5 Apr 2007, Borgi2008 wrote:
  Am Mittwoch, den 04.04.2007, 23:29 +0300 schrieb Antti Sepp?l?:
  
   Actually, looking at the code I cannot figure out why there has to be a
   spinlock in the first place.
  
   The lock is only taken in the interrupt handler which already runs in
   atomic context so there is no use in making the handler protected by a
   spinlock. Am I missing something here?
  
   I think the spinlock is unnecessary and should be removed entirely.

 Even on SMP systems? ISRs are only atomic on one CPU.

I thought ISRs were normally atomic even on SMP systems.  When an interrupt
occurs, that interrupt is disabled until the ISR returns.  Except in fast
handlers (which flexcop is not) all interrupts aren't disabled, and so an
ISR can be interrupted by a _different_ ISR, and a different ISR could be
running at the same time on another CPU.  But an ISR doesn't have to worry
about two copies of itself running at the same time.

At least, that's how I think it works.

I don't see how a spin lock that is only used from the ISR could be useful,
assuming the ISR is only called via an interrupt.  There is no reason you
couldn't call the isr function from some system call handler, but that
would be an unusual thing to do.

Normally an ISR does need a spinlock, to access data shared by the
non-interrupt part of the driver.  I wonder if there is a bug in flexcop in
that nothing else uses irq_lock?

___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-04 Thread Borgi2008
Hello,

i've created a bugfixes. Hope it could helps you.

Hendrik Borghorst


flexcop-lockdep.patch
Description: application/mbox
___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-04 Thread P. van Gaans
I've got a Technisat Airstar USB that doesn't lock, so I'd like to try 
the patch whatever it's for. How do I apply it?


Borgi2008 wrote:

Hello,

i've created a bugfixes. Hope it could helps you.

Hendrik Borghorst




___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


Re: [linux-dvb] [BUG] flexcop lockdep

2007-04-04 Thread Antti Seppälä

Borgi2008 wrote:

Hello,

i've created a bugfixes. Hope it could helps you.

Hendrik Borghorst




Actually, looking at the code I cannot figure out why there has to be a
spinlock in the first place.

The lock is only taken in the interrupt handler which already runs in
atomic context so there is no use in making the handler protected by a
spinlock. Am I missing something here?

I think the spinlock is unnecessary and should be removed entirely.


--
Antti Seppälä

___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


Re: [linux-dvb] [BUG] flexcop lockdep

2007-03-22 Thread Tim Small

Using hg from last night, with 2.6.21-rc4, I get:

Mar 22 08:34:46 192.168.222.2 b2c2-flexcop: B2C2 FlexcopII/II(b)/III 
digital TV receiver chip loaded successfully

Mar 22 08:34:52 192.168.222.2 flexcop-pci: will use the HW PID filter.
Mar 22 08:34:52 192.168.222.2 flexcop-pci: card revision 2
Mar 22 08:34:52 192.168.222.2 ACPI: PCI Interrupt :04:09.0[A] -
Mar 22 08:34:52 192.168.222.2 GSI 21 (level, low) - IRQ 21
Mar 22 08:34:52 192.168.222.2 INFO: trying to register non-static key.
Mar 22 08:34:52 192.168.222.2 the code is fine but needs lockdep annotation.
Mar 22 08:34:52 192.168.222.2 turning off the locking correctness validator.
Mar 22 08:34:52 192.168.222.2
Mar 22 08:34:52 192.168.222.2 Call Trace:
Mar 22 08:34:52 192.168.222.2  [8029d49d] 
__lock_acquire+0x162/0xbe0
Mar 22 08:34:52 192.168.222.2  [8027839e] 
flush_kernel_map+0x0/0x82
Mar 22 08:34:52 192.168.222.2  [884ed1a1] 
:b2c2_flexcop_pci:flexcop_pci_isr+0x2e/0x2fe

Mar 22 08:34:52 192.168.222.2  [8029df97] lock_acquire+0x7c/0xa0
Mar 22 08:34:52 192.168.222.2  [884ed1a1] 
:b2c2_flexcop_pci:flexcop_pci_isr+0x2e/0x2fe
Mar 22 08:34:52 192.168.222.2  [884ed173] 
:b2c2_flexcop_pci:flexcop_pci_isr+0x0/0x2fe

Mar 22 08:34:52 192.168.222.2  [802628fd] _spin_lock_irq+0x2b/0x38
Mar 22 08:34:52 192.168.222.2  [884ed1a1] 
:b2c2_flexcop_pci:flexcop_pci_isr+0x2e/0x2fe
Mar 22 08:34:52 192.168.222.2  [8020a5c6] 
kmem_cache_alloc+0x101/0x111
Mar 22 08:34:52 192.168.222.2  [884ed173] 
:b2c2_flexcop_pci:flexcop_pci_isr+0x0/0x2fe

Mar 22 08:34:52 192.168.222.2  [802b3561] request_irq+0xce/0x11f
Mar 22 08:34:52 192.168.222.2  [884ed5f6] 
:b2c2_flexcop_pci:flexcop_pci_probe+0x185/0x2d4
Mar 22 08:34:52 192.168.222.2  [80340278] 
pci_device_probe+0xd6/0x13d

Mar 22 08:34:52 192.168.222.2  [8039992d] really_probe+0xce/0x162
Mar 22 08:34:52 192.168.222.2  [80399a6f] 
driver_probe_device+0xae/0xba
Mar 22 08:34:52 192.168.222.2  [80399bac] 
__driver_attach+0x89/0xc9

Mar 22 08:34:52 192.168.222.2  [80399b23] __driver_attach+0x0/0xc9
Mar 22 08:34:52 192.168.222.2  [80398d47] 
bus_for_each_dev+0x49/0x7a

Mar 22 08:34:52 192.168.222.2  [8039976b] driver_attach+0x1c/0x1e
Mar 22 08:34:52 192.168.222.2  [803990c3] 
bus_add_driver+0x78/0x19c
Mar 22 08:34:52 192.168.222.2  [80399e22] 
driver_register+0x85/0x89
Mar 22 08:34:52 192.168.222.2  [80340493] 
__pci_register_driver+0x95/0xd1
Mar 22 08:34:52 192.168.222.2  [8806701e] 
:b2c2_flexcop_pci:flexcop_pci_module_init+0x1e/0x20
Mar 22 08:34:52 192.168.222.2  [802a42d7] 
sys_init_module+0x15ca/0x1723
Mar 22 08:34:52 192.168.222.2  [8033b11c] 
pci_bus_read_config_byte+0x0/0x76
Mar 22 08:34:52 192.168.222.2  [8026215e] 
trace_hardirqs_on_thunk+0x35/0x37

Mar 22 08:34:52 192.168.222.2  [8025b11e] system_call+0x7e/0x83
Mar 22 08:34:52 192.168.222.2

The machine then reboots on watchdog.

Thanks,

Tim.


Nico Sabbi wrote:

Hi,
using a very recent hg pull I can see in the logs the following 
message (tuning and receiving work fine)


BUG: at kernel/lockdep.c:1860 trace_hardirqs_on()
 [c010318c] show_trace_log_lvl+0x19/0x2e
 [c0103285] show_trace+0x12/0x14
 [c010329b] dump_stack+0x14/0x16
 [c012c573] trace_hardirqs_on+0xc2/0x141
 [c02b2e91] _spin_unlock_irq+0x22/0x26
 [e0a75433] flexcop_pci_isr+0x294/0x29e [b2c2_flexcop_pci]
 [c013b557] handle_IRQ_event+0x18/0x44
 [c013ca39] handle_level_irq+0x93/0xe2
 [c0104176] do_IRQ+0x95/0xaf
 [c0102e4a] common_interrupt+0x2e/0x34
 [c01019e0] cpu_idle+0x41/0x59
 [c010033a] rest_init+0x1e/0x20
 [c0386700] start_kernel+0x309/0x30b
 [] 0x0
 ===



___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb



___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


Re: [linux-dvb] [BUG] flexcop lockdep

2007-03-22 Thread Tim Small

BTW, the non-static key message is from linux-2.6.21-rc4/kernel/lockdep.c.

Tim.



___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb


[linux-dvb] [BUG] flexcop lockdep

2007-03-07 Thread Nico Sabbi

Hi,
using a very recent hg pull I can see in the logs the following 
message (tuning and receiving work fine)


BUG: at kernel/lockdep.c:1860 trace_hardirqs_on()
 [c010318c] show_trace_log_lvl+0x19/0x2e
 [c0103285] show_trace+0x12/0x14
 [c010329b] dump_stack+0x14/0x16
 [c012c573] trace_hardirqs_on+0xc2/0x141
 [c02b2e91] _spin_unlock_irq+0x22/0x26
 [e0a75433] flexcop_pci_isr+0x294/0x29e [b2c2_flexcop_pci]
 [c013b557] handle_IRQ_event+0x18/0x44
 [c013ca39] handle_level_irq+0x93/0xe2
 [c0104176] do_IRQ+0x95/0xaf
 [c0102e4a] common_interrupt+0x2e/0x34
 [c01019e0] cpu_idle+0x41/0x59
 [c010033a] rest_init+0x1e/0x20
 [c0386700] start_kernel+0x309/0x30b
 [] 0x0
 ===



___
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb