On 2020/8/4 ??5:21, Michael S. Tsirkin wrote:>>>>> ? +struct vhost_vring_call { >>>>> +??? struct eventfd_ctx *ctx; >>>>> +??? struct irq_bypass_producer producer; >>>>> +??? spinlock_t ctx_lock; >>>> It's not clear to me why we need ctx_lock here. >>>> >>>> Thanks >>> Hi Jason, >>> >>> we use this lock to protect the eventfd_ctx and irq from race conditions, >> We don't support irq notification from vDPA device driver in this version, >> do we still have race condition? >> >> Thanks > Jason I'm not sure what you are trying to say here.I meant we change the API from V4 so driver won't notify us if irq is changed. Then it looks to me there's no need for the ctx_lock, everyhing could be synchronized with vq mutex. Thanks> >
On Wed, Aug 05, 2020 at 10:16:16AM +0800, Jason Wang wrote:> > On 2020/8/4 ??5:21, Michael S. Tsirkin wrote: > > > > > > ? +struct vhost_vring_call { > > > > > > +??? struct eventfd_ctx *ctx; > > > > > > +??? struct irq_bypass_producer producer; > > > > > > +??? spinlock_t ctx_lock; > > > > > It's not clear to me why we need ctx_lock here. > > > > > > > > > > Thanks > > > > Hi Jason, > > > > > > > > we use this lock to protect the eventfd_ctx and irq from race conditions, > > > We don't support irq notification from vDPA device driver in this version, > > > do we still have race condition? > > > > > > Thanks > > Jason I'm not sure what you are trying to say here. > > > I meant we change the API from V4 so driver won't notify us if irq is > changed. > > Then it looks to me there's no need for the ctx_lock, everyhing could be > synchronized with vq mutex. > > ThanksJason do you want to post a cleanup patch simplifying code along these lines? Thanks,> > > >
On 2020/8/10 ??9:37, Michael S. Tsirkin wrote:> On Wed, Aug 05, 2020 at 10:16:16AM +0800, Jason Wang wrote: >> On 2020/8/4 ??5:21, Michael S. Tsirkin wrote: >>>>>>> ? +struct vhost_vring_call { >>>>>>> +??? struct eventfd_ctx *ctx; >>>>>>> +??? struct irq_bypass_producer producer; >>>>>>> +??? spinlock_t ctx_lock; >>>>>> It's not clear to me why we need ctx_lock here. >>>>>> >>>>>> Thanks >>>>> Hi Jason, >>>>> >>>>> we use this lock to protect the eventfd_ctx and irq from race conditions, >>>> We don't support irq notification from vDPA device driver in this version, >>>> do we still have race condition? >>>> >>>> Thanks >>> Jason I'm not sure what you are trying to say here. >> >> I meant we change the API from V4 so driver won't notify us if irq is >> changed. >> >> Then it looks to me there's no need for the ctx_lock, everyhing could be >> synchronized with vq mutex. >> >> Thanks > Jason do you want to post a cleanup patch simplifying code along these > lines?Ling Shan promised to post a patch to fix this. Thanks> > Thanks, > > >>>