On Mon, Sep 15, 2025 at 11:34:14AM -0300, Jason Gunthorpe wrote: > On Mon, Sep 15, 2025 at 02:51:52PM +0100, Lorenzo Stoakes wrote: > > > vmcore is a true MIXEDMAP, it isn't doing two actions. These mixedmap > > > helpers just aren't good for what mixedmap needs.. Mixed map need a > > > list of physical pfns with a bit indicating if they are "special" or > > > not. If you do it with a callback or a kmalloc allocation it doesn't > > > matter. > > > > Well it's a mix of actions to accomodate PFNs and normal pages as > > implemented via a custom hook that can invoke each. > > No it's not a mix of actions. The mixedmap helpers are just > wrong for actual mixedmap usage: > > +static inline void mmap_action_remap(struct mmap_action *action, > + unsigned long addr, unsigned long pfn, unsigned long size, > + pgprot_t pgprot) > + > +static inline void mmap_action_mixedmap(struct mmap_action *action, > + unsigned long addr, unsigned long pfn, unsigned long num_pages) > > Mixed map is a list of PFNs and a flag if the PFN is special or > not. That's what makes mixed map different from the other mapping > cases. > > One action per VMA, and mixed map is handled by supporting the above > lis tin some way.
I don't think any of the above is really useful for me to respond to, I think you've misunderstood what I'm saying, but it doesn't really matter because I agree that the interface you propose is better for mixed map. > > > > I think this series should drop the mixedmem stuff, it is the most > > > complicated action type. A vmalloc_user action is better for kcov. > > > > Fine, I mean if we could find a way to explicitly just give a list of stuff > > to map that'd be _great_ vs. having a custom hook. > > You already proposed to allocate memory to hold an array, I suggested > to have a per-range callback. Either could work as an API for > mixedmap. Again, I think you've misunderstood me, but it's moot, because I agree, this kind of interface is better. > > > So maybe I should drop the vmalloc_user() bits too and make this a > > remap-only change... > > Sure > > > But I don't want to tackle _all_ remap cases here. > > Due 4-5 or something to show the API is working. Things like my remark > to have a better helper that does whole-vma only should show up more > clearly with a few more conversions. I was trying to limit to mm or mm-adjacent as per the cover letter. But sure I will do that. > > It is generally a good idea when doing these reworks to look across It's not a rework :) cover letter describes why I'm doing this. > all the use cases patterns and try to simplify them. This is why a > series per pattern is a good idea because you are saying you found a > pattern, and here are N examples of the pattern to prove it. > > Eg if a huge number of drivers are just mmaping a linear range of > memory with a fixed pgoff then a helper to support exactly that > pattern with minimal driver code should be developed. Fine in spirit, let's be pragmatic also though. Again this isn't a refactoring exercise. But I agree we should try to get the API right as best we can. > > Like below, apparently vmalloc_user() is already a pattern and already > has a simplifying safe helper. > > > Anyway maybe if I simplify there's still a shot at this landing in time... > > Simplify is always good to help things get merged :) Yup :) > > > > Eg there are not that many places calling vmalloc_user(), a single > > > series could convert alot of them. > > > > > > If you did it this way we'd discover that there are already > > > helpers for vmalloc_user(): > > > > > > return remap_vmalloc_range(vma, mdev_state->memblk, 0); > > > > > > And kcov looks buggy to not be using it already. The above gets the > > > VMA type right and doesn't force mixedmap :) > > > > Right, I mean maybe. > > Maybe send out a single patch to change kcov to remap_vmalloc_range() > for this cycle? Answer the maybe? Sure I can probably do that. The question is time and, because most of my days are full of review as per my self-inflicted^W -selected role as a maintainer. This series will be the priority obviously :) > > Jason Cheers, Lorenzo
