* [PATCHv4 3/4] sparc64: Avoid irqsave/restore on vio.lock if in_softirq()
@ 2014-10-21 14:16 Sowmini Varadhan
2014-10-21 22:35 ` Julian Calaby
0 siblings, 1 reply; 6+ messages in thread
From: Sowmini Varadhan @ 2014-10-21 14:16 UTC (permalink / raw)
To: davem, sowmini.varadhan; +Cc: netdev, sparclinux
For NAPIfied drivers , there is no need to
synchronize by doing irqsave/restore on vio.lock in the I/O
path.
Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
---
arch/sparc/kernel/viohs.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/arch/sparc/kernel/viohs.c b/arch/sparc/kernel/viohs.c
index 7ef081a..d731586 100644
--- a/arch/sparc/kernel/viohs.c
+++ b/arch/sparc/kernel/viohs.c
@@ -747,10 +747,11 @@ EXPORT_SYMBOL(vio_ldc_free);
void vio_port_up(struct vio_driver_state *vio)
{
- unsigned long flags;
+ unsigned long flags = 0;
int err, state;
- spin_lock_irqsave(&vio->lock, flags);
+ if (!in_softirq())
+ spin_lock_irqsave(&vio->lock, flags);
state = ldc_state(vio->lp);
@@ -777,7 +778,8 @@ void vio_port_up(struct vio_driver_state *vio)
mod_timer(&vio->timer, expires);
}
- spin_unlock_irqrestore(&vio->lock, flags);
+ if (!in_softirq())
+ spin_unlock_irqrestore(&vio->lock, flags);
}
EXPORT_SYMBOL(vio_port_up);
--
1.8.4.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCHv4 3/4] sparc64: Avoid irqsave/restore on vio.lock if in_softirq()
2014-10-21 14:16 [PATCHv4 3/4] sparc64: Avoid irqsave/restore on vio.lock if in_softirq() Sowmini Varadhan
@ 2014-10-21 22:35 ` Julian Calaby
2014-10-21 22:39 ` Sowmini Varadhan
2014-10-21 23:56 ` Dave Kleikamp
0 siblings, 2 replies; 6+ messages in thread
From: Julian Calaby @ 2014-10-21 22:35 UTC (permalink / raw)
To: Sowmini Varadhan; +Cc: David S. Miller, netdev, sparclinux
Hi Sowmini,
On Wed, Oct 22, 2014 at 1:16 AM, Sowmini Varadhan
<sowmini.varadhan@oracle.com> wrote:
> For NAPIfied drivers , there is no need to
> synchronize by doing irqsave/restore on vio.lock in the I/O
> path.
>
> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
> ---
> arch/sparc/kernel/viohs.c | 8 +++++---
> 1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/arch/sparc/kernel/viohs.c b/arch/sparc/kernel/viohs.c
> index 7ef081a..d731586 100644
> --- a/arch/sparc/kernel/viohs.c
> +++ b/arch/sparc/kernel/viohs.c
> @@ -747,10 +747,11 @@ EXPORT_SYMBOL(vio_ldc_free);
>
> void vio_port_up(struct vio_driver_state *vio)
> {
> - unsigned long flags;
> + unsigned long flags = 0;
Is gcc not smart enough to know that this variable isn't used before
it's set? (I assume it isn't used elsewhere in this function)
> int err, state;
>
> - spin_lock_irqsave(&vio->lock, flags);
> + if (!in_softirq())
> + spin_lock_irqsave(&vio->lock, flags);
>
> state = ldc_state(vio->lp);
>
> @@ -777,7 +778,8 @@ void vio_port_up(struct vio_driver_state *vio)
> mod_timer(&vio->timer, expires);
> }
>
> - spin_unlock_irqrestore(&vio->lock, flags);
> + if (!in_softirq())
> + spin_unlock_irqrestore(&vio->lock, flags);
> }
> EXPORT_SYMBOL(vio_port_up);
Thanks,
--
Julian Calaby
Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv4 3/4] sparc64: Avoid irqsave/restore on vio.lock if in_softirq()
2014-10-21 22:35 ` Julian Calaby
@ 2014-10-21 22:39 ` Sowmini Varadhan
2014-10-21 22:42 ` Julian Calaby
2014-10-21 23:56 ` Dave Kleikamp
1 sibling, 1 reply; 6+ messages in thread
From: Sowmini Varadhan @ 2014-10-21 22:39 UTC (permalink / raw)
To: Julian Calaby; +Cc: David S. Miller, netdev, sparclinux
On (10/22/14 09:35), Julian Calaby wrote:
> > void vio_port_up(struct vio_driver_state *vio)
> > {
> > - unsigned long flags;
> > + unsigned long flags = 0;
>
> Is gcc not smart enough to know that this variable isn't used before
> it's set? (I assume it isn't used elsewhere in this function)
No, it's not used elsewhere in the function, and yes, I too was
surprised by the build warning, which is why I initialized it
as above.
--Sowmini
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv4 3/4] sparc64: Avoid irqsave/restore on vio.lock if in_softirq()
2014-10-21 22:39 ` Sowmini Varadhan
@ 2014-10-21 22:42 ` Julian Calaby
0 siblings, 0 replies; 6+ messages in thread
From: Julian Calaby @ 2014-10-21 22:42 UTC (permalink / raw)
To: Sowmini Varadhan; +Cc: David S. Miller, netdev, sparclinux
Hi Sowmini,
On Wed, Oct 22, 2014 at 9:39 AM, Sowmini Varadhan
<sowmini.varadhan@oracle.com> wrote:
> On (10/22/14 09:35), Julian Calaby wrote:
>> > void vio_port_up(struct vio_driver_state *vio)
>> > {
>> > - unsigned long flags;
>> > + unsigned long flags = 0;
>>
>> Is gcc not smart enough to know that this variable isn't used before
>> it's set? (I assume it isn't used elsewhere in this function)
>
> No, it's not used elsewhere in the function, and yes, I too was
> surprised by the build warning, which is why I initialized it
> as above.
Ok, fair enough then.
Thanks,
--
Julian Calaby
Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv4 3/4] sparc64: Avoid irqsave/restore on vio.lock if in_softirq()
2014-10-21 22:35 ` Julian Calaby
2014-10-21 22:39 ` Sowmini Varadhan
@ 2014-10-21 23:56 ` Dave Kleikamp
2014-10-22 0:16 ` Sowmini Varadhan
1 sibling, 1 reply; 6+ messages in thread
From: Dave Kleikamp @ 2014-10-21 23:56 UTC (permalink / raw)
To: Julian Calaby, Sowmini Varadhan; +Cc: David S. Miller, netdev, sparclinux
On 10/21/2014 05:35 PM, Julian Calaby wrote:
> Hi Sowmini,
>
> On Wed, Oct 22, 2014 at 1:16 AM, Sowmini Varadhan
> <sowmini.varadhan@oracle.com> wrote:
>> For NAPIfied drivers , there is no need to
>> synchronize by doing irqsave/restore on vio.lock in the I/O
>> path.
>>
>> Signed-off-by: Sowmini Varadhan <sowmini.varadhan@oracle.com>
>> ---
>> arch/sparc/kernel/viohs.c | 8 +++++---
>> 1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/sparc/kernel/viohs.c b/arch/sparc/kernel/viohs.c
>> index 7ef081a..d731586 100644
>> --- a/arch/sparc/kernel/viohs.c
>> +++ b/arch/sparc/kernel/viohs.c
>> @@ -747,10 +747,11 @@ EXPORT_SYMBOL(vio_ldc_free);
>>
>> void vio_port_up(struct vio_driver_state *vio)
>> {
>> - unsigned long flags;
>> + unsigned long flags = 0;
>
> Is gcc not smart enough to know that this variable isn't used before
> it's set? (I assume it isn't used elsewhere in this function)
It probably assumes in_softirq() might evaluate differently in the each
case.
>
>> int err, state;
>>
>> - spin_lock_irqsave(&vio->lock, flags);
>> + if (!in_softirq())
>> + spin_lock_irqsave(&vio->lock, flags);
>>
>> state = ldc_state(vio->lp);
>>
>> @@ -777,7 +778,8 @@ void vio_port_up(struct vio_driver_state *vio)
>> mod_timer(&vio->timer, expires);
>> }
>>
>> - spin_unlock_irqrestore(&vio->lock, flags);
>> + if (!in_softirq())
>> + spin_unlock_irqrestore(&vio->lock, flags);
>> }
>> EXPORT_SYMBOL(vio_port_up);
>
> Thanks,
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCHv4 3/4] sparc64: Avoid irqsave/restore on vio.lock if in_softirq()
2014-10-21 23:56 ` Dave Kleikamp
@ 2014-10-22 0:16 ` Sowmini Varadhan
0 siblings, 0 replies; 6+ messages in thread
From: Sowmini Varadhan @ 2014-10-22 0:16 UTC (permalink / raw)
To: Dave Kleikamp; +Cc: Julian Calaby, David S. Miller, netdev, sparclinux
On (10/21/14 18:56), Dave Kleikamp wrote:
> >
> > Is gcc not smart enough to know that this variable isn't used before
> > it's set? (I assume it isn't used elsewhere in this function)
>
> It probably assumes in_softirq() might evaluate differently in the each
> case.
yes, that's what I suspected too. I suppose it is possible
from the compiler's point of view that something in between
might change the result of in_softirq() so that we may be
using an uninit variable in the second call.
anyway, the warning was annoying, and would only numb the
user into ignoring other real issues, so I figured I might
as well silence the warning.
--Sowmini
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-10-22 0:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-21 14:16 [PATCHv4 3/4] sparc64: Avoid irqsave/restore on vio.lock if in_softirq() Sowmini Varadhan
2014-10-21 22:35 ` Julian Calaby
2014-10-21 22:39 ` Sowmini Varadhan
2014-10-21 22:42 ` Julian Calaby
2014-10-21 23:56 ` Dave Kleikamp
2014-10-22 0:16 ` Sowmini Varadhan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).