linux-kernel - Re: [PATCH v2 5/7] vduse: create vduse

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CACGkMEt6fdud4t5k_veh_jr+4BGmvgZzMyWUP8JA5ki3gVt5DQ@mail.gmail.com>
Date: Thu, 18 Sep 2025 14:04:13 +0800
From: Jason Wang <jasowang@...hat.com>
To: Eugenio Perez Martin <eperezma@...hat.com>
Cc: "Michael S . Tsirkin" <mst@...hat.com>, Stefano Garzarella <sgarzare@...hat.com>, 
	Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, linux-kernel@...r.kernel.org, 
	Maxime Coquelin <mcoqueli@...hat.com>, Yongji Xie <xieyongji@...edance.com>, 
	Cindy Lu <lulu@...hat.com>, Laurent Vivier <lvivier@...hat.com>, virtualization@...ts.linux.dev
Subject: Re: [PATCH v2 5/7] vduse: create vduse_as to make it an array

On Thu, Sep 18, 2025 at 12:35 AM Eugenio Perez Martin
<eperezma@...hat.com> wrote:
>
> On Wed, Sep 17, 2025 at 10:46 AM Jason Wang <jasowang@...hat.com> wrote:
> >
> > On Tue, Sep 16, 2025 at 9:09 PM Eugenio Pérez <eperezma@...hat.com> wrote:
> > >
> > > This is a first step so we can make more than one different address
> > > spaces.  No change on the colde flow intended.
> > >
> > > Acked-by: Jason Wang <jasowang@...hat.com>
> >
> > Sorry, I think I found something new.
> >
> > > Signed-off-by: Eugenio Pérez <eperezma@...hat.com>
> > > ---
> > >  drivers/vdpa/vdpa_user/vduse_dev.c | 114 +++++++++++++++--------------
> > >  1 file changed, 59 insertions(+), 55 deletions(-)
> > >
> > > diff --git a/drivers/vdpa/vdpa_user/vduse_dev.c b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > index 9c12ae72abc2..b45b1d22784f 100644
> > > --- a/drivers/vdpa/vdpa_user/vduse_dev.c
> > > +++ b/drivers/vdpa/vdpa_user/vduse_dev.c
> > > @@ -86,6 +86,12 @@ struct vduse_umem {
> > >         struct mm_struct *mm;
> > >  };
> > >
> > > +struct vduse_as {
> > > +       struct vduse_iova_domain *domain;
> > > +       struct vduse_umem *umem;
> > > +       struct mutex mem_lock;
> > > +};
> > > +
> > >  struct vduse_vq_group_int {
> > >         struct vduse_dev *dev;
> >
> > I would expect this has an indirection for as. E.g unsigned int as;
> >
>
> This is a change from the previous commit and *dev is needed for
> taking the rwlock of the vq_group -> asid relation anyway, as function
> like vduse_dev_sync_single_for_cpu could run at the same time as
> VHOST_VDPA_SET_GROUP_ASID.
>
> I'm ok with adding the "unsigned int as" indirection but it involves
> memory walking that is not needed.

I think it should be fine unless we can notice it during the
benchmark. What's more we can refactor vduse as without caring much
about the group in the future.

>
> > >  };
> > > @@ -94,7 +100,7 @@ struct vduse_dev {
> > >         struct vduse_vdpa *vdev;
> > >         struct device *dev;
> > >         struct vduse_virtqueue **vqs;
> > > -       struct vduse_iova_domain *domain;
> > > +       struct vduse_as as;
> >
> > This needs to be an array per title:
> >
> > "vduse: create vduse_as to make it an array"
> >
>
> I meant "to make it an array in next patches". Would it work if I just
> change the patch subject to:
>
> "create vduse_as to contain per-as members"
>
> And then specify that it will be converted to an array later in the
> patch message? Or do you prefer me just to squash the patches?

I would prefer either:

1) make it an array in this patch
or
2) squash this patch into the next one

to avoid unnecessary changes.

>
> > >         char *name;
> > >         struct mutex lock;
> > >         spinlock_t msg_lock;
> > > @@ -122,9 +128,7 @@ struct vduse_dev {
> > >         u32 vq_num;
> > >         u32 vq_align;
> > >         u32 ngroups;
> > > -       struct vduse_umem *umem;
> > >         struct vduse_vq_group_int *groups;
> > > -       struct mutex mem_lock;
> > >         unsigned int bounce_size;
> > >         rwlock_t domain_lock;
> > >  };
> > > @@ -439,7 +443,7 @@ static __poll_t vduse_dev_poll(struct file *file, poll_table *wait)
> > >  static void vduse_dev_reset(struct vduse_dev *dev)
> > >  {
> > >         int i;
> > > -       struct vduse_iova_domain *domain = dev->domain;
> > > +       struct vduse_iova_domain *domain = dev->as.domain;
> >
> > This should be an iteration of all address spaces?
> >
>
> Yes, it will be in next patches.
>
> The rest of the comments have the same reply actually, as this patch
> just prepares the struct to make it an array in patches as small as
> possible.

Right.

Thanks