[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <875xzbika0.fsf@yhuang6-desk2.ccr.corp.intel.com>
Date: Tue, 30 Jan 2024 11:15:35 +0800
From: "Huang, Ying" <ying.huang@...el.com>
To: Gregory Price <gregory.price@...verge.com>
Cc: Gregory Price <gourry.memverge@...il.com>, <linux-mm@...ck.org>,
<linux-kernel@...r.kernel.org>, <linux-doc@...r.kernel.org>,
<linux-fsdevel@...r.kernel.org>, <linux-api@...r.kernel.org>,
<corbet@....net>, <akpm@...ux-foundation.org>, <honggyu.kim@...com>,
<rakie.kim@...com>, <hyeongtak.ji@...com>, <mhocko@...nel.org>,
<vtavarespetr@...ron.com>, <jgroves@...ron.com>,
<ravis.opensrc@...ron.com>, <sthanneeru@...ron.com>,
<emirakhur@...ron.com>, <Hasan.Maruf@....com>,
<seungjun.ha@...sung.com>, <hannes@...xchg.org>,
<dan.j.williams@...el.com>
Subject: Re: [PATCH v3 4/4] mm/mempolicy: change cur_il_weight to atomic and
carry the node with it
Gregory Price <gregory.price@...verge.com> writes:
> On Mon, Jan 29, 2024 at 10:48:47AM -0500, Gregory Price wrote:
>> On Mon, Jan 29, 2024 at 04:17:46PM +0800, Huang, Ying wrote:
>> > Gregory Price <gregory.price@...verge.com> writes:
>> >
>> > But, in contrast, it's bad to put task-local "current weight" in
>> > mempolicy. So, I think that it's better to move cur_il_weight to
>> > task_struct. And maybe combine it with current->il_prev.
>> >
>> Style question: is it preferable add an anonymous union into task_struct:
>>
>> union {
>> short il_prev;
>> atomic_t wil_node_weight;
>> };
>>
>> Or should I break out that union explicitly in mempolicy.h?
>>
>
> Having attempted this, it looks like including mempolicy.h into sched.h
> is a non-starter. There are build issues likely associated from the
> nested include of uapi/linux/mempolicy.h
>
> So I went ahead and did the following. Style-wise If it's better to just
> integrate this as an anonymous union in task_struct, let me know, but it
> seemed better to add some documentation here.
>
> I also added static get/set functions to mempolicy.c to touch these
> values accordingly.
>
> As suggested, I changed things to allow 0-weight in il_prev.node_weight
> adjusted the logic accordingly. Will be testing this for a day or so
> before sending out new patches.
>
Thanks about this again. It seems that we don't need to touch
task->il_prev and task->il_weight during rebinding for weighted
interleave too.
For weighted interleaving, il_prev is the node used for previous
allocation, il_weight is the weight after previous allocation. So
weighted_interleave_nodes() could be as follows,
unsigned int weighted_interleave_nodes(struct mempolicy *policy)
{
unsigned int nid;
struct task_struct *me = current;
nid = me->il_prev;
if (!me->il_weight || !node_isset(nid, policy->nodes)) {
nid = next_node_in(...);
me->il_prev = nid;
me->il_weight = weights[nid];
}
me->il_weight--;
return nid;
}
If this works, we can just add il_weight into task_struct.
--
Best Regards,
Huang, Ying
Powered by blists - more mailing lists