Re: [PATCH v10 7/10] xen: implement new foreign copy hypercall
Frediano Ziglio <[email protected]>
| Newsgroups | gmane.comp.emulators.xen.devel |
|---|---|
| Message-ID | <CAHt6W4dUe6ozg==rzBcY0zTciXVd0k3sotsTvsHkQrWM1ALWvQ@mail.gmail.com> |
On Mon, 17 Aug 2026 at 09:04, Jan Beulich <[email protected]> wrote: > > On 14.08.2026 16:50, Frediano Ziglio wrote: > > On Fri, 14 Aug 2026 at 15:13, Jan Beulich <[email protected]> wrote: > >> On 14.08.2026 15:47, Frediano Ziglio wrote: > >>> On Thu, 13 Aug 2026 at 15:22, Jan Beulich <[email protected]> wrote: > >>>> On 13.08.2026 16:03, Frediano Ziglio wrote: > >>>>> On Thu, 13 Aug 2026 at 10:41, Jan Beulich <[email protected]> wrote: > >>>>>> On 10.08.2026 12:30, Frediano Ziglio wrote: > >>>>>>> --- a/xen/common/memory.c > >>>>>>> +++ b/xen/common/memory.c > >>>>>>> @@ -1548,6 +1548,141 @@ static int acquire_resource( > >>>>>>> return rc; > >>>>>>> } > >>>>>>> > >>>>>>> +/* > >>>>>>> + * The "noinline" qualifier avoids the compiler to create a large function > >>>>>>> + * consuming quite a lot of stack. > >>>>>>> + */ > >>>>>>> +static int noinline mem_foreigncopy( > >>>>>> > >>>>>> I'm wondering: Is the "mem" prefix really meaningful for a static function in > >>>>>> a file named memory.c? > >>>>>> > >>>>> > >>>>> Changed > >>>>> > >>>>>>> + XEN_GUEST_HANDLE_PARAM(xen_foreigncopy_t) arg) > >>>>>>> +{ > >>>>>>> + struct domain *d, *const currd = current->domain; > >>>>>> > >>>>>> With the comment on the new XSM hooks (below) in mind: currd wants to be > >>>>>> pointer-to-const. > >>>>>> > >>>>> > >>>>> Just rebased on master, all XSM hooks accept no-const pointers to domains. > >>>>> So the suggested change would create warnings. > >>>> > >>>> Well, as per below, I pointed you at a particular pending patch, a single > >>>> hunk of which could be broken out. > >>> > >>> Yes, but my changes would have to have casts from const pointers to > >>> no-const pointers to avoid warnings and the patch you are pointing to > >>> would have to remove these casts. I find this less clean than having > >>> one patch using the current code style (that is no-const pointers) and > >>> another that changes the style entirely. > >>> But obviously this is just my opinion. > >> > >> Such casts would be unacceptable. What instead I have been trying to convey: > >> Your patch wants to gain a dependency on my patch. And if my patch would > >> take too long to make it in, that one hunk could be broken out into a > >> separate, easy to get in patch. > > > > Okay, then the only choice that's left is the code producing warnings > > as const pointers are passed to functions requiring no-const pointers. > > Is this acceptable? Apparently as you are suggesting it it is. > > That's not acceptable, the more that due to -Werror this would break the > build. But that's also not what I said, and I'm having a hard time seeing > how what I said can be mis-interpreted. What exactly is not clear in "Your > patch wants to gain a dependency on my patch"? > > Btw, I'm about to submit v2 of that XSM series, where I've broken out that > hunk (for the change to then hopefully go in quickly, allowing you to > simply re-base rather than carrying a prereq patch). > > Jan Okay, being dependent on https://lists.xenproject.org/archives/html/xen-devel/2026-08/msg00768.html would be acceptable. What was not was being dependent on other large series. That is more in line to what I proposed multiple times, that is having a preparation patch for constification and code using const pointer on my patch. Frediano