On Mon, Sep 15, 2025 at 10:11:42AM -0300, Jason Gunthorpe wrote: > On Mon, Sep 15, 2025 at 01:54:05PM +0100, Lorenzo Stoakes wrote: > > > Just mark the functions as manipulating the action using the 'action' > > > in the fuction name. > > > > Because now sub-callers that partially map using one method and partially > > map > > using another now need to have a desc too that they have to 'just know' > > which > > fields to update or artificially set up. > > Huh? There is only on desc->action, how can you have more than one > action with this scheme?
Because you use a custom hook that can in turn perform actions? As I've implemented for vmcore? > > One action is the right thing anyhow, we can't meaningfully mix > different action types in the same VMA. That's nonsense. OK, except that's how 'true' mixed maps work though right? As vmcore is doing? > > You may need more flexible ways to get the address lists down the road > because not every driver will be contiguous, but that should still be > one action. > > > The vmcore case does something like this. > > 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. > > vmcore would then populate that list with its mixture of special and > non-sepcial memory and do a single mixedmem action. I'm confused as to why you say a helper would be no good here, then go on to delineate how a helper could work... > > 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. If we can avoid custom hooks altogether that'd be ideal. Anyway I'll drop the mixed map stuff, fine. > > And maybe that is just a comment overall. This would be nicer if each > series focused on adding one action with a three-four mmap users > converted to use it as an example case. In future series I'll try to group by the action type. This series is _setting up this to be a possibility at all_. The idea was that I could put fundamentals in that should cover most cases, I could then go on to implement them in (relative) peace... I mean once I drop the mixed map stuff, and refactor to vmalloc_user(), then we are pretty much doing that, modulo a single vmalloc_user() case. So maybe I should drop the vmalloc_user() bits too and make this a remap-only change... But I don't want to tackle _all_ remap cases here. I want to add this functionality in and have it ready for next cycle (yeah not so sure about that now...) so I can then do follow up work. Am trying to do it before Kernel Recipes which I'll be at and then a (very very very needed) couple weeks vacaation. Anyway maybe if I simplify there's still a shot at this landing in time... > > 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. If I can take care of low hanging fruit relatively easily then maybe it'll be more practical to refactor the 'odd ones out'. > > Then the series goals are a bit better we can actually fully convert > and remove things like remap_vmalloc_range() in single series. That > looks feasible to me. Right. I'd love to drop unused stuff earlier, so _that_ is not an unreasonable requirement. > > Jason I guess I'll do a respin then as per above. Cheers, Lorenzo
