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
