Re: [PATCH 5/9] mm/madvise: define and use madvise_behavior struct for madvise_do_behavior()

From: SeongJae Park
Date: Wed Mar 12 2025 - 13:29:58 EST


On Wed, 12 Mar 2025 05:47:02 +0000 Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> wrote:

> On Tue, Mar 11, 2025 at 01:56:17PM -0700, SeongJae Park wrote:
> > On Tue, 11 Mar 2025 12:17:40 +0000 Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx> wrote:
> >
> > > On Mon, Mar 10, 2025 at 10:23:14AM -0700, SeongJae Park wrote:
[...]
> > > (I wonder if that'd be better as a typedef tbh?)
> >
> > Something like below?
> >
> > typedef void *madvise_walk_arg;
> >
> > I think that could make the code easier to read. But I feel the void pointer
> > is also not very bad for the current simple static functions use case, so I'd
> > like keep this as is if you don't mind.
> >
> > Please let me know if I'm missing your point.
>
> No to be clear I meant the:
>
> int (*visit)(struct vm_area_struct *vma,
> struct vm_area_struct **prev, unsigned long start,
> unsigned long end, unsigned long arg)
>
> Function pointer.

Thanks for clarifying! And I agree this is a good idea.

>
> But this is not a big deal and let's leave it as-is for now, we can address
> this later potentially! :)

Agreed, either! :)

[...]
> Thanks for being so flexible on the feedback! Appreciated :>)

Thank you for your nice and helpful reviews :)


Thanks,
SJ