Re: [PATCH v2 16/16] kcov: update kcov to use mmap_prepare

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Mon, Sep 15, 2025 at 09:48:01AM -0300, Jason Gunthorpe wrote:
> On Mon, Sep 15, 2025 at 01:43:50PM +0100, Lorenzo Stoakes wrote:
> > > > +	if (kcov->area == NULL || desc->pgoff != 0 ||
> > > > +	    vma_desc_size(desc) != size) {
> > >
> > > IMHO these range checks should be cleaned up into a helper:
> > >
> > > /* Returns true if the VMA falls within starting_pgoff to
> > >      starting_pgoff + ROUND_DOWN(length_bytes, PAGE_SIZE))
> > >    Is careful to avoid any arithmetic overflow.
> > >  */
> >
> > Right, but I can't refactor every driver I touch, it's not really tractable. I'd
> > like to get this change done before I retire :)
>
> I don't think it is a big deal, and these helpers should be part of
> the new api. You are reading and touching anyhow.

x ~230 becomes a big deal.

>
> > > If anything the action should be called mmap_action_vmalloc_user() to
> > > match how the memory was allocated instead of open coding something.
> >
> > Again we're getting into the same issue - my workload doesn't really permit
> > me to refactor every user of .mmap beyond converting sensibly to the new
> > scheme.
>
> If you are adding this explicit action concept then it should be a
> sane set of actions. Using a mixed map action to insert a vmalloc_user
> is not a reasonable thing to do.

Right I'm obviously intending there to be a sane interface.

And there are users who use mixed map to insert actual mixed map pages, so
having an interface for _that_ isn't crazy. So it's not like this is
compromising that.

(I mean an aside is we need to clean up a lot there anyway, it's a mess, but
that's out of scope here.)

>
> Jason

Anwyay, for the sake of getting this series in since you seem adament, I'll
go ahead and refactor in this case. But it's really not reasonable to
expect me to do this in each instance.

I will obviously try my best to ensure the API is as good as it can be, and
adapted to what mmap users need. That bit I am trying to get as right as I
can...

But in each individual driver's case, we have to be pragmatic.

Cheers, Lorenzo