* Re: [PATCH v5 1/6] qspinlock: powerpc support qspinlock
From: Benjamin Herrenschmidt @ 2016-06-03 4:33 UTC (permalink / raw)
To: xinhui, linux-kernel, linuxppc-dev, virtualization
Cc: peterz, mpe, waiman.long, mingo, paulus, paulmck
In-Reply-To: <57510353.1020209@linux.vnet.ibm.com>
On Fri, 2016-06-03 at 12:10 +0800, xinhui wrote:
> On 2016年06月03日 09:32, Benjamin Herrenschmidt wrote:
> > On Fri, 2016-06-03 at 11:32 +1000, Benjamin Herrenschmidt wrote:
> >> On Thu, 2016-06-02 at 17:22 +0800, Pan Xinhui wrote:
> >>>
> >>> Base code to enable qspinlock on powerpc. this patch add some
> >>> #ifdef
> >>> here and there. Although there is no paravirt related code, we
> can
> >>> successfully build a qspinlock kernel after apply this patch.
> >> This is missing the IO_SYNC stuff ... It means we'll fail to do a
> >> full
> >> sync to order vs MMIOs.
> >>
> >> You need to add that back in the unlock path.
> >
> > Well, and in the lock path as well...
> >
> Oh, yes. I missed IO_SYNC stuff.
>
> thank you, Ben :)
Ok couple of other things that would be nice from my perspective (and
Michael's) if you can produce them:
- Some benchmarks of the qspinlock alone, without the PV stuff,
so we understand how much of the overhead is inherent to the
qspinlock and how much is introduced by the PV bits.
- For the above, can you show (or describe) where the qspinlock
improves things compared to our current locks. While there's
theory and to some extent practice on x86, it would be nice to
validate the effects on POWER.
- Comparative benchmark with the PV stuff in on a bare metal system
to understand the overhead there.
- Comparative benchmark with the PV stuff under pHyp and KVM
Spinlocks are fiddly and a critical piece of infrastructure, it's
important we fully understand the performance implications before we
decide to switch to a new model.
Cheers,
Ben.
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v5 1/6] qspinlock: powerpc support qspinlock
From: xinhui @ 2016-06-03 4:10 UTC (permalink / raw)
To: benh, linux-kernel, linuxppc-dev, virtualization
Cc: peterz, mpe, waiman.long, mingo, paulus, paulmck
In-Reply-To: <1464917548.26773.12.camel@au1.ibm.com>
On 2016年06月03日 09:32, Benjamin Herrenschmidt wrote:
> On Fri, 2016-06-03 at 11:32 +1000, Benjamin Herrenschmidt wrote:
>> On Thu, 2016-06-02 at 17:22 +0800, Pan Xinhui wrote:
>>>
>>> Base code to enable qspinlock on powerpc. this patch add some
>>> #ifdef
>>> here and there. Although there is no paravirt related code, we can
>>> successfully build a qspinlock kernel after apply this patch.
>> This is missing the IO_SYNC stuff ... It means we'll fail to do a
>> full
>> sync to order vs MMIOs.
>>
>> You need to add that back in the unlock path.
>
> Well, and in the lock path as well...
>
Oh, yes. I missed IO_SYNC stuff.
thank you, Ben :)
> Cheers,
> Ben.
>
>>>
>>> Signed-off-by: Pan Xinhui <xinhui.pan@linux.vnet.ibm.com>
>>> ---
>>> arch/powerpc/include/asm/qspinlock.h | 26
>>> ++++++++++++++++++++++++++
>>> arch/powerpc/include/asm/spinlock.h | 27 +++++++++++++++----
>>> --------
>>> arch/powerpc/include/asm/spinlock_types.h | 4 ++++
>>> arch/powerpc/lib/locks.c | 4 ++++
>>> 4 files changed, 49 insertions(+), 12 deletions(-)
>>> create mode 100644 arch/powerpc/include/asm/qspinlock.h
>>>
>>> diff --git a/arch/powerpc/include/asm/qspinlock.h
>>> b/arch/powerpc/include/asm/qspinlock.h
>>> new file mode 100644
>>> index 0000000..fc83cd2
>>> --- /dev/null
>>> +++ b/arch/powerpc/include/asm/qspinlock.h
>>> @@ -0,0 +1,26 @@
>>> +#ifndef _ASM_POWERPC_QSPINLOCK_H
>>> +#define _ASM_POWERPC_QSPINLOCK_H
>>> +
>>> +#include <asm-generic/qspinlock_types.h>
>>> +
>>> +#define SPIN_THRESHOLD (1 << 15)
>>> +#define queued_spin_unlock queued_spin_unlock
>>> +
>>> +static inline void native_queued_spin_unlock(struct qspinlock
>>> *lock)
>>> +{
>>> + u8 *locked = (u8 *)lock;
>>> +#ifdef __BIG_ENDIAN
>>> + locked += 3;
>>> +#endif
>>> + /* no load/store can be across the unlock()*/
>>> + smp_store_release(locked, 0);
>>> +}
>>> +
>>> +static inline void queued_spin_unlock(struct qspinlock *lock)
>>> +{
>>> + native_queued_spin_unlock(lock);
>>> +}
>>> +
>>> +#include <asm-generic/qspinlock.h>
>>> +
>>> +#endif /* _ASM_POWERPC_QSPINLOCK_H */
>>> diff --git a/arch/powerpc/include/asm/spinlock.h
>>> b/arch/powerpc/include/asm/spinlock.h
>>> index 523673d..4359ee6 100644
>>> --- a/arch/powerpc/include/asm/spinlock.h
>>> +++ b/arch/powerpc/include/asm/spinlock.h
>>> @@ -52,6 +52,20 @@
>>> #define SYNC_IO
>>> #endif
>>>
>>> +#if defined(CONFIG_PPC_SPLPAR)
>>> +/* We only yield to the hypervisor if we are in shared processor
>>> mode */
>>> +#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca-
>>>>
>>>> lppaca_ptr))
>>> +extern void __spin_yield(arch_spinlock_t *lock);
>>> +extern void __rw_yield(arch_rwlock_t *lock);
>>> +#else /* SPLPAR */
>>> +#define __spin_yield(x) barrier()
>>> +#define __rw_yield(x) barrier()
>>> +#define SHARED_PROCESSOR 0
>>> +#endif
>>> +
>>> +#ifdef CONFIG_QUEUED_SPINLOCKS
>>> +#include <asm/qspinlock.h>
>>> +#else
>>> static __always_inline int
>>> arch_spin_value_unlocked(arch_spinlock_t
>>> lock)
>>> {
>>> return lock.slock == 0;
>>> @@ -106,18 +120,6 @@ static inline int
>>> arch_spin_trylock(arch_spinlock_t *lock)
>>> * held. Conveniently, we have a word in the paca that holds this
>>> * value.
>>> */
>>> -
>>> -#if defined(CONFIG_PPC_SPLPAR)
>>> -/* We only yield to the hypervisor if we are in shared processor
>>> mode */
>>> -#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca-
>>>>
>>>> lppaca_ptr))
>>> -extern void __spin_yield(arch_spinlock_t *lock);
>>> -extern void __rw_yield(arch_rwlock_t *lock);
>>> -#else /* SPLPAR */
>>> -#define __spin_yield(x) barrier()
>>> -#define __rw_yield(x) barrier()
>>> -#define SHARED_PROCESSOR 0
>>> -#endif
>>> -
>>> static inline void arch_spin_lock(arch_spinlock_t *lock)
>>> {
>>> CLEAR_IO_SYNC;
>>> @@ -169,6 +171,7 @@ extern void
>>> arch_spin_unlock_wait(arch_spinlock_t
>>> *lock);
>>> do { while (arch_spin_is_locked(lock)) cpu_relax(); }
>>> while
>>> (0)
>>> #endif
>>>
>>> +#endif /* !CONFIG_QUEUED_SPINLOCKS */
>>> /*
>>> * Read-write spinlocks, allowing multiple readers
>>> * but only one writer.
>>> diff --git a/arch/powerpc/include/asm/spinlock_types.h
>>> b/arch/powerpc/include/asm/spinlock_types.h
>>> index 2351adc..bd7144e 100644
>>> --- a/arch/powerpc/include/asm/spinlock_types.h
>>> +++ b/arch/powerpc/include/asm/spinlock_types.h
>>> @@ -5,11 +5,15 @@
>>> # error "please don't include this file directly"
>>> #endif
>>>
>>> +#ifdef CONFIG_QUEUED_SPINLOCKS
>>> +#include <asm-generic/qspinlock_types.h>
>>> +#else
>>> typedef struct {
>>> volatile unsigned int slock;
>>> } arch_spinlock_t;
>>>
>>> #define __ARCH_SPIN_LOCK_UNLOCKED { 0 }
>>> +#endif
>>>
>>> typedef struct {
>>> volatile signed int lock;
>>> diff --git a/arch/powerpc/lib/locks.c b/arch/powerpc/lib/locks.c
>>> index f7deebd..a9ebd71 100644
>>> --- a/arch/powerpc/lib/locks.c
>>> +++ b/arch/powerpc/lib/locks.c
>>> @@ -23,6 +23,7 @@
>>> #include <asm/hvcall.h>
>>> #include <asm/smp.h>
>>>
>>> +#ifndef CONFIG_QUEUED_SPINLOCKS
>>> void __spin_yield(arch_spinlock_t *lock)
>>> {
>>> unsigned int lock_value, holder_cpu, yield_count;
>>> @@ -42,6 +43,7 @@ void __spin_yield(arch_spinlock_t *lock)
>>> get_hard_smp_processor_id(holder_cpu),
>>> yield_count);
>>> }
>>> EXPORT_SYMBOL_GPL(__spin_yield);
>>> +#endif
>>>
>>> /*
>>> * Waiting for a read lock or a write lock on a rwlock...
>>> @@ -69,6 +71,7 @@ void __rw_yield(arch_rwlock_t *rw)
>>> }
>>> #endif
>>>
>>> +#ifndef CONFIG_QUEUED_SPINLOCKS
>>> void arch_spin_unlock_wait(arch_spinlock_t *lock)
>>> {
>>> smp_mb();
>>> @@ -84,3 +87,4 @@ void arch_spin_unlock_wait(arch_spinlock_t *lock)
>>> }
>>>
>>> EXPORT_SYMBOL(arch_spin_unlock_wait);
>>> +#endif
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v5 1/6] qspinlock: powerpc support qspinlock
From: Benjamin Herrenschmidt @ 2016-06-03 1:32 UTC (permalink / raw)
To: Pan Xinhui, linux-kernel, linuxppc-dev, virtualization
Cc: peterz, mpe, waiman.long, mingo, paulus, paulmck
In-Reply-To: <1464917520.26773.11.camel@kernel.crashing.org>
On Fri, 2016-06-03 at 11:32 +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2016-06-02 at 17:22 +0800, Pan Xinhui wrote:
> >
> > Base code to enable qspinlock on powerpc. this patch add some
> > #ifdef
> > here and there. Although there is no paravirt related code, we can
> > successfully build a qspinlock kernel after apply this patch.
> This is missing the IO_SYNC stuff ... It means we'll fail to do a
> full
> sync to order vs MMIOs.
>
> You need to add that back in the unlock path.
Well, and in the lock path as well...
Cheers,
Ben.
> >
> > Signed-off-by: Pan Xinhui <xinhui.pan@linux.vnet.ibm.com>
> > ---
> > arch/powerpc/include/asm/qspinlock.h | 26
> > ++++++++++++++++++++++++++
> > arch/powerpc/include/asm/spinlock.h | 27 +++++++++++++++----
> > --------
> > arch/powerpc/include/asm/spinlock_types.h | 4 ++++
> > arch/powerpc/lib/locks.c | 4 ++++
> > 4 files changed, 49 insertions(+), 12 deletions(-)
> > create mode 100644 arch/powerpc/include/asm/qspinlock.h
> >
> > diff --git a/arch/powerpc/include/asm/qspinlock.h
> > b/arch/powerpc/include/asm/qspinlock.h
> > new file mode 100644
> > index 0000000..fc83cd2
> > --- /dev/null
> > +++ b/arch/powerpc/include/asm/qspinlock.h
> > @@ -0,0 +1,26 @@
> > +#ifndef _ASM_POWERPC_QSPINLOCK_H
> > +#define _ASM_POWERPC_QSPINLOCK_H
> > +
> > +#include <asm-generic/qspinlock_types.h>
> > +
> > +#define SPIN_THRESHOLD (1 << 15)
> > +#define queued_spin_unlock queued_spin_unlock
> > +
> > +static inline void native_queued_spin_unlock(struct qspinlock
> > *lock)
> > +{
> > + u8 *locked = (u8 *)lock;
> > +#ifdef __BIG_ENDIAN
> > + locked += 3;
> > +#endif
> > + /* no load/store can be across the unlock()*/
> > + smp_store_release(locked, 0);
> > +}
> > +
> > +static inline void queued_spin_unlock(struct qspinlock *lock)
> > +{
> > + native_queued_spin_unlock(lock);
> > +}
> > +
> > +#include <asm-generic/qspinlock.h>
> > +
> > +#endif /* _ASM_POWERPC_QSPINLOCK_H */
> > diff --git a/arch/powerpc/include/asm/spinlock.h
> > b/arch/powerpc/include/asm/spinlock.h
> > index 523673d..4359ee6 100644
> > --- a/arch/powerpc/include/asm/spinlock.h
> > +++ b/arch/powerpc/include/asm/spinlock.h
> > @@ -52,6 +52,20 @@
> > #define SYNC_IO
> > #endif
> >
> > +#if defined(CONFIG_PPC_SPLPAR)
> > +/* We only yield to the hypervisor if we are in shared processor
> > mode */
> > +#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca-
> > >
> > > lppaca_ptr))
> > +extern void __spin_yield(arch_spinlock_t *lock);
> > +extern void __rw_yield(arch_rwlock_t *lock);
> > +#else /* SPLPAR */
> > +#define __spin_yield(x) barrier()
> > +#define __rw_yield(x) barrier()
> > +#define SHARED_PROCESSOR 0
> > +#endif
> > +
> > +#ifdef CONFIG_QUEUED_SPINLOCKS
> > +#include <asm/qspinlock.h>
> > +#else
> > static __always_inline int
> > arch_spin_value_unlocked(arch_spinlock_t
> > lock)
> > {
> > return lock.slock == 0;
> > @@ -106,18 +120,6 @@ static inline int
> > arch_spin_trylock(arch_spinlock_t *lock)
> > * held. Conveniently, we have a word in the paca that holds this
> > * value.
> > */
> > -
> > -#if defined(CONFIG_PPC_SPLPAR)
> > -/* We only yield to the hypervisor if we are in shared processor
> > mode */
> > -#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca-
> > >
> > > lppaca_ptr))
> > -extern void __spin_yield(arch_spinlock_t *lock);
> > -extern void __rw_yield(arch_rwlock_t *lock);
> > -#else /* SPLPAR */
> > -#define __spin_yield(x) barrier()
> > -#define __rw_yield(x) barrier()
> > -#define SHARED_PROCESSOR 0
> > -#endif
> > -
> > static inline void arch_spin_lock(arch_spinlock_t *lock)
> > {
> > CLEAR_IO_SYNC;
> > @@ -169,6 +171,7 @@ extern void
> > arch_spin_unlock_wait(arch_spinlock_t
> > *lock);
> > do { while (arch_spin_is_locked(lock)) cpu_relax(); }
> > while
> > (0)
> > #endif
> >
> > +#endif /* !CONFIG_QUEUED_SPINLOCKS */
> > /*
> > * Read-write spinlocks, allowing multiple readers
> > * but only one writer.
> > diff --git a/arch/powerpc/include/asm/spinlock_types.h
> > b/arch/powerpc/include/asm/spinlock_types.h
> > index 2351adc..bd7144e 100644
> > --- a/arch/powerpc/include/asm/spinlock_types.h
> > +++ b/arch/powerpc/include/asm/spinlock_types.h
> > @@ -5,11 +5,15 @@
> > # error "please don't include this file directly"
> > #endif
> >
> > +#ifdef CONFIG_QUEUED_SPINLOCKS
> > +#include <asm-generic/qspinlock_types.h>
> > +#else
> > typedef struct {
> > volatile unsigned int slock;
> > } arch_spinlock_t;
> >
> > #define __ARCH_SPIN_LOCK_UNLOCKED { 0 }
> > +#endif
> >
> > typedef struct {
> > volatile signed int lock;
> > diff --git a/arch/powerpc/lib/locks.c b/arch/powerpc/lib/locks.c
> > index f7deebd..a9ebd71 100644
> > --- a/arch/powerpc/lib/locks.c
> > +++ b/arch/powerpc/lib/locks.c
> > @@ -23,6 +23,7 @@
> > #include <asm/hvcall.h>
> > #include <asm/smp.h>
> >
> > +#ifndef CONFIG_QUEUED_SPINLOCKS
> > void __spin_yield(arch_spinlock_t *lock)
> > {
> > unsigned int lock_value, holder_cpu, yield_count;
> > @@ -42,6 +43,7 @@ void __spin_yield(arch_spinlock_t *lock)
> > get_hard_smp_processor_id(holder_cpu),
> > yield_count);
> > }
> > EXPORT_SYMBOL_GPL(__spin_yield);
> > +#endif
> >
> > /*
> > * Waiting for a read lock or a write lock on a rwlock...
> > @@ -69,6 +71,7 @@ void __rw_yield(arch_rwlock_t *rw)
> > }
> > #endif
> >
> > +#ifndef CONFIG_QUEUED_SPINLOCKS
> > void arch_spin_unlock_wait(arch_spinlock_t *lock)
> > {
> > smp_mb();
> > @@ -84,3 +87,4 @@ void arch_spin_unlock_wait(arch_spinlock_t *lock)
> > }
> >
> > EXPORT_SYMBOL(arch_spin_unlock_wait);
> > +#endif
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH v5 1/6] qspinlock: powerpc support qspinlock
From: Benjamin Herrenschmidt @ 2016-06-03 1:32 UTC (permalink / raw)
To: Pan Xinhui, linux-kernel, linuxppc-dev, virtualization
Cc: peterz, mpe, waiman.long, mingo, paulus, paulmck
In-Reply-To: <1464859370-5162-3-git-send-email-xinhui.pan@linux.vnet.ibm.com>
On Thu, 2016-06-02 at 17:22 +0800, Pan Xinhui wrote:
> Base code to enable qspinlock on powerpc. this patch add some #ifdef
> here and there. Although there is no paravirt related code, we can
> successfully build a qspinlock kernel after apply this patch.
This is missing the IO_SYNC stuff ... It means we'll fail to do a full
sync to order vs MMIOs.
You need to add that back in the unlock path.
> Signed-off-by: Pan Xinhui <xinhui.pan@linux.vnet.ibm.com>
> ---
> arch/powerpc/include/asm/qspinlock.h | 26
> ++++++++++++++++++++++++++
> arch/powerpc/include/asm/spinlock.h | 27 +++++++++++++++----
> --------
> arch/powerpc/include/asm/spinlock_types.h | 4 ++++
> arch/powerpc/lib/locks.c | 4 ++++
> 4 files changed, 49 insertions(+), 12 deletions(-)
> create mode 100644 arch/powerpc/include/asm/qspinlock.h
>
> diff --git a/arch/powerpc/include/asm/qspinlock.h
> b/arch/powerpc/include/asm/qspinlock.h
> new file mode 100644
> index 0000000..fc83cd2
> --- /dev/null
> +++ b/arch/powerpc/include/asm/qspinlock.h
> @@ -0,0 +1,26 @@
> +#ifndef _ASM_POWERPC_QSPINLOCK_H
> +#define _ASM_POWERPC_QSPINLOCK_H
> +
> +#include <asm-generic/qspinlock_types.h>
> +
> +#define SPIN_THRESHOLD (1 << 15)
> +#define queued_spin_unlock queued_spin_unlock
> +
> +static inline void native_queued_spin_unlock(struct qspinlock *lock)
> +{
> + u8 *locked = (u8 *)lock;
> +#ifdef __BIG_ENDIAN
> + locked += 3;
> +#endif
> + /* no load/store can be across the unlock()*/
> + smp_store_release(locked, 0);
> +}
> +
> +static inline void queued_spin_unlock(struct qspinlock *lock)
> +{
> + native_queued_spin_unlock(lock);
> +}
> +
> +#include <asm-generic/qspinlock.h>
> +
> +#endif /* _ASM_POWERPC_QSPINLOCK_H */
> diff --git a/arch/powerpc/include/asm/spinlock.h
> b/arch/powerpc/include/asm/spinlock.h
> index 523673d..4359ee6 100644
> --- a/arch/powerpc/include/asm/spinlock.h
> +++ b/arch/powerpc/include/asm/spinlock.h
> @@ -52,6 +52,20 @@
> #define SYNC_IO
> #endif
>
> +#if defined(CONFIG_PPC_SPLPAR)
> +/* We only yield to the hypervisor if we are in shared processor
> mode */
> +#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca-
> >lppaca_ptr))
> +extern void __spin_yield(arch_spinlock_t *lock);
> +extern void __rw_yield(arch_rwlock_t *lock);
> +#else /* SPLPAR */
> +#define __spin_yield(x) barrier()
> +#define __rw_yield(x) barrier()
> +#define SHARED_PROCESSOR 0
> +#endif
> +
> +#ifdef CONFIG_QUEUED_SPINLOCKS
> +#include <asm/qspinlock.h>
> +#else
> static __always_inline int arch_spin_value_unlocked(arch_spinlock_t
> lock)
> {
> return lock.slock == 0;
> @@ -106,18 +120,6 @@ static inline int
> arch_spin_trylock(arch_spinlock_t *lock)
> * held. Conveniently, we have a word in the paca that holds this
> * value.
> */
> -
> -#if defined(CONFIG_PPC_SPLPAR)
> -/* We only yield to the hypervisor if we are in shared processor
> mode */
> -#define SHARED_PROCESSOR (lppaca_shared_proc(local_paca-
> >lppaca_ptr))
> -extern void __spin_yield(arch_spinlock_t *lock);
> -extern void __rw_yield(arch_rwlock_t *lock);
> -#else /* SPLPAR */
> -#define __spin_yield(x) barrier()
> -#define __rw_yield(x) barrier()
> -#define SHARED_PROCESSOR 0
> -#endif
> -
> static inline void arch_spin_lock(arch_spinlock_t *lock)
> {
> CLEAR_IO_SYNC;
> @@ -169,6 +171,7 @@ extern void arch_spin_unlock_wait(arch_spinlock_t
> *lock);
> do { while (arch_spin_is_locked(lock)) cpu_relax(); } while
> (0)
> #endif
>
> +#endif /* !CONFIG_QUEUED_SPINLOCKS */
> /*
> * Read-write spinlocks, allowing multiple readers
> * but only one writer.
> diff --git a/arch/powerpc/include/asm/spinlock_types.h
> b/arch/powerpc/include/asm/spinlock_types.h
> index 2351adc..bd7144e 100644
> --- a/arch/powerpc/include/asm/spinlock_types.h
> +++ b/arch/powerpc/include/asm/spinlock_types.h
> @@ -5,11 +5,15 @@
> # error "please don't include this file directly"
> #endif
>
> +#ifdef CONFIG_QUEUED_SPINLOCKS
> +#include <asm-generic/qspinlock_types.h>
> +#else
> typedef struct {
> volatile unsigned int slock;
> } arch_spinlock_t;
>
> #define __ARCH_SPIN_LOCK_UNLOCKED { 0 }
> +#endif
>
> typedef struct {
> volatile signed int lock;
> diff --git a/arch/powerpc/lib/locks.c b/arch/powerpc/lib/locks.c
> index f7deebd..a9ebd71 100644
> --- a/arch/powerpc/lib/locks.c
> +++ b/arch/powerpc/lib/locks.c
> @@ -23,6 +23,7 @@
> #include <asm/hvcall.h>
> #include <asm/smp.h>
>
> +#ifndef CONFIG_QUEUED_SPINLOCKS
> void __spin_yield(arch_spinlock_t *lock)
> {
> unsigned int lock_value, holder_cpu, yield_count;
> @@ -42,6 +43,7 @@ void __spin_yield(arch_spinlock_t *lock)
> get_hard_smp_processor_id(holder_cpu), yield_count);
> }
> EXPORT_SYMBOL_GPL(__spin_yield);
> +#endif
>
> /*
> * Waiting for a read lock or a write lock on a rwlock...
> @@ -69,6 +71,7 @@ void __rw_yield(arch_rwlock_t *rw)
> }
> #endif
>
> +#ifndef CONFIG_QUEUED_SPINLOCKS
> void arch_spin_unlock_wait(arch_spinlock_t *lock)
> {
> smp_mb();
> @@ -84,3 +87,4 @@ void arch_spin_unlock_wait(arch_spinlock_t *lock)
> }
>
> EXPORT_SYMBOL(arch_spin_unlock_wait);
> +#endif
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [Intel-gfx] [PATCH 01/20] drm/atomic: Fix remaining places where !funcs->best_encoder is valid
From: Chris Wilson @ 2016-06-02 22:40 UTC (permalink / raw)
To: Daniel Vetter
Cc: Krzysztof Kozlowski, Heiko Stuebner, David Airlie, dri-devel,
open list:VIRTIO CORE, NET..., Matthias Brugger, Thierry Re ding,
Laurent Pinchart, Benjamin Gaignard, Daniel Vetter,
Boris Brezillon, linux-samsung-soc, Joonyoung Shim,
Alexey Brodkin, open list:ARM/Rockchip SoC..., Chen-Yu Tsai,
Kukjin Kim, Stephen Warren, linux-arm-msm
In-Reply-To: <CAKMK7uHyqNbA8Scw2fE0geAyNt0a0XVtzf81nwWjEhbZ-gAvKw@mail.gmail.com>
On Thu, Jun 02, 2016 at 11:57:02PM +0200, Daniel Vetter wrote:
> drm_encoder_find is an idr lookup. That should be plenty fast,
> especially for modeset code. Usually what's too expensive even for
> modeset code is linear list walks. But Chris just submitted patches to
> convert most of them into simple lookups.
For the idr_find, I'm tempted to replace the mutex with a rwlock. It
helps pathological cases, but in reality there are more crucial
locks around the hw that limit concurrency. ;-)
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply
* Re: [PATCH 01/20] drm/atomic: Fix remaining places where !funcs->best_encoder is valid
From: Daniel Vetter @ 2016-06-02 21:57 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Krzysztof Kozlowski, Heiko Stuebner, David Airlie, dri-devel,
open list:VIRTIO CORE, NET..., Eric Anholt, Thierry Re ding,
Benjamin Gaignard, Daniel Vetter, Boris Brezillon,
linux-samsung-soc, Joonyoung Shim, Alexey Brodkin, Rob Clark,
open list:ARM/Rockchip SoC..., Chen-Yu Tsai, Kukjin Kim,
linux-tegra@vger.kernel.org, Stephen Warren,
linux-arm-msm <linux-ar>
In-Reply-To: <8671722.LHaiaBNqfE@avalon>
On Thu, Jun 2, 2016 at 11:05 PM, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
> Hi Boris,
>
> Thank you for the patch.
>
> On Thursday 02 Jun 2016 16:31:28 Boris Brezillon wrote:
>> Adapt drm_pick_crtcs() and update_connector_routing() to fallback to
>> drm_atomic_helper_best_encoder() if funcs->best_encoder() is NULL so
>> that DRM drivers can leave this hook unassigned if they know they want
>> to use drm_atomic_helper_best_encoder().
>
> Could you please update include/drm/drm_modeset_helper_vtables.h to document
> this new behaviour ?
Thanks for reminding me. Please update hooks for both
atomic_best_encoder and best_encoder. Also mention that it's only
optional for atomic drivers. There's lots of examples in that file for
the wording usually used.
> The only drawback I see in this patch is the additional object lookup
> performed by drm_atomic_helper_best_encoder() at runtime. I wonder if we could
> somehow cache the information, as the assignment can't change when there's a
> 1:1 correspondence between encoders and connectors.
drm_encoder_find is an idr lookup. That should be plenty fast,
especially for modeset code. Usually what's too expensive even for
modeset code is linear list walks. But Chris just submitted patches to
convert most of them into simple lookups.
-Daniel
>>> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
>> ---
>> drivers/gpu/drm/drm_atomic_helper.c | 4 +++-
>> drivers/gpu/drm/drm_fb_helper.c | 13 ++++++++++++-
>> 2 files changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
>> b/drivers/gpu/drm/drm_atomic_helper.c index f6a3350..849d029 100644
>> --- a/drivers/gpu/drm/drm_atomic_helper.c
>> +++ b/drivers/gpu/drm/drm_atomic_helper.c
>> @@ -300,8 +300,10 @@ update_connector_routing(struct drm_atomic_state
>> *state, if (funcs->atomic_best_encoder)
>> new_encoder = funcs->atomic_best_encoder(connector,
>> connector_state);
>> - else
>> + else if (funcs->best_encoder)
>> new_encoder = funcs->best_encoder(connector);
>> + else
>> + new_encoder = drm_atomic_helper_best_encoder(connector);
>>
>> if (!new_encoder) {
>> DRM_DEBUG_ATOMIC("No suitable encoder found for [CONNECTOR:%d:%s]\n",
>> diff --git a/drivers/gpu/drm/drm_fb_helper.c
>> b/drivers/gpu/drm/drm_fb_helper.c index 7c2eb75..d44389a 100644
>> --- a/drivers/gpu/drm/drm_fb_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_helper.c
>> @@ -2000,7 +2000,18 @@ static int drm_pick_crtcs(struct drm_fb_helper
>> *fb_helper, my_score++;
>>
>> connector_funcs = connector->helper_private;
>> - encoder = connector_funcs->best_encoder(connector);
>> +
>> + /*
>> + * If the DRM device implements atomic hooks and ->best_encoder() is
>> + * NULL we fallback to the default drm_atomic_helper_best_encoder()
>> + * helper.
>> + */
>> + if (fb_helper->dev->mode_config.funcs->atomic_commit &&
>> + !connector_funcs->best_encoder)
>> + encoder = drm_atomic_helper_best_encoder(connector);
>> + else
>> + encoder = connector_funcs->best_encoder(connector);
>> +
>> if (!encoder)
>> goto out;
>
> --
> Regards,
>
> Laurent Pinchart
>
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply
* [PATCH v2 -next] virtio-net: Add initial MTU advice feature
From: Aaron Conole @ 2016-06-02 21:10 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel, Michael S. Tsirkin
This commit adds the feature bit and associated mtu device entry for the
virtio network device. When a virtio device comes up, it checks the
feature bit for the VIRTIO_NET_F_MTU feature. If such feature bit is
enabled, the driver will read the advised MTU and use it as the initial
value.
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
v1->v2:
* Fixed omitted hunk from virtio_net.h
* Squashed to a single commit
* Fixed commit message.
drivers/net/virtio_net.c | 7 +++++++
include/uapi/linux/virtio_net.h | 3 +++
2 files changed, 10 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index e0638e5..ef5ee01 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1896,6 +1896,12 @@ static int virtnet_probe(struct virtio_device *vdev)
if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
vi->has_cvq = true;
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
+ dev->mtu = virtio_cread16(vdev,
+ offsetof(struct virtio_net_config,
+ mtu));
+ }
+
if (vi->any_header_sg)
dev->needed_headroom = vi->hdr_len;
@@ -2067,6 +2073,7 @@ static unsigned int features[] = {
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
VIRTIO_NET_F_CTRL_MAC_ADDR,
VIRTIO_F_ANY_LAYOUT,
+ VIRTIO_NET_F_MTU,
};
static struct virtio_driver virtio_net_driver = {
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index ec32293..1ab4ea6 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -55,6 +55,7 @@
#define VIRTIO_NET_F_MQ 22 /* Device supports Receive Flow
* Steering */
#define VIRTIO_NET_F_CTRL_MAC_ADDR 23 /* Set MAC address */
+#define VIRTIO_NET_F_MTU 25 /* Initial MTU advice */
#ifndef VIRTIO_NET_NO_LEGACY
#define VIRTIO_NET_F_GSO 6 /* Host handles pkts w/ any GSO type */
@@ -73,6 +74,8 @@ struct virtio_net_config {
* Legal values are between 1 and 0x8000
*/
__u16 max_virtqueue_pairs;
+ /* Default maximum transmit unit advice */
+ __u16 mtu;
} __attribute__((packed));
/*
--
2.5.5
^ permalink raw reply related
* Re: [PATCH 01/20] drm/atomic: Fix remaining places where !funcs->best_encoder is valid
From: Laurent Pinchart @ 2016-06-02 21:05 UTC (permalink / raw)
To: Boris Brezillon
Cc: Krzysztof Kozlowski, Heiko Stuebner, David Airlie, dri-devel,
virtualization, Eric Anholt, Th ierry Re ding, Benjamin Gaignard,
Daniel Vetter, Alexandre Courbot, linux-samsung-soc,
Joonyoung Shim, Alexey Brodkin, Kyungmin Park, linux-rockchip,
Chen-Yu Tsai, Kukjin Kim, linux-tegra, Stephen Warren,
linux-arm-msm, intel-gfx, Jani Nikula, Inki Dae, linux-mediatek
In-Reply-To: <1464877907-27723-2-git-send-email-boris.brezillon@free-electrons.com>
Hi Boris,
Thank you for the patch.
On Thursday 02 Jun 2016 16:31:28 Boris Brezillon wrote:
> Adapt drm_pick_crtcs() and update_connector_routing() to fallback to
> drm_atomic_helper_best_encoder() if funcs->best_encoder() is NULL so
> that DRM drivers can leave this hook unassigned if they know they want
> to use drm_atomic_helper_best_encoder().
Could you please update include/drm/drm_modeset_helper_vtables.h to document
this new behaviour ?
The only drawback I see in this patch is the additional object lookup
performed by drm_atomic_helper_best_encoder() at runtime. I wonder if we could
somehow cache the information, as the assignment can't change when there's a
1:1 correspondence between encoders and connectors.
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 4 +++-
> drivers/gpu/drm/drm_fb_helper.c | 13 ++++++++++++-
> 2 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c index f6a3350..849d029 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -300,8 +300,10 @@ update_connector_routing(struct drm_atomic_state
> *state, if (funcs->atomic_best_encoder)
> new_encoder = funcs->atomic_best_encoder(connector,
> connector_state);
> - else
> + else if (funcs->best_encoder)
> new_encoder = funcs->best_encoder(connector);
> + else
> + new_encoder = drm_atomic_helper_best_encoder(connector);
>
> if (!new_encoder) {
> DRM_DEBUG_ATOMIC("No suitable encoder found for [CONNECTOR:%d:%s]\n",
> diff --git a/drivers/gpu/drm/drm_fb_helper.c
> b/drivers/gpu/drm/drm_fb_helper.c index 7c2eb75..d44389a 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -2000,7 +2000,18 @@ static int drm_pick_crtcs(struct drm_fb_helper
> *fb_helper, my_score++;
>
> connector_funcs = connector->helper_private;
> - encoder = connector_funcs->best_encoder(connector);
> +
> + /*
> + * If the DRM device implements atomic hooks and ->best_encoder() is
> + * NULL we fallback to the default drm_atomic_helper_best_encoder()
> + * helper.
> + */
> + if (fb_helper->dev->mode_config.funcs->atomic_commit &&
> + !connector_funcs->best_encoder)
> + encoder = drm_atomic_helper_best_encoder(connector);
> + else
> + encoder = connector_funcs->best_encoder(connector);
> +
> if (!encoder)
> goto out;
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 16/20] drm: omap: Rely on the default ->best_encoder() behavior
From: Laurent Pinchart @ 2016-06-02 21:00 UTC (permalink / raw)
To: Boris Brezillon
Cc: Krzysztof Kozlowski, Heiko Stuebner, David Airlie, dri-devel,
virtualization, Eric Anholt, Th ierry Re ding, Benjamin Gaignard,
Daniel Vetter, Alexandre Courbot, linux-samsung-soc,
Joonyoung Shim, Alexey Brodkin, Kyungmin Park, linux-rockchip,
Chen-Yu Tsai, Kukjin Kim, linux-tegra, Stephen Warren,
linux-arm-msm, intel-gfx, Jani Nikula, Inki Dae, linux-mediatek
In-Reply-To: <1464877907-27723-17-git-send-email-boris.brezillon@free-electrons.com>
Hi Boris,
Thank you for the patch.
On Thursday 02 Jun 2016 16:31:43 Boris Brezillon wrote:
> We have a 1:1 relationship between connectors and encoders and the
> driver is relying on the atomic helpers: we can drop the custom
> ->best_encoder() implementation and let the core call
> drm_atomic_helper_best_encoder() for us.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> drivers/gpu/drm/omapdrm/omap_connector.c | 8 --------
> 1 file changed, 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c
> b/drivers/gpu/drm/omapdrm/omap_connector.c index ce2d67b..80af5e1 100644
> --- a/drivers/gpu/drm/omapdrm/omap_connector.c
> +++ b/drivers/gpu/drm/omapdrm/omap_connector.c
> @@ -256,13 +256,6 @@ static int omap_connector_mode_valid(struct
> drm_connector *connector, return ret;
> }
>
> -struct drm_encoder *omap_connector_attached_encoder(
> - struct drm_connector *connector)
> -{
> - struct omap_connector *omap_connector = to_omap_connector(connector);
> - return omap_connector->encoder;
The omap_connector::encoder field is assigned but not used anymore, you can
remove it.
With that fixed,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> -}
> -
> static const struct drm_connector_funcs omap_connector_funcs = {
> .dpms = drm_atomic_helper_connector_dpms,
> .reset = drm_atomic_helper_connector_reset,
> @@ -276,7 +269,6 @@ static const struct drm_connector_funcs
> omap_connector_funcs = { static const struct drm_connector_helper_funcs
> omap_connector_helper_funcs = { .get_modes = omap_connector_get_modes,
> .mode_valid = omap_connector_mode_valid,
> - .best_encoder = omap_connector_attached_encoder,
> };
>
> /* initialize connector */
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH 09/20] drm: rcar-du: Rely on the default ->best_encoder() behavior
From: Laurent Pinchart @ 2016-06-02 20:57 UTC (permalink / raw)
To: Boris Brezillon
Cc: Krzysztof Kozlowski, Heiko Stuebner, David Airlie, dri-devel,
virtualization, Eric Anholt, Th ierry Re ding, Benjamin Gaignard,
Daniel Vetter, Alexandre Courbot, linux-samsung-soc,
Joonyoung Shim, Alexey Brodkin, Kyungmin Park, linux-rockchip,
Chen-Yu Tsai, Kukjin Kim, linux-tegra, Stephen Warren,
linux-arm-msm, intel-gfx, Jani Nikula, Inki Dae, linux-mediatek
In-Reply-To: <1464877907-27723-10-git-send-email-boris.brezillon@free-electrons.com>
Hi Boris,
Thank you for the patch.
On Thursday 02 Jun 2016 16:31:36 Boris Brezillon wrote:
> All outputs have a 1:1 relationship between connectors and encoders,
> and the driver is relying on the atomic helpers: we can drop the custom
> ->best_encoder() implementations and let the core call
> drm_atomic_helper_best_encoder() for us.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> drivers/gpu/drm/rcar-du/rcar_du_encoder.c | 12 ------------
> drivers/gpu/drm/rcar-du/rcar_du_encoder.h | 3 ---
> drivers/gpu/drm/rcar-du/rcar_du_hdmicon.c | 1 -
> drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c | 1 -
> drivers/gpu/drm/rcar-du/rcar_du_vgacon.c | 1 -
> 5 files changed, 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c index 4e939e4..55149e9 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.c
> @@ -27,18 +27,6 @@
> #include "rcar_du_vgacon.h"
>
> /*
> ---------------------------------------------------------------------------
> -- - * Common connector functions
> - */
> -
> -struct drm_encoder *
> -rcar_du_connector_best_encoder(struct drm_connector *connector)
> -{
> - struct rcar_du_connector *rcon = to_rcar_connector(connector);
> -
> - return rcar_encoder_to_drm_encoder(rcon->encoder);
> -}
> -
> -/*
> ---------------------------------------------------------------------------
> -- * Encoder
> */
>
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
> b/drivers/gpu/drm/rcar-du/rcar_du_encoder.h index 719b6f2a..a8669c3 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_encoder.h
> @@ -49,9 +49,6 @@ struct rcar_du_connector {
> #define to_rcar_connector(c) \
> container_of(c, struct rcar_du_connector, connector)
>
> -struct drm_encoder *
> -rcar_du_connector_best_encoder(struct drm_connector *connector);
> -
> int rcar_du_encoder_init(struct rcar_du_device *rcdu,
> enum rcar_du_encoder_type type,
> enum rcar_du_output output,
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_hdmicon.c
> b/drivers/gpu/drm/rcar-du/rcar_du_hdmicon.c index 6c92714..612b4d5 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_hdmicon.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_hdmicon.c
> @@ -52,7 +52,6 @@ static int rcar_du_hdmi_connector_mode_valid(struct
> drm_connector *connector, static const struct drm_connector_helper_funcs
> connector_helper_funcs = { .get_modes = rcar_du_hdmi_connector_get_modes,
> .mode_valid = rcar_du_hdmi_connector_mode_valid,
> - .best_encoder = rcar_du_connector_best_encoder,
> };
>
> static enum drm_connector_status
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
> b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c index e905f5d..6afd0af 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_lvdscon.c
> @@ -59,7 +59,6 @@ static int rcar_du_lvds_connector_get_modes(struct
> drm_connector *connector)
>
> static const struct drm_connector_helper_funcs connector_helper_funcs = {
> .get_modes = rcar_du_lvds_connector_get_modes,
> - .best_encoder = rcar_du_connector_best_encoder,
> };
>
> static enum drm_connector_status
> diff --git a/drivers/gpu/drm/rcar-du/rcar_du_vgacon.c
> b/drivers/gpu/drm/rcar-du/rcar_du_vgacon.c index 9d7e5c9..68f7ffa 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_du_vgacon.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_du_vgacon.c
> @@ -28,7 +28,6 @@ static int rcar_du_vga_connector_get_modes(struct
> drm_connector *connector)
>
> static const struct drm_connector_helper_funcs connector_helper_funcs = {
> .get_modes = rcar_du_vga_connector_get_modes,
> - .best_encoder = rcar_du_connector_best_encoder,
> };
>
> static enum drm_connector_status
You can also remove
rcon->encoder = renc;
from rcar_du_vga_connector_init(), it's not needed anymore. The same code in
rcar_du_hdmi_connector_init() has to stay for now though, as it's used to
locate the slave encoder in the HDMI support code. That should change when the
driver will be converted to use drm_bridge.
I can also fix this during the conversion to drm_bridge if you don't want to
resubmit. In any case,
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH V3 0/2] vhost_net polling optimization
From: David Miller @ 2016-06-02 19:08 UTC (permalink / raw)
To: jasowang; +Cc: netdev, virtualization, linux-kernel, kvm, mst
In-Reply-To: <1464760594-30326-1-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Wed, 1 Jun 2016 01:56:32 -0400
> This series tries to optimize vhost_net polling at two points:
>
> - Stop rx polling for reduicng the unnecessary wakeups during
> handle_rx().
> - Conditonally enable tx polling for reducing the unnecessary
> traversing and spinlock touching.
>
> Test shows about 17% improvement on rx pps.
>
> Please review
>
> Changes from V2:
> - Don't enable rx vq if we meet an err or rx vq is empty
> Changes from V1:
> - use vhost_net_disable_vq()/vhost_net_enable_vq() instead of open
> coding.
> - Add a new patch for conditionally enable tx polling.
Michael, please review this patch series.
Thanks.
^ permalink raw reply
* Re: [PATCH -next 2/2] virtio_net: Read the advised MTU
From: Rick Jones @ 2016-06-02 18:02 UTC (permalink / raw)
To: Aaron Conole; +Cc: netdev, virtualization, linux-kernel, Michael S. Tsirkin
In-Reply-To: <f7tmvn3sep3.fsf@redhat.com>
On 06/02/2016 10:06 AM, Aaron Conole wrote:
> Rick Jones <rick.jones2@hpe.com> writes:
>> One of the things I've been doing has been setting-up a cluster
>> (OpenStack) with JumboFrames, and then setting MTUs on instance vNICs
>> by hand to measure different MTU sizes. It would be a shame if such a
>> thing were not possible in the future. Keeping a warning if shrinking
>> the MTU would be good, leave the error (perhaps) to if an attempt is
>> made to go beyond the advised value.
>
> This was cut because it didn't make sense for such a warning to
> be issued, but it seems like perhaps you may want such a feature? I
> agree with Michael, after thinking about it, that I don't know what sort
> of use the warning would serve. After all, if you're changing the MTU,
> you must have wanted such a change to occur?
I don't need a warning, was simply willing to live with one when
shrinking the MTU. Didn't want an error.
happy benchmarking,
rick jones
^ permalink raw reply
* Re: [PATCH -next 2/2] virtio_net: Read the advised MTU
From: Aaron Conole @ 2016-06-02 17:48 UTC (permalink / raw)
To: kbuild test robot
Cc: netdev, Michael S. Tsirkin, kbuild-all, linux-kernel,
virtualization
In-Reply-To: <201606030134.9z1SDe8C%fengguang.wu@intel.com>
kbuild test robot <lkp@intel.com> writes:
> Hi,
>
> [auto build test ERROR on next-20160602]
>
> url: https://github.com/0day-ci/linux/commits/Aaron-Conole/virtio-net-Advised-MTU-feature/20160603-000714
> config: i386-allmodconfig (attached as .config)
> compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386
>
> Note: the linux-review/Aaron-Conole/virtio-net-Advised-MTU-feature/20160603-000714 HEAD d909da4df3c52f78b4f5fcccd89aea5e38722d10 builds fine.
> It only hurts bisectibility.
>
> All errors (new ones prefixed by >>):
>
> drivers/net/virtio_net.c: In function 'virtnet_probe':
>>> drivers/net/virtio_net.c:1899:31: error: 'VIRTIO_NET_F_MTU'
>>> undeclared (first use in this function)
> if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
> ^~~~~~~~~~~~~~~~
> drivers/net/virtio_net.c:1899:31: note: each undeclared identifier is reported only once for each function it appears in
> drivers/net/virtio_net.c: At top level:
>>> drivers/net/virtio_net.c:2076:2: error: 'VIRTIO_NET_F_MTU'
>>> undeclared here (not in a function)
> VIRTIO_NET_F_MTU,
> ^~~~~~~~~~~~~~~~
Oops, hunk was dropped during rebase. Sorry for this, v2 will fix this
error, as well and I'll do a boot test before submission.
Thanks kbuild robot!
> vim +/VIRTIO_NET_F_MTU +1899 drivers/net/virtio_net.c
>
> 1893 virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
> 1894 vi->any_header_sg = true;
> 1895
> 1896 if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
> 1897 vi->has_cvq = true;
> 1898
>> 1899 if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
> 1900 dev->mtu = virtio_cread16(vdev,
> 1901 offsetof(struct virtio_net_config,
> 1902 mtu));
> 1903 }
> 1904
> 1905 if (vi->any_header_sg)
> 1906 dev->needed_headroom = vi->hdr_len;
> 1907
> 1908 /* Use single tx/rx queue pair as default */
> 1909 vi->curr_queue_pairs = 1;
> 1910 vi->max_queue_pairs = max_queue_pairs;
> 1911
> 1912 /* Allocate/initialize the rx/tx queues, and invoke find_vqs */
> 1913 err = init_vqs(vi);
> 1914 if (err)
> 1915 goto free_stats;
> 1916
> 1917 #ifdef CONFIG_SYSFS
> 1918 if (vi->mergeable_rx_bufs)
> 1919 dev->sysfs_rx_queue_group = &virtio_net_mrg_rx_group;
> 1920 #endif
> 1921 netif_set_real_num_tx_queues(dev, vi->curr_queue_pairs);
> 1922 netif_set_real_num_rx_queues(dev, vi->curr_queue_pairs);
> 1923
> 1924 virtnet_init_settings(dev);
> 1925
> 1926 err = register_netdev(dev);
> 1927 if (err) {
> 1928 pr_debug("virtio_net: registering device failed\n");
> 1929 goto free_vqs;
> 1930 }
> 1931
> 1932 virtio_device_ready(vdev);
> 1933
> 1934 vi->nb.notifier_call = &virtnet_cpu_callback;
> 1935 err = register_hotcpu_notifier(&vi->nb);
> 1936 if (err) {
> 1937 pr_debug("virtio_net: registering cpu notifier failed\n");
> 1938 goto free_unregister_netdev;
> 1939 }
> 1940
> 1941 /* Assume link up if device can't report link status,
> 1942 otherwise get link status from config. */
> 1943 if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
> 1944 netif_carrier_off(dev);
> 1945 schedule_work(&vi->config_work);
> 1946 } else {
> 1947 vi->status = VIRTIO_NET_S_LINK_UP;
> 1948 netif_carrier_on(dev);
> 1949 }
> 1950
> 1951 pr_debug("virtnet: registered device %s with %d RX and TX vq's\n",
> 1952 dev->name, max_queue_pairs);
> 1953
> 1954 return 0;
> 1955
> 1956 free_unregister_netdev:
> 1957 vi->vdev->config->reset(vdev);
> 1958
> 1959 unregister_netdev(dev);
> 1960 free_vqs:
> 1961 cancel_delayed_work_sync(&vi->refill);
> 1962 free_receive_page_frags(vi);
> 1963 virtnet_del_vqs(vi);
> 1964 free_stats:
> 1965 free_percpu(vi->stats);
> 1966 free:
> 1967 free_netdev(dev);
> 1968 return err;
> 1969 }
> 1970
> 1971 static void remove_vq_common(struct virtnet_info *vi)
> 1972 {
> 1973 vi->vdev->config->reset(vi->vdev);
> 1974
> 1975 /* Free unused buffers in both send and recv, if any. */
> 1976 free_unused_bufs(vi);
> 1977
> 1978 free_receive_bufs(vi);
> 1979
> 1980 free_receive_page_frags(vi);
> 1981
> 1982 virtnet_del_vqs(vi);
> 1983 }
> 1984
> 1985 static void virtnet_remove(struct virtio_device *vdev)
> 1986 {
> 1987 struct virtnet_info *vi = vdev->priv;
> 1988
> 1989 unregister_hotcpu_notifier(&vi->nb);
> 1990
> 1991 /* Make sure no work handler is accessing the device. */
> 1992 flush_work(&vi->config_work);
> 1993
> 1994 unregister_netdev(vi->dev);
> 1995
> 1996 remove_vq_common(vi);
> 1997
> 1998 free_percpu(vi->stats);
> 1999 free_netdev(vi->dev);
> 2000 }
> 2001
> 2002 #ifdef CONFIG_PM_SLEEP
> 2003 static int virtnet_freeze(struct virtio_device *vdev)
> 2004 {
> 2005 struct virtnet_info *vi = vdev->priv;
> 2006 int i;
> 2007
> 2008 unregister_hotcpu_notifier(&vi->nb);
> 2009
> 2010 /* Make sure no work handler is accessing the device */
> 2011 flush_work(&vi->config_work);
> 2012
> 2013 netif_device_detach(vi->dev);
> 2014 cancel_delayed_work_sync(&vi->refill);
> 2015
> 2016 if (netif_running(vi->dev)) {
> 2017 for (i = 0; i < vi->max_queue_pairs; i++)
> 2018 napi_disable(&vi->rq[i].napi);
> 2019 }
> 2020
> 2021 remove_vq_common(vi);
> 2022
> 2023 return 0;
> 2024 }
> 2025
> 2026 static int virtnet_restore(struct virtio_device *vdev)
> 2027 {
> 2028 struct virtnet_info *vi = vdev->priv;
> 2029 int err, i;
> 2030
> 2031 err = init_vqs(vi);
> 2032 if (err)
> 2033 return err;
> 2034
> 2035 virtio_device_ready(vdev);
> 2036
> 2037 if (netif_running(vi->dev)) {
> 2038 for (i = 0; i < vi->curr_queue_pairs; i++)
> 2039 if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
> 2040 schedule_delayed_work(&vi->refill, 0);
> 2041
> 2042 for (i = 0; i < vi->max_queue_pairs; i++)
> 2043 virtnet_napi_enable(&vi->rq[i]);
> 2044 }
> 2045
> 2046 netif_device_attach(vi->dev);
> 2047
> 2048 rtnl_lock();
> 2049 virtnet_set_queues(vi, vi->curr_queue_pairs);
> 2050 rtnl_unlock();
> 2051
> 2052 err = register_hotcpu_notifier(&vi->nb);
> 2053 if (err)
> 2054 return err;
> 2055
> 2056 return 0;
> 2057 }
> 2058 #endif
> 2059
> 2060 static struct virtio_device_id id_table[] = {
> 2061 { VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
> 2062 { 0 },
> 2063 };
> 2064
> 2065 static unsigned int features[] = {
> 2066 VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM,
> 2067 VIRTIO_NET_F_GSO, VIRTIO_NET_F_MAC,
> 2068 VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO,
> VIRTIO_NET_F_HOST_TSO6,
> 2069 VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4,
> VIRTIO_NET_F_GUEST_TSO6,
> 2070 VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_UFO,
> 2071 VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
> 2072 VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
> 2073 VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
> 2074 VIRTIO_NET_F_CTRL_MAC_ADDR,
> 2075 VIRTIO_F_ANY_LAYOUT,
>> 2076 VIRTIO_NET_F_MTU,
> 2077 };
> 2078
> 2079 static struct virtio_driver virtio_net_driver = {
>
> ---
> 0-DAY kernel test infrastructure Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all Intel Corporation
^ permalink raw reply
* Re: [PATCH -next 2/2] virtio_net: Read the advised MTU
From: kbuild test robot @ 2016-06-02 17:25 UTC (permalink / raw)
To: Aaron Conole
Cc: netdev, Michael S. Tsirkin, kbuild-all, linux-kernel,
virtualization
In-Reply-To: <1464882211-18723-3-git-send-email-aconole@redhat.com>
[-- Attachment #1: Type: text/plain, Size: 6930 bytes --]
Hi,
[auto build test ERROR on next-20160602]
url: https://github.com/0day-ci/linux/commits/Aaron-Conole/virtio-net-Advised-MTU-feature/20160603-000714
config: i386-allmodconfig (attached as .config)
compiler: gcc-6 (Debian 6.1.1-1) 6.1.1 20160430
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
Note: the linux-review/Aaron-Conole/virtio-net-Advised-MTU-feature/20160603-000714 HEAD d909da4df3c52f78b4f5fcccd89aea5e38722d10 builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
drivers/net/virtio_net.c: In function 'virtnet_probe':
>> drivers/net/virtio_net.c:1899:31: error: 'VIRTIO_NET_F_MTU' undeclared (first use in this function)
if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
^~~~~~~~~~~~~~~~
drivers/net/virtio_net.c:1899:31: note: each undeclared identifier is reported only once for each function it appears in
drivers/net/virtio_net.c: At top level:
>> drivers/net/virtio_net.c:2076:2: error: 'VIRTIO_NET_F_MTU' undeclared here (not in a function)
VIRTIO_NET_F_MTU,
^~~~~~~~~~~~~~~~
vim +/VIRTIO_NET_F_MTU +1899 drivers/net/virtio_net.c
1893 virtio_has_feature(vdev, VIRTIO_F_VERSION_1))
1894 vi->any_header_sg = true;
1895
1896 if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
1897 vi->has_cvq = true;
1898
> 1899 if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
1900 dev->mtu = virtio_cread16(vdev,
1901 offsetof(struct virtio_net_config,
1902 mtu));
1903 }
1904
1905 if (vi->any_header_sg)
1906 dev->needed_headroom = vi->hdr_len;
1907
1908 /* Use single tx/rx queue pair as default */
1909 vi->curr_queue_pairs = 1;
1910 vi->max_queue_pairs = max_queue_pairs;
1911
1912 /* Allocate/initialize the rx/tx queues, and invoke find_vqs */
1913 err = init_vqs(vi);
1914 if (err)
1915 goto free_stats;
1916
1917 #ifdef CONFIG_SYSFS
1918 if (vi->mergeable_rx_bufs)
1919 dev->sysfs_rx_queue_group = &virtio_net_mrg_rx_group;
1920 #endif
1921 netif_set_real_num_tx_queues(dev, vi->curr_queue_pairs);
1922 netif_set_real_num_rx_queues(dev, vi->curr_queue_pairs);
1923
1924 virtnet_init_settings(dev);
1925
1926 err = register_netdev(dev);
1927 if (err) {
1928 pr_debug("virtio_net: registering device failed\n");
1929 goto free_vqs;
1930 }
1931
1932 virtio_device_ready(vdev);
1933
1934 vi->nb.notifier_call = &virtnet_cpu_callback;
1935 err = register_hotcpu_notifier(&vi->nb);
1936 if (err) {
1937 pr_debug("virtio_net: registering cpu notifier failed\n");
1938 goto free_unregister_netdev;
1939 }
1940
1941 /* Assume link up if device can't report link status,
1942 otherwise get link status from config. */
1943 if (virtio_has_feature(vi->vdev, VIRTIO_NET_F_STATUS)) {
1944 netif_carrier_off(dev);
1945 schedule_work(&vi->config_work);
1946 } else {
1947 vi->status = VIRTIO_NET_S_LINK_UP;
1948 netif_carrier_on(dev);
1949 }
1950
1951 pr_debug("virtnet: registered device %s with %d RX and TX vq's\n",
1952 dev->name, max_queue_pairs);
1953
1954 return 0;
1955
1956 free_unregister_netdev:
1957 vi->vdev->config->reset(vdev);
1958
1959 unregister_netdev(dev);
1960 free_vqs:
1961 cancel_delayed_work_sync(&vi->refill);
1962 free_receive_page_frags(vi);
1963 virtnet_del_vqs(vi);
1964 free_stats:
1965 free_percpu(vi->stats);
1966 free:
1967 free_netdev(dev);
1968 return err;
1969 }
1970
1971 static void remove_vq_common(struct virtnet_info *vi)
1972 {
1973 vi->vdev->config->reset(vi->vdev);
1974
1975 /* Free unused buffers in both send and recv, if any. */
1976 free_unused_bufs(vi);
1977
1978 free_receive_bufs(vi);
1979
1980 free_receive_page_frags(vi);
1981
1982 virtnet_del_vqs(vi);
1983 }
1984
1985 static void virtnet_remove(struct virtio_device *vdev)
1986 {
1987 struct virtnet_info *vi = vdev->priv;
1988
1989 unregister_hotcpu_notifier(&vi->nb);
1990
1991 /* Make sure no work handler is accessing the device. */
1992 flush_work(&vi->config_work);
1993
1994 unregister_netdev(vi->dev);
1995
1996 remove_vq_common(vi);
1997
1998 free_percpu(vi->stats);
1999 free_netdev(vi->dev);
2000 }
2001
2002 #ifdef CONFIG_PM_SLEEP
2003 static int virtnet_freeze(struct virtio_device *vdev)
2004 {
2005 struct virtnet_info *vi = vdev->priv;
2006 int i;
2007
2008 unregister_hotcpu_notifier(&vi->nb);
2009
2010 /* Make sure no work handler is accessing the device */
2011 flush_work(&vi->config_work);
2012
2013 netif_device_detach(vi->dev);
2014 cancel_delayed_work_sync(&vi->refill);
2015
2016 if (netif_running(vi->dev)) {
2017 for (i = 0; i < vi->max_queue_pairs; i++)
2018 napi_disable(&vi->rq[i].napi);
2019 }
2020
2021 remove_vq_common(vi);
2022
2023 return 0;
2024 }
2025
2026 static int virtnet_restore(struct virtio_device *vdev)
2027 {
2028 struct virtnet_info *vi = vdev->priv;
2029 int err, i;
2030
2031 err = init_vqs(vi);
2032 if (err)
2033 return err;
2034
2035 virtio_device_ready(vdev);
2036
2037 if (netif_running(vi->dev)) {
2038 for (i = 0; i < vi->curr_queue_pairs; i++)
2039 if (!try_fill_recv(vi, &vi->rq[i], GFP_KERNEL))
2040 schedule_delayed_work(&vi->refill, 0);
2041
2042 for (i = 0; i < vi->max_queue_pairs; i++)
2043 virtnet_napi_enable(&vi->rq[i]);
2044 }
2045
2046 netif_device_attach(vi->dev);
2047
2048 rtnl_lock();
2049 virtnet_set_queues(vi, vi->curr_queue_pairs);
2050 rtnl_unlock();
2051
2052 err = register_hotcpu_notifier(&vi->nb);
2053 if (err)
2054 return err;
2055
2056 return 0;
2057 }
2058 #endif
2059
2060 static struct virtio_device_id id_table[] = {
2061 { VIRTIO_ID_NET, VIRTIO_DEV_ANY_ID },
2062 { 0 },
2063 };
2064
2065 static unsigned int features[] = {
2066 VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM,
2067 VIRTIO_NET_F_GSO, VIRTIO_NET_F_MAC,
2068 VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_UFO, VIRTIO_NET_F_HOST_TSO6,
2069 VIRTIO_NET_F_HOST_ECN, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6,
2070 VIRTIO_NET_F_GUEST_ECN, VIRTIO_NET_F_GUEST_UFO,
2071 VIRTIO_NET_F_MRG_RXBUF, VIRTIO_NET_F_STATUS, VIRTIO_NET_F_CTRL_VQ,
2072 VIRTIO_NET_F_CTRL_RX, VIRTIO_NET_F_CTRL_VLAN,
2073 VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
2074 VIRTIO_NET_F_CTRL_MAC_ADDR,
2075 VIRTIO_F_ANY_LAYOUT,
> 2076 VIRTIO_NET_F_MTU,
2077 };
2078
2079 static struct virtio_driver virtio_net_driver = {
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 55156 bytes --]
[-- Attachment #3: Type: text/plain, Size: 183 bytes --]
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
^ permalink raw reply
* Re: [PATCH -next 2/2] virtio_net: Read the advised MTU
From: Aaron Conole @ 2016-06-02 17:10 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, virtualization
In-Reply-To: <20160602191632-mutt-send-email-mst@redhat.com>
"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Thu, Jun 02, 2016 at 11:43:31AM -0400, Aaron Conole wrote:
>> This patch checks the feature bit for the VIRTIO_NET_F_MTU feature. If it
>> exists, read the advised MTU and use it.
>>
>> No proper error handling is provided for the case where a user changes the
>> negotiated MTU. A future commit will add proper error handling. Instead, a
>> warning is emitted if the guest changes the device MTU after previously
>> being given advice.
>
> I don't see a warning and I don't think it's needed.
> Patch is ok commit log isn't.
Okay, I'll fix it when I submit v2.
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>> drivers/net/virtio_net.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
>> index e0638e5..ef5ee01 100644
>> --- a/drivers/net/virtio_net.c
>> +++ b/drivers/net/virtio_net.c
>> @@ -1896,6 +1896,12 @@ static int virtnet_probe(struct virtio_device *vdev)
>> if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
>> vi->has_cvq = true;
>>
>> + if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
>> + dev->mtu = virtio_cread16(vdev,
>> + offsetof(struct virtio_net_config,
>> + mtu));
>> + }
>> +
>> if (vi->any_header_sg)
>> dev->needed_headroom = vi->hdr_len;
>>
>> @@ -2067,6 +2073,7 @@ static unsigned int features[] = {
>> VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
>> VIRTIO_NET_F_CTRL_MAC_ADDR,
>> VIRTIO_F_ANY_LAYOUT,
>> + VIRTIO_NET_F_MTU,
>> };
>>
>> static struct virtio_driver virtio_net_driver = {
>> --
>> 2.5.5
^ permalink raw reply
* Re: [PATCH -next 1/2] virtio: Start feature MTU support
From: Aaron Conole @ 2016-06-02 17:10 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: netdev, linux-kernel, virtualization
In-Reply-To: <20160602191702-mutt-send-email-mst@redhat.com>
"Michael S. Tsirkin" <mst@redhat.com> writes:
> On Thu, Jun 02, 2016 at 11:43:30AM -0400, Aaron Conole wrote:
>> This commit adds the feature bit and associated mtu device entry for the
>> virtio network device. Future commits will make use of these bits to
>> support negotiated MTU.
>
> why split it out? Pls squash with the next patch.
Okay. I thought the usual method was make a commit which introduces the
data structure changes, and then make a commit which hooks it up. I'll
squash this in the future.
>> Signed-off-by: Aaron Conole <aconole@redhat.com>
>> ---
>> include/uapi/linux/virtio_net.h | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
>> index ec32293..751ff59 100644
>> --- a/include/uapi/linux/virtio_net.h
>> +++ b/include/uapi/linux/virtio_net.h
>> @@ -73,6 +73,8 @@ struct virtio_net_config {
>> * Legal values are between 1 and 0x8000
>> */
>> __u16 max_virtqueue_pairs;
>> + /* Default maximum transmit unit advice */
>> + __u16 mtu;
>> } __attribute__((packed));
>>
>> /*
>> --
>> 2.5.5
^ permalink raw reply
* Re: [PATCH -next 2/2] virtio_net: Read the advised MTU
From: Aaron Conole @ 2016-06-02 17:06 UTC (permalink / raw)
To: Rick Jones; +Cc: netdev, virtualization, linux-kernel, Michael S. Tsirkin
In-Reply-To: <5750578E.4040406@hpe.com>
Hi Rick,
In the future, please don't cut the list.
Rick Jones <rick.jones2@hpe.com> writes:
> On 06/02/2016 08:43 AM, Aaron Conole wrote:
>> This patch checks the feature bit for the VIRTIO_NET_F_MTU feature. If it
>> exists, read the advised MTU and use it.
>>
>> No proper error handling is provided for the case where a user changes the
>> negotiated MTU. A future commit will add proper error handling. Instead, a
>> warning is emitted if the guest changes the device MTU after previously
>> being given advice.
>
> One of the things I've been doing has been setting-up a cluster
> (OpenStack) with JumboFrames, and then setting MTUs on instance vNICs
> by hand to measure different MTU sizes. It would be a shame if such a
> thing were not possible in the future. Keeping a warning if shrinking
> the MTU would be good, leave the error (perhaps) to if an attempt is
> made to go beyond the advised value.
This was cut because it didn't make sense for such a warning to
be issued, but it seems like perhaps you may want such a feature? I
agree with Michael, after thinking about it, that I don't know what sort
of use the warning would serve. After all, if you're changing the MTU,
you must have wanted such a change to occur?
-Aaron
> happy benchmarking,
>
> rick jones
^ permalink raw reply
* Re: [PATCH -next 1/2] virtio: Start feature MTU support
From: Michael S. Tsirkin @ 2016-06-02 16:17 UTC (permalink / raw)
To: Aaron Conole; +Cc: netdev, linux-kernel, virtualization
In-Reply-To: <1464882211-18723-2-git-send-email-aconole@redhat.com>
On Thu, Jun 02, 2016 at 11:43:30AM -0400, Aaron Conole wrote:
> This commit adds the feature bit and associated mtu device entry for the
> virtio network device. Future commits will make use of these bits to
> support negotiated MTU.
why split it out? Pls squash with the next patch.
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
> include/uapi/linux/virtio_net.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index ec32293..751ff59 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -73,6 +73,8 @@ struct virtio_net_config {
> * Legal values are between 1 and 0x8000
> */
> __u16 max_virtqueue_pairs;
> + /* Default maximum transmit unit advice */
> + __u16 mtu;
> } __attribute__((packed));
>
> /*
> --
> 2.5.5
^ permalink raw reply
* Re: [PATCH -next 2/2] virtio_net: Read the advised MTU
From: Michael S. Tsirkin @ 2016-06-02 16:16 UTC (permalink / raw)
To: Aaron Conole; +Cc: netdev, linux-kernel, virtualization
In-Reply-To: <1464882211-18723-3-git-send-email-aconole@redhat.com>
On Thu, Jun 02, 2016 at 11:43:31AM -0400, Aaron Conole wrote:
> This patch checks the feature bit for the VIRTIO_NET_F_MTU feature. If it
> exists, read the advised MTU and use it.
>
> No proper error handling is provided for the case where a user changes the
> negotiated MTU. A future commit will add proper error handling. Instead, a
> warning is emitted if the guest changes the device MTU after previously
> being given advice.
I don't see a warning and I don't think it's needed.
Patch is ok commit log isn't.
> Signed-off-by: Aaron Conole <aconole@redhat.com>
> ---
> drivers/net/virtio_net.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index e0638e5..ef5ee01 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1896,6 +1896,12 @@ static int virtnet_probe(struct virtio_device *vdev)
> if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
> vi->has_cvq = true;
>
> + if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
> + dev->mtu = virtio_cread16(vdev,
> + offsetof(struct virtio_net_config,
> + mtu));
> + }
> +
> if (vi->any_header_sg)
> dev->needed_headroom = vi->hdr_len;
>
> @@ -2067,6 +2073,7 @@ static unsigned int features[] = {
> VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
> VIRTIO_NET_F_CTRL_MAC_ADDR,
> VIRTIO_F_ANY_LAYOUT,
> + VIRTIO_NET_F_MTU,
> };
>
> static struct virtio_driver virtio_net_driver = {
> --
> 2.5.5
^ permalink raw reply
* [PATCH -next 2/2] virtio_net: Read the advised MTU
From: Aaron Conole @ 2016-06-02 15:43 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel, Michael S. Tsirkin
In-Reply-To: <1464882211-18723-1-git-send-email-aconole@redhat.com>
This patch checks the feature bit for the VIRTIO_NET_F_MTU feature. If it
exists, read the advised MTU and use it.
No proper error handling is provided for the case where a user changes the
negotiated MTU. A future commit will add proper error handling. Instead, a
warning is emitted if the guest changes the device MTU after previously
being given advice.
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
drivers/net/virtio_net.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index e0638e5..ef5ee01 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -1896,6 +1896,12 @@ static int virtnet_probe(struct virtio_device *vdev)
if (virtio_has_feature(vdev, VIRTIO_NET_F_CTRL_VQ))
vi->has_cvq = true;
+ if (virtio_has_feature(vdev, VIRTIO_NET_F_MTU)) {
+ dev->mtu = virtio_cread16(vdev,
+ offsetof(struct virtio_net_config,
+ mtu));
+ }
+
if (vi->any_header_sg)
dev->needed_headroom = vi->hdr_len;
@@ -2067,6 +2073,7 @@ static unsigned int features[] = {
VIRTIO_NET_F_GUEST_ANNOUNCE, VIRTIO_NET_F_MQ,
VIRTIO_NET_F_CTRL_MAC_ADDR,
VIRTIO_F_ANY_LAYOUT,
+ VIRTIO_NET_F_MTU,
};
static struct virtio_driver virtio_net_driver = {
--
2.5.5
^ permalink raw reply related
* [PATCH -next 1/2] virtio: Start feature MTU support
From: Aaron Conole @ 2016-06-02 15:43 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel, Michael S. Tsirkin
In-Reply-To: <1464882211-18723-1-git-send-email-aconole@redhat.com>
This commit adds the feature bit and associated mtu device entry for the
virtio network device. Future commits will make use of these bits to
support negotiated MTU.
Signed-off-by: Aaron Conole <aconole@redhat.com>
---
include/uapi/linux/virtio_net.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
index ec32293..751ff59 100644
--- a/include/uapi/linux/virtio_net.h
+++ b/include/uapi/linux/virtio_net.h
@@ -73,6 +73,8 @@ struct virtio_net_config {
* Legal values are between 1 and 0x8000
*/
__u16 max_virtqueue_pairs;
+ /* Default maximum transmit unit advice */
+ __u16 mtu;
} __attribute__((packed));
/*
--
2.5.5
^ permalink raw reply related
* [PATCH -next 0/2] virtio-net: Advised MTU feature
From: Aaron Conole @ 2016-06-02 15:43 UTC (permalink / raw)
To: virtualization, netdev, linux-kernel, Michael S. Tsirkin
The following series adds the ability for a hypervisor to set an MTU on the
guest during feature negotiation phase. This is useful for VM orchestration
when, for instance, tunneling is involved and the MTU of the various systems
should be homogenous.
The first patch adds the feature bit as described in the proposed VFIO spec
addition found at
https://lists.oasis-open.org/archives/virtio-dev/201603/msg00001.html
The second patch adds a user of the bit, and a warning when the guest changes
the MTU from the hypervisor advised MTU. Future patches may add more thorough
error handling.
Aaron Conole (2):
virtio: Start feature MTU support
virtio_net: Read the advised MTU
drivers/net/virtio_net.c | 7 +++++++
include/uapi/linux/virtio_net.h | 2 ++
2 files changed, 9 insertions(+)
--
2.5.5
^ permalink raw reply
* [PATCH 20/20] drm/bridge: dw-hdmi: Use drm_atomic_helper_best_encoder()
From: Boris Brezillon @ 2016-06-02 14:31 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, dri-devel, Daniel Vetter
Cc: Krzysztof Kozlowski, Heiko Stuebner, Stefan Agner, virtualization,
Eric Anholt, Thierry Reding, Laurent Pinchart, Benjamin Gaignard,
Alexandre Courbot, linux-samsung-soc, Joonyoung Shim,
Boris Brezillon, Alexey Brodkin, Kyungmin Park, linux-rockchip,
Chen-Yu Tsai, Kukjin Kim, linux-tegra, Stephen Warren,
linux-arm-msm, intel-gfx, Jani Nikula, Inki Dae
In-Reply-To: <1464877907-27723-1-git-send-email-boris.brezillon@free-electrons.com>
We have a 1:1 relationship between connectors and encoders, which means
we can rely on the drm_atomic_helper_best_encoder() behavior.
We still have to explicitly assign ->best_encoder() to
drm_atomic_helper_best_encoder(), because the automated fallback to
drm_atomic_helper_best_encoder() when ->best_encoder() is NULL is only
available when the DRM device is using the atomic helpers, and this bridge
is compatible with non-atomic and atomic devices.
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
drivers/gpu/drm/bridge/dw-hdmi.c | 11 +----------
1 file changed, 1 insertion(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/dw-hdmi.c b/drivers/gpu/drm/bridge/dw-hdmi.c
index c9d9412..70b1f7d 100644
--- a/drivers/gpu/drm/bridge/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/dw-hdmi.c
@@ -1476,15 +1476,6 @@ dw_hdmi_connector_mode_valid(struct drm_connector *connector,
return mode_status;
}
-static struct drm_encoder *dw_hdmi_connector_best_encoder(struct drm_connector
- *connector)
-{
- struct dw_hdmi *hdmi = container_of(connector, struct dw_hdmi,
- connector);
-
- return hdmi->encoder;
-}
-
static void dw_hdmi_connector_destroy(struct drm_connector *connector)
{
drm_connector_unregister(connector);
@@ -1525,7 +1516,7 @@ static const struct drm_connector_funcs dw_hdmi_atomic_connector_funcs = {
static const struct drm_connector_helper_funcs dw_hdmi_connector_helper_funcs = {
.get_modes = dw_hdmi_connector_get_modes,
.mode_valid = dw_hdmi_connector_mode_valid,
- .best_encoder = dw_hdmi_connector_best_encoder,
+ .best_encoder = drm_atomic_helper_best_encoder,
};
static const struct drm_bridge_funcs dw_hdmi_bridge_funcs = {
--
2.7.4
^ permalink raw reply related
* [PATCH 19/20] drm/bridge: ps8622: Rely on the default ->best_encoder() behavior
From: Boris Brezillon @ 2016-06-02 14:31 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, dri-devel, Daniel Vetter
Cc: Krzysztof Kozlowski, Heiko Stuebner, Stefan Agner, virtualization,
Eric Anholt, Thierry Reding, Laurent Pinchart, Benjamin Gaignard,
Alexandre Courbot, linux-samsung-soc, Joonyoung Shim,
Boris Brezillon, Alexey Brodkin, Kyungmin Park, linux-rockchip,
Chen-Yu Tsai, Kukjin Kim, linux-tegra, Stephen Warren,
linux-arm-msm, intel-gfx, Jani Nikula, Inki Dae
In-Reply-To: <1464877907-27723-1-git-send-email-boris.brezillon@free-electrons.com>
We have a 1:1 relationship between connectors and encoders, and the driver
is relying on the atomic helpers: we can drop the custom ->best_encoder(),
and let the core call drm_atomic_helper_best_encoder() for us.
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
drivers/gpu/drm/bridge/parade-ps8622.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/gpu/drm/bridge/parade-ps8622.c b/drivers/gpu/drm/bridge/parade-ps8622.c
index be881e9..5cd8dd7 100644
--- a/drivers/gpu/drm/bridge/parade-ps8622.c
+++ b/drivers/gpu/drm/bridge/parade-ps8622.c
@@ -474,18 +474,8 @@ static int ps8622_get_modes(struct drm_connector *connector)
return drm_panel_get_modes(ps8622->panel);
}
-static struct drm_encoder *ps8622_best_encoder(struct drm_connector *connector)
-{
- struct ps8622_bridge *ps8622;
-
- ps8622 = connector_to_ps8622(connector);
-
- return ps8622->bridge.encoder;
-}
-
static const struct drm_connector_helper_funcs ps8622_connector_helper_funcs = {
.get_modes = ps8622_get_modes,
- .best_encoder = ps8622_best_encoder,
};
static enum drm_connector_status ps8622_detect(struct drm_connector *connector,
--
2.7.4
^ permalink raw reply related
* [PATCH 18/20] drm/bridge: ptn3460: Rely on the default ->best_encoder() behavior
From: Boris Brezillon @ 2016-06-02 14:31 UTC (permalink / raw)
To: David Airlie, Daniel Vetter, dri-devel, Daniel Vetter
Cc: Krzysztof Kozlowski, Heiko Stuebner, Stefan Agner, virtualization,
Eric Anholt, Thierry Reding, Laurent Pinchart, Benjamin Gaignard,
Alexandre Courbot, linux-samsung-soc, Joonyoung Shim,
Boris Brezillon, Alexey Brodkin, Kyungmin Park, linux-rockchip,
Chen-Yu Tsai, Kukjin Kim, linux-tegra, Stephen Warren,
linux-arm-msm, intel-gfx, Jani Nikula, Inki Dae
In-Reply-To: <1464877907-27723-1-git-send-email-boris.brezillon@free-electrons.com>
We have a 1:1 relationship between connectors and encoders, and the driver
is relying on the atomic helpers: we can drop the custom ->best_encoder(),
and let the core call drm_atomic_helper_best_encoder() for us.
Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
drivers/gpu/drm/bridge/nxp-ptn3460.c | 8 --------
1 file changed, 8 deletions(-)
diff --git a/drivers/gpu/drm/bridge/nxp-ptn3460.c b/drivers/gpu/drm/bridge/nxp-ptn3460.c
index 7ecd59f..93f3dac 100644
--- a/drivers/gpu/drm/bridge/nxp-ptn3460.c
+++ b/drivers/gpu/drm/bridge/nxp-ptn3460.c
@@ -235,16 +235,8 @@ out:
return num_modes;
}
-static struct drm_encoder *ptn3460_best_encoder(struct drm_connector *connector)
-{
- struct ptn3460_bridge *ptn_bridge = connector_to_ptn3460(connector);
-
- return ptn_bridge->bridge.encoder;
-}
-
static const struct drm_connector_helper_funcs ptn3460_connector_helper_funcs = {
.get_modes = ptn3460_get_modes,
- .best_encoder = ptn3460_best_encoder,
};
static enum drm_connector_status ptn3460_detect(struct drm_connector *connector,
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox