Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <91bb0e78-7ca9-63fa-a3c4-d55294b33f27@amd.com>
Date:   Wed, 18 Jan 2023 11:17:22 +0530
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.13.1
Subject: Re: [RFC PATCH V1 1/1] sched/numa: Enhance vma scanning logic
Content-Language: en-US
From:   Raghavendra K T <raghavendra.kt@amd.com>
To:     Mel Gorman <mgorman@suse.de>
Cc:     linux-kernel@vger.kernel.org, linux-mm@kvack.org,
        Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Juri Lelli <juri.lelli@redhat.com>,
        Vincent Guittot <vincent.guittot@linaro.org>,
        Dietmar Eggemann <dietmar.eggemann@arm.com>,
        Steven Rostedt <rostedt@goodmis.org>,
        Ben Segall <bsegall@google.com>,
        Daniel Bristot de Oliveira <bristot@redhat.com>,
        Valentin Schneider <vschneid@redhat.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Matthew Wilcox <willy@infradead.org>,
        Vlastimil Babka <vbabka@suse.cz>,
        "Liam R . Howlett" <Liam.Howlett@Oracle.com>,
        Peter Xu <peterx@redhat.com>,
        David Hildenbrand <david@redhat.com>,
        xu xin <cgel.zte@gmail.com>, Yu Zhao <yuzhao@google.com>,
        Colin Cross <ccross@google.com>, Arnd Bergmann <arnd@arndb.de>,
        Hugh Dickins <hughd@google.com>,
        Bharata B Rao <bharata@amd.com>,
        Disha Talreja <dishaa.talreja@amd.com>
References: <cover.1673610485.git.raghavendra.kt@amd.com>
 <67bf778d592c39d02444825c416c2ed11d2ef4b2.1673610485.git.raghavendra.kt@amd.com>
 <20230117145951.s2jmva4v54lfrhds@suse.de>
 <10a06a2f-0dfc-6f36-3b7b-f4fd03153f66@amd.com>
In-Reply-To: <10a06a2f-0dfc-6f36-3b7b-f4fd03153f66@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?a1BQaExZS0ZQL0JUSk5TLzlUTmk1bFc0eGZBYm5PVFNBNEFYVzViaHpOQ05D?=
 =?utf-8?B?cHRUZzFpWk5qaU9vVE5majg0QjkzZXBsVDZzb2x1a2V2OFMyRWFPQmJENEd2?=
 =?utf-8?B?QUZ3dWRabGIweWpqRFdzODB5alNsVEIzblRTak9DQ001cnBNbEptVm83R01H?=
 =?utf-8?B?Qkx1ZkVlQlVxZmZiS2lpd1liSW9KOENDQ0VTS052dmpKb3ZmckdybGpQR0Nq?=
 =?utf-8?B?RHhWS1daYWN2RnhrR0h3K1I2NjM1WXV3dXJQazN3dHNRdEV3R0ROU3ZaL2tm?=
 =?utf-8?B?eGpuN2s1ZGdMYzVGNFhUUTI2RlV6KzdMV1ByYVBsZWVxK0hXeW1lQ1V1N0pX?=
 =?utf-8?B?UUl5LytFZU9QVGtUdkg5Q2ZyUEhtS2RvcnRRU3pKYTZoYW5pdlloOXJBM0M5?=
 =?utf-8?B?ZVNGcTFlUVJvSEVUVythZ3I3dyswRlNmdFZ5SkxuTG5CcHRhYjZiRnZ3RkQw?=
 =?utf-8?B?SVkyWTZEOEIreFZYa1hwY0p6ZnJscCs4UWlRY0dmdVg0Z2pNMHVockRDa0pp?=
 =?utf-8?B?NDBIWFVhLzVSRXgyZGtsTVRzdWpOV245K3FIYXlpTU1ETm9oWWJtQTNWWndL?=
 =?utf-8?B?SFRaV1huOHhVRGxaMzFoYjBlbG5WWmx3a3kvemxURlNCL0kxY1JMTERZamx3?=
 =?utf-8?B?ZHJyQlNVSDhpK0R0VzUySW9mRktCWHBsWUdpdXZYZUhEY2RaVW0wRHYweFRz?=
 =?utf-8?B?LzMvQWV4U2lWREZhWEVvWXp6S0diUW9JbGVzcHFOdXdGVjQ1T2w5eDlSelF3?=
 =?utf-8?B?T3Arc2hlVGM2L2dmaHFqRVIrdmV3WUhsUk52L2RwcVpSUnBiNkYxSm5kTzdo?=
 =?utf-8?B?dkczRjJ0ekswdlV4RmEvQ3FsUXFZODVFSGV4NzgxUVc0UngvTUhSS1BuZHFE?=
 =?utf-8?B?OXJubnNySjlRSTkrczVYcWlrQVY5dlVSdVprSEdjU3FmL1RXV3RIdi9sb0tD?=
 =?utf-8?B?cWYxZnk5NXdnUUpseUhDemptUkhwZmV1aVVxTnlWU1hGWlhsUEZpYi9pcmMy?=
 =?utf-8?B?dUk5VFZmVGdDdG9FNTlSTGM4MG1zTlV6NFRNMFRBamtoMEpGeDFRSXVybEhq?=
 =?utf-8?B?Q1BkcEJackIzS0p0d3A2NlcwT3pyMFgrZkNmb2RrUjVmWUo0czFnRTVCRzRW?=
 =?utf-8?B?emRONktsUkpYNERTVmxaUFRqZDJBQ0srRFdkQkpmS0JyNVpGaUpGZGZ0QXRF?=
 =?utf-8?B?S3NjeHRJenowTTAwbFAxcnhLN2Fodlc1RXNkejR0cklJU2xYUThPMDJuR0RM?=
 =?utf-8?B?L3FYdTNYcnQyOGNFcjRRRXQ2OEF3V0ROWmczNGFVSEpFRDVtQnpZdzExUzFx?=
 =?utf-8?B?ZWVKa2FyTFRzY3dUZDlHYU5Yc1VoNjRUMWV3V3NGQmlRZGg4Rk42TzRtcVpq?=
 =?utf-8?B?REREOGtwRldENG1IV2d0RUljalZjbys4TDhNWGE5bXBmc2lIdmtPbkVsZ05Y?=
 =?utf-8?B?VnplRDJ6cHFkazFCRmVuTkd5L0l4WkFvQ3g0QllLNTBKQ1JjVVMrSWVNU2Nm?=
 =?utf-8?B?QjU3d21yQ2xpNmJ0UGJZbVlLVTJRWnRTbGNLTGRQSENGOFZmU2RneDI5WnNM?=
 =?utf-8?B?ejZlMlZIQ3ExVnVzdUhBTkRKVEd5WHpyRlYxcTBWL2FyWUNhc25PRGVXUVlT?=
 =?utf-8?B?Smd2TVRvNW4vdm1XTzl1QkhwM3htSHlTVDlBd1kzazZJWlAvYVZoQnA0UkFo?=
 =?utf-8?B?Q1hjeDU2Sk8rUXd0TG1qa1hnK0pPU1Vjc3JHQXhSOHlNUXAxcVlJeXBYM2sw?=
 =?utf-8?B?M0IzYVhEYlcySURWSFc1Qm1xNUtlZDRrTGt2eDYxOGhBc014bUFDZDlCSC94?=
 =?utf-8?B?MTUzeGhPa2tSanFOcWg3RThCNCtOL250c0JrRUlmZFBjTUFzSGRBK1gxVGJv?=
 =?utf-8?B?blJFZlB1eTJYcEI3VjFocVJ2cGUwYjF5Tjg1Zld1N3ZjN0JYQXFkTDRxQm0r?=
 =?utf-8?B?TWppQ01LU1VwQmJHSVJlWEZSc1JUaXNXdCt4NlhybDIwZCszK3l1eXpsRktl?=
 =?utf-8?B?MDFuWE1ReXVDMDdTQklrRFg1VVZuOWVIQ1VEVUxGQ1NGUXdYMkhzbUFRR1RG?=
 =?utf-8?B?SGlmcGdzcVltc1lucXE3SVZwSFJ5ZFZZZzVITUtoZmJLai9hWG9VVHE3Nzlt?=
 =?utf-8?Q?fOoYhpXHDuam0ugolnlcRVCqp?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ff57b12c-395a-4ea6-7969-08daf917812a
X-MS-Exchange-CrossTenant-AuthSource: MN2PR12MB3008.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 Jan 2023 05:47:37.7173
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Uk1TnapNxyLLfzC5Oo/uF3hdO9AuZxNLDiH/Pt4dBqgafYSk2x//qf4G80pD+5L76saOG3H6PtsIhaf1iSW3qQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR12MB5315
X-Spam-Status: No, score=-2.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 1/17/2023 11:15 PM, Raghavendra K T wrote:
> On 1/17/2023 8:29 PM, Mel Gorman wrote:
>> Note that the cc list is excessive for the topic.
>>
[...]
> 
>>>   struct kioctx_table;
>>> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
>>> index e4a0b8bd941c..944d2e3b0b3c 100644
>>> --- a/kernel/sched/fair.c
>>> +++ b/kernel/sched/fair.c
>>> @@ -2916,6 +2916,35 @@ static void reset_ptenuma_scan(struct 
>>> task_struct *p)
>>>       p->mm->numa_scan_offset = 0;
>>>   }
>>> +static bool vma_is_accessed(struct vm_area_struct *vma)
>>> +{
>>> +    int i;
>>> +    bool more_pids_exist;
>>> +    unsigned long pid, max_pids;
>>> +    unsigned long current_pid = current->pid & LAST__PID_MASK;
>>> +
>>> +    max_pids = sizeof(unsigned int) * BITS_PER_BYTE / LAST__PID_SHIFT;
>>> +
>>> +    /* By default we assume >= max_pids exist */
>>> +    more_pids_exist = true;
>>> +
>>> +    if (READ_ONCE(current->mm->numa_scan_seq) < 2)
>>> +        return true;
>>> +
>>> +    for (i = 0; i < max_pids; i++) {
>>> +        pid = (vma->accessing_pids >> i * LAST__PID_SHIFT) &
>>> +            LAST__PID_MASK;
>>> +        if (pid == current_pid)
>>> +            return true;
>>> +        if (pid == 0) {
>>> +            more_pids_exist = false;
>>> +            break;
>>> +        }
>>> +    }
>>> +
>>> +    return more_pids_exist;
>>> +}
>>
>> I get the intent is to avoid PIDs scanning VMAs that it has never faulted
>> within but it seems unnecessarily complex to search on every fault to 
>> track
>> just 4 pids with no recent access information. The pid modulo 
>> BITS_PER_WORD
>> couls be used to set a bit on an unsigned long to track approximate 
>> recent
>> acceses and skip VMAs that do not have the bit set. That would allow more
>> recent PIDs to be tracked although false positives would still exist. It
>> would be necessary to reset the mask periodically.
> 
> Got the idea but I lost you on pid modulo BITS_PER_WORD, (is it
> extracting last 5 or 8 bits of PID?) OR
> Do you intend to say we can just do
> 
> vma->accessing_pids | = current_pid..
> 
> so that later we can just check
> if (vma->accessing_pids | current_pid) == vma->accessing_pids then it is
> a hit..
> This becomes simple and we avoid iteration, duplicate tracking etc
> 

Did more brainstorming/thought on this, I see that you meant

active_bit = (current_pid % BITS_PER_LONG);
accessing_pids |= (1UL << active_bit);

In scan path:
active_bit = (current_pid % BITS_PER_LONG);
if (!(accessing_pids & (1UL << active_bit))
         goto skip_scanning;

My approach above would perhaps give more false positive, this seems 
better thing to..

Thanks, will come up with numbers for this patch  + your vma scan delay 
patch.

>>
>> Even tracking 4 pids, a reset is periodically needed. Otherwise it'll
>> be vulnerable to changes in phase behaviour causing all pids to scan all
>> VMAs again.
>>
> 
> Agree. Yes this will be the key thing to do. On a related note I saw
> huge increment in numa_scan_seq because we frequently visit scanning
> after the patch
> 
[...]