Re: [PATCH v7 1/7] vduse: make domain_lock an rwlock

["Michael S. Tsirkin" <mst@redhat.com>]

On Thu, Oct 23, 2025 at 10:59:01AM +0200, Eugenio Perez Martin wrote:
> On Fri, Oct 10, 2025 at 2:34 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> >
> 
> ...
> 
> >
> >
> > > @@ -2045,11 +2046,11 @@ static int vdpa_dev_add(struct vdpa_mgmt_dev *mdev, const char *name,
> > >       if (ret)
> > >               return ret;
> > >
> > > -     mutex_lock(&dev->domain_lock);
> > > +     write_lock(&dev->domain_lock);
> > >       if (!dev->domain)
> > >               dev->domain = vduse_domain_create(VDUSE_IOVA_SIZE - 1,
> > >                                                 dev->bounce_size);
> > > -     mutex_unlock(&dev->domain_lock);
> > > +     write_unlock(&dev->domain_lock);
> > >       if (!dev->domain) {
> > >               put_device(&dev->vdev->vdpa.dev);
> > >               return -ENOMEM;
> >
> >
> > Let's look at this example:
> >
> > So now you are invoking this under an rw lock:
> >
> >
> >
> >
> > struct vduse_iova_domain *
> > vduse_domain_create(unsigned long iova_limit, size_t bounce_size)
> > {
> >         struct vduse_iova_domain *domain;
> >         struct file *file;
> >         struct vduse_bounce_map *map;
> >         unsigned long pfn, bounce_pfns;
> >         int ret;
> >
> >         bounce_pfns = PAGE_ALIGN(bounce_size) >> BOUNCE_MAP_SHIFT;
> >         if (iova_limit <= bounce_size)
> >                 return NULL;
> >
> >         domain = kzalloc(sizeof(*domain), GFP_KERNEL);
> >         if (!domain)
> >                 return NULL;
> >
> >
> > ...
> >
> >
> >
> > Which unless I am mistaken will produce a lockdep splat and deadlock.
> >
> 
> Can you expand on this? All that code works with and without lockdep.


GFP_KERNEL can sleep and if that happens and another thread runs and
will try to take the lock, it will spin forever.


> >
> > So it looks like the previous version did not compile
> > and this one looks DOA.  What's up?
> >
> 
> In case it is useful, I forgot to enable some vendor vdpa drivers of
> HW I don't have, and that was the reason why it didn't compile. But I
> didn't send it without testing it of course.
> 
> > At this stage please include information about configs you
> > tested, and how.
> >
> > And any locking changes should also be tested with lockdep enabled
> > please.
> >
> 
> This is the text I'm planning to include in the next series, please
> let me know if you want me to expand:
> 
> Tested by creating a VDUSE device OVS with and without MQ, and live
> migrating between two hosts back and forth while maintaining ping
> alive in all the stages. All tested with and without lockdep.