linux-kernel - Re: [PATCH v7 1/7] vduse: make domain

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20251027125520-mutt-send-email-mst@kernel.org>
Date: Mon, 27 Oct 2025 12:57:11 -0400
From: "Michael S. Tsirkin" <mst@...hat.com>
To: Eugenio Perez Martin <eperezma@...hat.com>
Cc: Yongji Xie <xieyongji@...edance.com>,
	Stefano Garzarella <sgarzare@...hat.com>,
	Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
	virtualization@...ts.linux.dev, linux-kernel@...r.kernel.org,
	jasowang@...hat.com, Maxime Coquelin <mcoqueli@...hat.com>,
	Laurent Vivier <lvivier@...hat.com>, Cindy Lu <lulu@...hat.com>
Subject: Re: [PATCH v7 1/7] vduse: make domain_lock an rwlock

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@...hat.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.