[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<SN6PR02MB4157EE75A14F5F7EA7D6D070D4F6A@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Fri, 17 Oct 2025 17:33:58 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Nuno Das Neves <nunodasneves@...ux.microsoft.com>,
"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC: "kys@...rosoft.com" <kys@...rosoft.com>, "haiyangz@...rosoft.com"
<haiyangz@...rosoft.com>, "wei.liu@...nel.org" <wei.liu@...nel.org>,
"decui@...rosoft.com" <decui@...rosoft.com>, "arnd@...db.de" <arnd@...db.de>,
"mrathor@...ux.microsoft.com" <mrathor@...ux.microsoft.com>,
"skinsburskii@...ux.microsoft.com" <skinsburskii@...ux.microsoft.com>
Subject: RE: [PATCH] mshv: Fix deposit memory in MSHV_ROOT_HVCALL
From: Nuno Das Neves <nunodasneves@...ux.microsoft.com> Sent: Friday, October 17, 2025 10:26 AM
>
> On 10/16/2025 6:12 PM, Michael Kelley wrote:
> > From: Nuno Das Neves <nunodasneves@...ux.microsoft.com> Sent: Thursday, October 16, 2025 12:54 PM
> >>
> >> When the MSHV_ROOT_HVCALL ioctl is executing a hypercall, and gets
> >> HV_STATUS_INSUFFICIENT_MEMORY, it deposits memory and then returns
> >> -EAGAIN to userspace.
> >>
> >> However, it's much easier and efficient if the driver simply deposits
> >> memory on demand and immediately retries the hypercall as is done with
> >> all the other hypercall helper functions.
> >>
> >> But unlike those, in MSHV_ROOT_HVCALL the input is opaque to the
> >> kernel. This is problematic for rep hypercalls, because the next part
> >> of the input list can't be copied on each loop after depositing pages
> >> (this was the original reason for returning -EAGAIN in this case).
> >>
> >> Introduce hv_do_rep_hypercall_ex(), which adds a 'rep_start'
> >> parameter. This solves the issue, allowing the deposit loop in
> >> MSHV_ROOT_HVCALL to restart a rep hypercall after depositing pages
> >> partway through.
> >
> >>From reading the above, I'm pretty sure this code change is an
> > optimization that lets user space avoid having to deal with the
> > -EAGAIN result by resubmitting the ioctl with a different
> > starting point for a rep hypercall. As such, I'd suggest the patch
> > title should be "Improve deposit memory ...." (or something similar).
> > The word "Fix" makes it sound like a bug fix.
> >
> > Or is user space code currently faulty in its handling of -EAGAIN, and
> > this really is an indirect bug fix to make things work? If so, do you
> > want a Fixes: tag so the change is backported?
> >
>
> It's the latter case, userspace doesn't handle it correctly, so I
> consider it a fix more than just an improvement.
OK, thanks. You might want to tweak the commit message a bit
to clarify that this really is a bug fix and why. I was leaning
toward the wrong conclusion based on the current commit
message.
Michael
>
> I'll add a Fixes: tag pointing back to the original /dev/mshv patch.
>
Powered by blists - more mailing lists