Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1266853 > unrolled thread

Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

Started byFrancois Romieu <romieu@fr.zoreil.com>
First post2015-11-11 00:40 +0100
Last post2015-11-11 17:30 +0100
Articles 5 on this page of 25 — 4 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller Francois Romieu <romieu@fr.zoreil.com> - 2015-11-11 00:40 +0100
    Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller Måns Rullgård <mans@mansr.com> - 2015-11-11 01:50 +0100
      Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller David Miller <davem@davemloft.net> - 2015-11-11 03:20 +0100
        Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller Måns Rullgård <mans@mansr.com> - 2015-11-11 13:30 +0100
          Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller Måns Rullgård <mans@mansr.com> - 2015-11-11 14:10 +0100
            Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-11 14:40 +0100
              Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller Måns Rullgård <mans@mansr.com> - 2015-11-11 14:50 +0100
                Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-11 15:10 +0100
                  Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller Måns Rullgård <mans@mansr.com> - 2015-11-11 15:20 +0100
                    Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller Måns Rullgård <mans@mansr.com> - 2015-11-11 15:40 +0100
                      Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-11 15:50 +0100
                    Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller Eric Dumazet <eric.dumazet@gmail.com> - 2015-11-11 15:50 +0100
            Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller David Miller <davem@davemloft.net> - 2015-11-11 17:30 +0100
              Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller Måns Rullgård <mans@mansr.com> - 2015-11-11 19:30 +0100
                Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller Måns Rullgård <mans@mansr.com> - 2015-11-11 20:10 +0100
                  Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller David Miller <davem@davemloft.net> - 2015-11-11 20:20 +0100
                    Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller Måns Rullgård <mans@mansr.com> - 2015-11-11 20:20 +0100
                      Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller David Miller <davem@davemloft.net> - 2015-11-11 20:30 +0100
                        Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller David Miller <davem@davemloft.net> - 2015-11-11 20:30 +0100
                          Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller Måns Rullgård <mans@mansr.com> - 2015-11-11 20:40 +0100
                            Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller David Miller <davem@davemloft.net> - 2015-11-11 20:50 +0100
                              Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller Måns Rullgård <mans@mansr.com> - 2015-11-11 21:50 +0100
                        Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller Måns Rullgård <mans@mansr.com> - 2015-11-11 20:30 +0100
                Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller David Miller <davem@davemloft.net> - 2015-11-11 20:10 +0100
          Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800  Ethernet controller David Miller <davem@davemloft.net> - 2015-11-11 17:30 +0100

Page 2 of 2 — ← Prev page 1 [2]


#1267414

FromDavid Miller <davem@davemloft.net>
Date2015-11-11 20:50 +0100
Message-ID<qtJz3-75S-5@gated-at.bofh.it>
In reply to#1267408
From: Måns Rullgård <mans@mansr.com>
Date: Wed, 11 Nov 2015 19:35:05 +0000

>> I don't think it's silly at all.
> 
> I'm sure I read somewhere that the time spent spinning on a lock should
> be kept as small as possible.
> 
>> And unless you can measure it making a difference, don't knock the idea.
> 
> I tried using netif_tx_lock() in the IRQ handler instead, and it locked
> up solid.  Clearly that was the wrong thing to do.

Oh that's right, it's a BH lock not an IRQ one.

Yet another argument for doing everything in ->poll(), thus making all
operations outside of NAPI scheduling run in software interrupt
context, and therefore being able to make use of the TXQ lock for
this.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1267443 — Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

FromMåns Rullgård <mans@mansr.com>
Date2015-11-11 21:50 +0100
SubjectRe: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller
Message-ID<qtKv8-7HI-3@gated-at.bofh.it>
In reply to#1267414
David Miller <davem@davemloft.net> writes:

> From: Måns Rullgård <mans@mansr.com>
> Date: Wed, 11 Nov 2015 19:35:05 +0000
>
>>> I don't think it's silly at all.
>> 
>> I'm sure I read somewhere that the time spent spinning on a lock should
>> be kept as small as possible.
>> 
>>> And unless you can measure it making a difference, don't knock the idea.
>> 
>> I tried using netif_tx_lock() in the IRQ handler instead, and it locked
>> up solid.  Clearly that was the wrong thing to do.
>
> Oh that's right, it's a BH lock not an IRQ one.
>
> Yet another argument for doing everything in ->poll(), thus making all
> operations outside of NAPI scheduling run in software interrupt
> context, and therefore being able to make use of the TXQ lock for
> this.

Well, I tried calling the DMA restart function from NAPI poll under
netif_tx_lock().  Now it works only as long as there is incoming
traffic.

-- 
Måns Rullgård
mans@mansr.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1267407 — Re: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller

FromMåns Rullgård <mans@mansr.com>
Date2015-11-11 20:30 +0100
SubjectRe: [PATCH v5] net: ethernet: add driver for Aurora VLSI NB8800 Ethernet controller
Message-ID<qtJfI-6Yd-11@gated-at.bofh.it>
In reply to#1267405
David Miller <davem@davemloft.net> writes:

> From: Måns Rullgård <mans@mansr.com>
> Date: Wed, 11 Nov 2015 19:17:07 +0000
>
>> David Miller <davem@davemloft.net> writes:
>> 
>>> From: Måns Rullgård <mans@mansr.com>
>>> Date: Wed, 11 Nov 2015 19:09:19 +0000
>>>
>>>> David Miller <davem@davemloft.net> writes:
>>>> 
>>>>> From: Måns Rullgård <mans@mansr.com>
>>>>> Date: Wed, 11 Nov 2015 18:25:05 +0000
>>>>>
>>>>>> If the TX DMA channel is idle when start_xmit is called, it can be
>>>>>> started immediately.  Checking the DMA status and starting it if
>>>>>> idle has to be done atomically somehow.
>>>>>
>>>>> ->ndo_start_xmit() is guaranteed to be invoked atomically, protected
>>>>> by the TX queue spinlock.
>>>> 
>>>> Yes, but the DMA needs to be restarted from some other context if it was
>>>> busy when start_xmit checked.
>>>
>>> Then you can probably use the TXQ lock in the interrupt handler just for
>>> that.
>> 
>> That seems a bit heavy-handed when the critical section for this is only
>> a tiny part of the start_xmit function.
>
> Then what synchornization primitive other than spin locks are you going
> to use for this?
>
> My point is that there is a spinlock the core code is _already_ taking,
> unconditionally, when ->ndo_start_xmit() executes.  And you can therefore
> take advantage of that rather than using another lock of your own.

I get that.  But that remains locked for the duration of ndo_start_xmit()
whereas the part that needs to be synchronised with the DMA completion
IRQ handler is tiny.  Having the IRQ handler spin for the duration of
ndo_start_xmit() seemed silly to me.

-- 
Måns Rullgård
mans@mansr.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1267392

FromDavid Miller <davem@davemloft.net>
Date2015-11-11 20:10 +0100
Message-ID<qtIWm-6Rf-21@gated-at.bofh.it>
In reply to#1267370
From: Måns Rullgård <mans@mansr.com>
Date: Wed, 11 Nov 2015 18:25:05 +0000

> If the TX DMA channel is idle when start_xmit is called, it can be
> started immediately.  Checking the DMA status and starting it if
> idle has to be done atomically somehow.

->ndo_start_xmit() is guaranteed to be invoked atomically, protected
by the TX queue spinlock.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]


#1267298

FromDavid Miller <davem@davemloft.net>
Date2015-11-11 17:30 +0100
Message-ID<qtGry-592-59@gated-at.bofh.it>
In reply to#1267155
From: Måns Rullgård <mans@mansr.com>
Date: Wed, 11 Nov 2015 12:22:28 +0000

> David Miller <davem@davemloft.net> writes:
> 
>> From: Måns Rullgård <mans@mansr.com>
>> Date: Wed, 11 Nov 2015 00:40:09 +0000
>>
>>> When the DMA complete interrupt arrives, the next chain should be
>>> kicked off as quickly as possible, and I don't see why that would
>>> benefit from being done in napi context.
>>
>> NAPI isn't about low latency, it's about fairness and interrupt
>> mitigation.
>>
>> You probably don't even realize that all of the TX SKB freeing you do
>> in the hardware interrupt handler end up being actually processed by a
>> scheduled software interrupt anyways.
>>
>> So you are gaining almost nothing by not doing TX completion in NAPI
>> context, whereas by doing so you would be gaining a lot including
>> more simplified locking or even the ability to do no locking at all.
> 
> TX completion is separate from restarting the DMA, and moving that to
> NAPI may well be a good idea.  Should I simply napi_schedule() if the
> hardware indicates TX is complete and do the cleanup in the NAPI poll
> function?

... just like every other high end driver... Yes.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]


Page 2 of 2 — ← Prev page 1 [2]

Back to top | Article view | linux.kernel


csiph-web