> -----Original Message----- > From: Wang, Xiao W > Sent: Monday, January 8, 2018 11:12 PM > To: Yuanhan Liu <[email protected]> > Cc: Bie, Tiwei <[email protected]>; [email protected]; > [email protected] > Subject: RE: [PATCH v6 2/3] net/virtio: add packet injection method > > > > > -----Original Message----- > > From: Yuanhan Liu [mailto:[email protected]] > > Sent: Monday, January 8, 2018 9:04 PM > > To: Wang, Xiao W <[email protected]> > > Cc: Bie, Tiwei <[email protected]>; [email protected]; > > [email protected] > > 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
