Re: [PATCH v3] mailbox: PCC: Fix lockdep warning when request PCC channel
Hi, On Sun, Nov 13, 2016 at 11:44 AM, Prakash, Prashanth wrote: > HI Hoan, > > On 11/9/2016 11:39 PM, Hoan Tran wrote: >> Hi All, >> >> On Thu, Oct 27, 2016 at 3:34 PM, Hoan Tran wrote: >>> This patch fixes the lockdep warning below >>> >>> [7.229767] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags)) >>> [7.229776] [ cut here ] >>> [7.229787] WARNING: CPU: 1 PID: 1 at >>> linux-next/kernel/locking/lockdep.c:2876 loc >>> kdep_trace_alloc+0xe0/0xf0 >>> [7.229790] Modules linked in: >>> [7.229793] >>> [7.229798] CPU: 1 PID: 1 Comm: swapper/0 Not tainted >>> 4.8.0-11756-g86c5152 #46 >>> ... >>> [7.229900] Call trace: >>> [7.229903] Exception stack(0x8007da837890 to 0x8007da8379c0) >>> [7.229906] 7880: 8007da834000 >>> 0001 >>> [7.229909] 78a0: 8007da837a70 08a0 60c5 >>> 003d >>> [7.229912] 78c0: 9374bc6a7f3c7832 00381878 09db7ab8 >>> 002f >>> [7.229915] 78e0: 0811aabc 08be2548 8007da837990 >>> 0811adf8 >>> [7.229918] 7900: 8007da834000 024080c0 00c0 >>> 09021000 >>> [7.229921] 7920: 08c8f7c8 >>> 8007da579810 >>> [7.229923] 7940: 002f 8007da858000 >>> 0001 >>> [7.229926] 7960: 0001 0811a468 >>> 0002 >>> [7.229929] 7980: 656c62617369645f 00038187 00ee >>> 8007da837850 >>> [7.229932] 79a0: 09db50c0 09db569d 0006 >>> 89db568f >>> [7.229936] [] lockdep_trace_alloc+0xe0/0xf0 >>> [7.229940] [] __kmalloc_track_caller+0x50/0x250 >>> [7.229945] [] devres_alloc_node+0x28/0x60 >>> [7.229949] [] devm_request_threaded_irq+0x50/0xe0 >>> [7.229955] [] pcc_mbox_request_channel+0x110/0x170 >>> [7.229960] [] acpi_cppc_processor_probe+0x264/0x414 >>> [7.229963] [] __acpi_processor_start+0x28/0xa0 >>> [7.229966] [] acpi_processor_start+0x44/0x54 >>> [7.229970] [] driver_probe_device+0x1fc/0x2b0 >>> [7.229974] [] __driver_attach+0xb4/0xc0 >>> [7.229977] [] bus_for_each_dev+0x5c/0xa0 >>> [7.229980] [] driver_attach+0x20/0x30 >>> [7.229983] [] bus_add_driver+0x110/0x230 >>> [7.229987] [] driver_register+0x60/0x100 >>> [7.229991] [] acpi_processor_driver_init+0x2c/0xb0 >>> [7.229996] [] do_one_initcall+0x38/0x130 >>> [7.23] [] kernel_init_freeable+0x210/0x2b4 >>> [7.230004] [] kernel_init+0x10/0x110 >>> [7.230007] [] ret_from_fork+0x10/0x50 >>> >>> It's because the spinlock inside pcc_mbox_request_channel() is >>> kept too long. This patch releases spinlock before request_irq() >>> and free_irq() to fix this issue as spinlock is only needed to >>> protect the channel data. >>> >>> Signed-off-by: Hoan Tran >>> --- >>> v3 >>> * Free mailbox irq before reset the channel data >>> * Free channel if it fails to request the mailbox irq >>> >>> v2 >>> * Release spinlock before request_irq() and free_irq() instead of >>> using mutex >>> >>> >>> drivers/mailbox/pcc.c | 70 >>> +-- >>> 1 file changed, 35 insertions(+), 35 deletions(-) >>> >>> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c >>> index 08c87fa..4b2f061 100644 >>> --- a/drivers/mailbox/pcc.c >>> +++ b/drivers/mailbox/pcc.c >>> @@ -224,6 +224,38 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p) >>> } >>> >>> /** >>> + * pcc_mbox_free_channel - Clients call this to free their Channel. >>> + * >>> + * @chan: Pointer to the mailbox channel as returned by >>> + * pcc_mbox_request_channel() >>> + */ >>> +void pcc_mbox_free_channel(struct mbox_chan *chan) >>> +{ >>> + u32 id = chan - pcc_mbox_channels; >>> + unsigned long flags; >>> + >>> + if (!chan || !chan->cl) >>> + return; >>> + >>> + if (id >= pcc_mbox_ctrl.num_chans) { >>> + pr_debug("pcc_mbox_free_channel: Invalid mbox_chan >>> passed\n"); >>> + return; >>> + } >>> + >>> + if (pcc_doorbell_irq[id] > 0) >>> + devm_free_irq(chan->mbox->dev, pcc_doorbell_irq[id], chan); >>> + >>> + spin_lock_irqsave(&chan->lock, flags); >>> + chan->cl = NULL; >>> + chan->active_req = NULL; >>> + if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK)) >>> + chan->txdone_method = TXDONE_BY_POLL; >>> + >>> + spin_unlock_irqrestore(&chan->lock, flags); >>> +} >>> +EXPORT_SYMBOL_GPL(pcc_mbox_free_channel); >>> + >>> +/** >>> * pcc_mbox_request_channel - PCC clients call this function to >>> * request a pointer to their PCC subspace, from which they >>> * can get the details of communicating with the remote. >>> @@ -267,6 +299,8 @@ struct mbox
Re: [PATCH v3] mailbox: PCC: Fix lockdep warning when request PCC channel
HI Hoan, On 11/9/2016 11:39 PM, Hoan Tran wrote: > Hi All, > > On Thu, Oct 27, 2016 at 3:34 PM, Hoan Tran wrote: >> This patch fixes the lockdep warning below >> >> [7.229767] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags)) >> [7.229776] [ cut here ] >> [7.229787] WARNING: CPU: 1 PID: 1 at >> linux-next/kernel/locking/lockdep.c:2876 loc >> kdep_trace_alloc+0xe0/0xf0 >> [7.229790] Modules linked in: >> [7.229793] >> [7.229798] CPU: 1 PID: 1 Comm: swapper/0 Not tainted >> 4.8.0-11756-g86c5152 #46 >> ... >> [7.229900] Call trace: >> [7.229903] Exception stack(0x8007da837890 to 0x8007da8379c0) >> [7.229906] 7880: 8007da834000 >> 0001 >> [7.229909] 78a0: 8007da837a70 08a0 60c5 >> 003d >> [7.229912] 78c0: 9374bc6a7f3c7832 00381878 09db7ab8 >> 002f >> [7.229915] 78e0: 0811aabc 08be2548 8007da837990 >> 0811adf8 >> [7.229918] 7900: 8007da834000 024080c0 00c0 >> 09021000 >> [7.229921] 7920: 08c8f7c8 >> 8007da579810 >> [7.229923] 7940: 002f 8007da858000 >> 0001 >> [7.229926] 7960: 0001 0811a468 >> 0002 >> [7.229929] 7980: 656c62617369645f 00038187 00ee >> 8007da837850 >> [7.229932] 79a0: 09db50c0 09db569d 0006 >> 89db568f >> [7.229936] [] lockdep_trace_alloc+0xe0/0xf0 >> [7.229940] [] __kmalloc_track_caller+0x50/0x250 >> [7.229945] [] devres_alloc_node+0x28/0x60 >> [7.229949] [] devm_request_threaded_irq+0x50/0xe0 >> [7.229955] [] pcc_mbox_request_channel+0x110/0x170 >> [7.229960] [] acpi_cppc_processor_probe+0x264/0x414 >> [7.229963] [] __acpi_processor_start+0x28/0xa0 >> [7.229966] [] acpi_processor_start+0x44/0x54 >> [7.229970] [] driver_probe_device+0x1fc/0x2b0 >> [7.229974] [] __driver_attach+0xb4/0xc0 >> [7.229977] [] bus_for_each_dev+0x5c/0xa0 >> [7.229980] [] driver_attach+0x20/0x30 >> [7.229983] [] bus_add_driver+0x110/0x230 >> [7.229987] [] driver_register+0x60/0x100 >> [7.229991] [] acpi_processor_driver_init+0x2c/0xb0 >> [7.229996] [] do_one_initcall+0x38/0x130 >> [7.23] [] kernel_init_freeable+0x210/0x2b4 >> [7.230004] [] kernel_init+0x10/0x110 >> [7.230007] [] ret_from_fork+0x10/0x50 >> >> It's because the spinlock inside pcc_mbox_request_channel() is >> kept too long. This patch releases spinlock before request_irq() >> and free_irq() to fix this issue as spinlock is only needed to >> protect the channel data. >> >> Signed-off-by: Hoan Tran >> --- >> v3 >> * Free mailbox irq before reset the channel data >> * Free channel if it fails to request the mailbox irq >> >> v2 >> * Release spinlock before request_irq() and free_irq() instead of >> using mutex >> >> >> drivers/mailbox/pcc.c | 70 >> +-- >> 1 file changed, 35 insertions(+), 35 deletions(-) >> >> diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c >> index 08c87fa..4b2f061 100644 >> --- a/drivers/mailbox/pcc.c >> +++ b/drivers/mailbox/pcc.c >> @@ -224,6 +224,38 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p) >> } >> >> /** >> + * pcc_mbox_free_channel - Clients call this to free their Channel. >> + * >> + * @chan: Pointer to the mailbox channel as returned by >> + * pcc_mbox_request_channel() >> + */ >> +void pcc_mbox_free_channel(struct mbox_chan *chan) >> +{ >> + u32 id = chan - pcc_mbox_channels; >> + unsigned long flags; >> + >> + if (!chan || !chan->cl) >> + return; >> + >> + if (id >= pcc_mbox_ctrl.num_chans) { >> + pr_debug("pcc_mbox_free_channel: Invalid mbox_chan >> passed\n"); >> + return; >> + } >> + >> + if (pcc_doorbell_irq[id] > 0) >> + devm_free_irq(chan->mbox->dev, pcc_doorbell_irq[id], chan); >> + >> + spin_lock_irqsave(&chan->lock, flags); >> + chan->cl = NULL; >> + chan->active_req = NULL; >> + if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK)) >> + chan->txdone_method = TXDONE_BY_POLL; >> + >> + spin_unlock_irqrestore(&chan->lock, flags); >> +} >> +EXPORT_SYMBOL_GPL(pcc_mbox_free_channel); >> + >> +/** >> * pcc_mbox_request_channel - PCC clients call this function to >> * request a pointer to their PCC subspace, from which they >> * can get the details of communicating with the remote. >> @@ -267,6 +299,8 @@ struct mbox_chan *pcc_mbox_request_channel(struct >> mbox_client *cl, >> if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone) >> chan->txdone_method |= TXDONE_BY_ACK; >>
Re: [PATCH v3] mailbox: PCC: Fix lockdep warning when request PCC channel
Hi All, On Thu, Oct 27, 2016 at 3:34 PM, Hoan Tran wrote: > This patch fixes the lockdep warning below > > [7.229767] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags)) > [7.229776] [ cut here ] > [7.229787] WARNING: CPU: 1 PID: 1 at > linux-next/kernel/locking/lockdep.c:2876 loc > kdep_trace_alloc+0xe0/0xf0 > [7.229790] Modules linked in: > [7.229793] > [7.229798] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.8.0-11756-g86c5152 > #46 > ... > [7.229900] Call trace: > [7.229903] Exception stack(0x8007da837890 to 0x8007da8379c0) > [7.229906] 7880: 8007da834000 > 0001 > [7.229909] 78a0: 8007da837a70 08a0 60c5 > 003d > [7.229912] 78c0: 9374bc6a7f3c7832 00381878 09db7ab8 > 002f > [7.229915] 78e0: 0811aabc 08be2548 8007da837990 > 0811adf8 > [7.229918] 7900: 8007da834000 024080c0 00c0 > 09021000 > [7.229921] 7920: 08c8f7c8 > 8007da579810 > [7.229923] 7940: 002f 8007da858000 > 0001 > [7.229926] 7960: 0001 0811a468 > 0002 > [7.229929] 7980: 656c62617369645f 00038187 00ee > 8007da837850 > [7.229932] 79a0: 09db50c0 09db569d 0006 > 89db568f > [7.229936] [] lockdep_trace_alloc+0xe0/0xf0 > [7.229940] [] __kmalloc_track_caller+0x50/0x250 > [7.229945] [] devres_alloc_node+0x28/0x60 > [7.229949] [] devm_request_threaded_irq+0x50/0xe0 > [7.229955] [] pcc_mbox_request_channel+0x110/0x170 > [7.229960] [] acpi_cppc_processor_probe+0x264/0x414 > [7.229963] [] __acpi_processor_start+0x28/0xa0 > [7.229966] [] acpi_processor_start+0x44/0x54 > [7.229970] [] driver_probe_device+0x1fc/0x2b0 > [7.229974] [] __driver_attach+0xb4/0xc0 > [7.229977] [] bus_for_each_dev+0x5c/0xa0 > [7.229980] [] driver_attach+0x20/0x30 > [7.229983] [] bus_add_driver+0x110/0x230 > [7.229987] [] driver_register+0x60/0x100 > [7.229991] [] acpi_processor_driver_init+0x2c/0xb0 > [7.229996] [] do_one_initcall+0x38/0x130 > [7.23] [] kernel_init_freeable+0x210/0x2b4 > [7.230004] [] kernel_init+0x10/0x110 > [7.230007] [] ret_from_fork+0x10/0x50 > > It's because the spinlock inside pcc_mbox_request_channel() is > kept too long. This patch releases spinlock before request_irq() > and free_irq() to fix this issue as spinlock is only needed to > protect the channel data. > > Signed-off-by: Hoan Tran > --- > v3 > * Free mailbox irq before reset the channel data > * Free channel if it fails to request the mailbox irq > > v2 > * Release spinlock before request_irq() and free_irq() instead of > using mutex > > > drivers/mailbox/pcc.c | 70 > +-- > 1 file changed, 35 insertions(+), 35 deletions(-) > > diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c > index 08c87fa..4b2f061 100644 > --- a/drivers/mailbox/pcc.c > +++ b/drivers/mailbox/pcc.c > @@ -224,6 +224,38 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p) > } > > /** > + * pcc_mbox_free_channel - Clients call this to free their Channel. > + * > + * @chan: Pointer to the mailbox channel as returned by > + * pcc_mbox_request_channel() > + */ > +void pcc_mbox_free_channel(struct mbox_chan *chan) > +{ > + u32 id = chan - pcc_mbox_channels; > + unsigned long flags; > + > + if (!chan || !chan->cl) > + return; > + > + if (id >= pcc_mbox_ctrl.num_chans) { > + pr_debug("pcc_mbox_free_channel: Invalid mbox_chan passed\n"); > + return; > + } > + > + if (pcc_doorbell_irq[id] > 0) > + devm_free_irq(chan->mbox->dev, pcc_doorbell_irq[id], chan); > + > + spin_lock_irqsave(&chan->lock, flags); > + chan->cl = NULL; > + chan->active_req = NULL; > + if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK)) > + chan->txdone_method = TXDONE_BY_POLL; > + > + spin_unlock_irqrestore(&chan->lock, flags); > +} > +EXPORT_SYMBOL_GPL(pcc_mbox_free_channel); > + > +/** > * pcc_mbox_request_channel - PCC clients call this function to > * request a pointer to their PCC subspace, from which they > * can get the details of communicating with the remote. > @@ -267,6 +299,8 @@ struct mbox_chan *pcc_mbox_request_channel(struct > mbox_client *cl, > if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone) > chan->txdone_method |= TXDONE_BY_ACK; > > + spin_unlock_irqrestore(&chan->lock, flags); > + > if (pcc_doorbell_irq[subspace_id] > 0) { > int rc; > > @@ -275,50 +309,16 @@ struct mbox_chan *p
[PATCH v3] mailbox: PCC: Fix lockdep warning when request PCC channel
This patch fixes the lockdep warning below [7.229767] DEBUG_LOCKS_WARN_ON(irqs_disabled_flags(flags)) [7.229776] [ cut here ] [7.229787] WARNING: CPU: 1 PID: 1 at linux-next/kernel/locking/lockdep.c:2876 loc kdep_trace_alloc+0xe0/0xf0 [7.229790] Modules linked in: [7.229793] [7.229798] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.8.0-11756-g86c5152 #46 ... [7.229900] Call trace: [7.229903] Exception stack(0x8007da837890 to 0x8007da8379c0) [7.229906] 7880: 8007da834000 0001 [7.229909] 78a0: 8007da837a70 08a0 60c5 003d [7.229912] 78c0: 9374bc6a7f3c7832 00381878 09db7ab8 002f [7.229915] 78e0: 0811aabc 08be2548 8007da837990 0811adf8 [7.229918] 7900: 8007da834000 024080c0 00c0 09021000 [7.229921] 7920: 08c8f7c8 8007da579810 [7.229923] 7940: 002f 8007da858000 0001 [7.229926] 7960: 0001 0811a468 0002 [7.229929] 7980: 656c62617369645f 00038187 00ee 8007da837850 [7.229932] 79a0: 09db50c0 09db569d 0006 89db568f [7.229936] [] lockdep_trace_alloc+0xe0/0xf0 [7.229940] [] __kmalloc_track_caller+0x50/0x250 [7.229945] [] devres_alloc_node+0x28/0x60 [7.229949] [] devm_request_threaded_irq+0x50/0xe0 [7.229955] [] pcc_mbox_request_channel+0x110/0x170 [7.229960] [] acpi_cppc_processor_probe+0x264/0x414 [7.229963] [] __acpi_processor_start+0x28/0xa0 [7.229966] [] acpi_processor_start+0x44/0x54 [7.229970] [] driver_probe_device+0x1fc/0x2b0 [7.229974] [] __driver_attach+0xb4/0xc0 [7.229977] [] bus_for_each_dev+0x5c/0xa0 [7.229980] [] driver_attach+0x20/0x30 [7.229983] [] bus_add_driver+0x110/0x230 [7.229987] [] driver_register+0x60/0x100 [7.229991] [] acpi_processor_driver_init+0x2c/0xb0 [7.229996] [] do_one_initcall+0x38/0x130 [7.23] [] kernel_init_freeable+0x210/0x2b4 [7.230004] [] kernel_init+0x10/0x110 [7.230007] [] ret_from_fork+0x10/0x50 It's because the spinlock inside pcc_mbox_request_channel() is kept too long. This patch releases spinlock before request_irq() and free_irq() to fix this issue as spinlock is only needed to protect the channel data. Signed-off-by: Hoan Tran --- v3 * Free mailbox irq before reset the channel data * Free channel if it fails to request the mailbox irq v2 * Release spinlock before request_irq() and free_irq() instead of using mutex drivers/mailbox/pcc.c | 70 +-- 1 file changed, 35 insertions(+), 35 deletions(-) diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c index 08c87fa..4b2f061 100644 --- a/drivers/mailbox/pcc.c +++ b/drivers/mailbox/pcc.c @@ -224,6 +224,38 @@ static irqreturn_t pcc_mbox_irq(int irq, void *p) } /** + * pcc_mbox_free_channel - Clients call this to free their Channel. + * + * @chan: Pointer to the mailbox channel as returned by + * pcc_mbox_request_channel() + */ +void pcc_mbox_free_channel(struct mbox_chan *chan) +{ + u32 id = chan - pcc_mbox_channels; + unsigned long flags; + + if (!chan || !chan->cl) + return; + + if (id >= pcc_mbox_ctrl.num_chans) { + pr_debug("pcc_mbox_free_channel: Invalid mbox_chan passed\n"); + return; + } + + if (pcc_doorbell_irq[id] > 0) + devm_free_irq(chan->mbox->dev, pcc_doorbell_irq[id], chan); + + spin_lock_irqsave(&chan->lock, flags); + chan->cl = NULL; + chan->active_req = NULL; + if (chan->txdone_method == (TXDONE_BY_POLL | TXDONE_BY_ACK)) + chan->txdone_method = TXDONE_BY_POLL; + + spin_unlock_irqrestore(&chan->lock, flags); +} +EXPORT_SYMBOL_GPL(pcc_mbox_free_channel); + +/** * pcc_mbox_request_channel - PCC clients call this function to * request a pointer to their PCC subspace, from which they * can get the details of communicating with the remote. @@ -267,6 +299,8 @@ struct mbox_chan *pcc_mbox_request_channel(struct mbox_client *cl, if (chan->txdone_method == TXDONE_BY_POLL && cl->knows_txdone) chan->txdone_method |= TXDONE_BY_ACK; + spin_unlock_irqrestore(&chan->lock, flags); + if (pcc_doorbell_irq[subspace_id] > 0) { int rc; @@ -275,50 +309,16 @@ struct mbox_chan *pcc_mbox_request_channel(struct mbox_client *cl, if (unlikely(rc)) { dev_err(dev, "failed to register PCC interrupt %d\n", pcc_doorbell_irq[subspace_id]); + pcc_mbox_free_channel(chan);