[dpdk-dev] [PATCH v6 2/3] net/virtio: add packet injection method

Wang, Xiao W xiao.w.wang at intel.com
Tue Jan 9 03:55:44 CET 2018



> -----Original Message-----
> From: Wang, Xiao W
> Sent: Monday, January 8, 2018 11:12 PM
> To: Yuanhan Liu <yliu at fridaylinux.org>
> Cc: Bie, Tiwei <tiwei.bie at intel.com>; dev at dpdk.org;
> stephen at networkplumber.org
> Subject: RE: [PATCH v6 2/3] net/virtio: add packet injection method
> 
> 
> 
> > -----Original Message-----
> > From: Yuanhan Liu [mailto:yliu at fridaylinux.org]
> > Sent: Monday, January 8, 2018 9:04 PM
> > To: Wang, Xiao W <xiao.w.wang at intel.com>
> > Cc: Bie, Tiwei <tiwei.bie at intel.com>; dev at dpdk.org;
> > stephen at networkplumber.org
> > Subject: Re: [PATCH v6 2/3] net/virtio: add packet injection method
> >
> > On Sun, Jan 07, 2018 at 04:05:12AM -0800, Xiao Wang wrote:
> > > +	/*
> > > +	 * App management thread and virtio interrupt handler thread
> > > +	 * both can change the 'started' flag, this lock is meant to
> > > +	 * avoid such a contention.
> > > +	 */
> > > +	rte_spinlock_t state_lock;
> >
> > Why not turning the "started" to atomic type, so that you don't need
> > the lock?
> >

> During the interrupt handler routine, there are a series of instructions
> between lock acquire and release. An atomic value is not suitable for this
> scenario.
> 

The current comment may doesn't explain the state_lock correctly, this lock needs to be acquired in dev_pause and released in dev_resume, so it's not just used to protect the "started" value.

I would improve the comment as " App management thread and virtio interrupt handler thread both can change device state, ..."

Thanks for comments,
Xiao


More information about the dev mailing list