Re: [PATCH 3/3] mm/lru_gen: Don't build multi-gen LRU page table walk code on architecture not supported

[Aneesh Kumar K V <aneesh.kumar@linux.ibm.com>]

On 6/13/23 7:06 PM, Matthew Wilcox wrote:
> On Tue, Jun 13, 2023 at 06:58:41PM +0530, Aneesh Kumar K V wrote:
>> On 6/13/23 5:53 PM, Matthew Wilcox wrote:
>>> On Tue, Jun 13, 2023 at 05:30:47PM +0530, Aneesh Kumar K.V wrote:
>>>> @@ -4498,7 +4533,7 @@ static bool try_to_inc_max_seq(struct lruvec *lruvec, unsigned long max_seq,
>>>>  		goto done;
>>>>  	}
>>>>  
>>>> -	walk = set_mm_walk(NULL, true);
>>>> +	walk = (struct lru_gen_mm_walk *)set_mm_walk(NULL, true);
>>>
>>> This isn't C++.
>>>
>>
>> We have similar pattern for things like kmalloc()?
> 
> No.  No, we don't.  Nobody does that.  Perhaps some really crappy code
> in staging.  DO NOT USE CASTS.
> 
>> I understand the desire to have functions return
>> the correct type. But the amount of code that we are able to avoid with this patch for certain architecture is
>> really large. 
> 
> There's probably a better way to do what you're trying to do, but the
> simple fact remains that the cast you added is needed in C++ and not in C.
> Linux is not written in C++.  Do not add the cast.

What I want is to allow the usage of set_mm_walk() such that i don't need to have that #ifdef throughout the code.
I also want to keep the definition of struct lru_gen_mm_walk {} within that #ifdef as below. 

#ifdef CONFIG_LRU_TASK_PAGE_AGING

struct lru_gen_mm_walk {
	/* the lruvec under reclaim */
	...
};

static void *set_mm_walk(struct pglist_data *pgdat, bool force_alloc)
{
	struct lru_gen_mm_walk *walk = current->reclaim_state->mm_walk;
	...
	return walk;
}
#else

static inline void *set_mm_walk(struct pglist_data *pgdat, bool force_alloc)
{
	return NULL;
}

#endif