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: <9774ddd6-60d9-245c-77ac-59951c13b263@amd.com>
Date:   Thu, 27 Oct 2022 04:07:26 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.4.0
Subject: Re: [PATCH] drm/scheduler: set current_entity to next when remove
 from rq
Content-Language: en-CA
From:   Luben Tuikov <luben.tuikov@amd.com>
To:     brolerliew <brolerliew@gmail.com>,
        "Koenig, Christian" <Christian.Koenig@amd.com>
Cc:     Alex Deucher <alexdeucher@gmail.com>, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org
References: <20221025061846.447975-1-brolerliew@gmail.com>
 <CADnq5_NweAo-7snRLkidNkOizu7Ft+7GgXCS2Rnv1oxmRFb_RQ@mail.gmail.com>
 <a70dcda0-6723-38f6-efeb-e8edb16dab55@amd.com>
 <69e672a5-a68e-7bad-fc49-4281c1c6039d@amd.com>
In-Reply-To: <69e672a5-a68e-7bad-fc49-4281c1c6039d@amd.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?OTYrKy9RR2ZxOXM2WTJGUE9QVlFjb0x1TUREQlNGVWQ4bkc5bDRWVlNPNFlE?=
 =?utf-8?B?c2VDTjNESDVlVUMvQU5PSkZmR1F2RHVCdEhSeTBERUl4cUttUW1tYVJ3b1l5?=
 =?utf-8?B?VGV2WURhWHp6dUZMQWxqRjY4aTZ0emRkN01hemNVd25LYUJtUVdZYVR1bUVx?=
 =?utf-8?B?SlN2Y2lmYVk2Qy96bk1BVlBkdk5pYkYwS1hwdDg2UzdtbnlPM1JDUGZIdVpP?=
 =?utf-8?B?YWFQeG1LTExZWG44bDc1VVV3ZDNVVWlxUHJ5VVBQQnlVWmRwNFZBVmk4aHIv?=
 =?utf-8?B?UDdIREZEcXBTdElHVEpxWml1cTQ3ZWR0dkFOVUNoOWYyQmhGYkVGTXRCbk1r?=
 =?utf-8?B?Mm1UbUtnWjd6aUtYRmR4NHJLUXQ4d1IrK011MTBVejN4S0RxZ214WUpneExm?=
 =?utf-8?B?U2gvWVN3QVpwTHpKR1FDOW9pbGRoekhYSlg0OVdiVGVOb2tkRElnNG54cWlu?=
 =?utf-8?B?Q1hNMll3TEk0NWJCNlhqLzQ0MzdnYnRIN2dyM0tEMXJxNW5lejBXNXBvT05p?=
 =?utf-8?B?L0VOY2tWeTEzUWpVSnloSTVPbTJRWVVNVFJZRFVGeE9PMnEzOHg1WjVjQ216?=
 =?utf-8?B?aUxTQTRpdWt5UmRiM25VSTgyaXJiVm13b2FPNDNocHZEa2FQUG9mdng1eFlP?=
 =?utf-8?B?TEpScjBWdVlDWE5ORmpEUUIwbklKWTVZQVJGeUNuRS8vY1FxSW5JT1llMy9t?=
 =?utf-8?B?RGdINTV5WGxiR2pGajZlZFpRaWV6c1VmMFUrZHZOT0tkMWd5TzFBUlgydEM5?=
 =?utf-8?B?RWZWd0xIL1MrQU96UDllVXNGMkpzUG5waSsxQjlBaHRBOXhCSlhiaEU5dlRS?=
 =?utf-8?B?ZXR2Vi80MzNmV2NLT21adGI3VWtYaGV5Tm01eHBHbGFsL0NZRGIyazlza1By?=
 =?utf-8?B?Q2NWYmg4R0FZeGpqTTNpbnJNQUNZdS80S3JPcGNMVC8vd1VqWHp5RThLWGdW?=
 =?utf-8?B?d0NEclFvL2gvMG05VHI3VHViVTJRemxtNHJseFltczExRmxQdzlBRVdOb0Jv?=
 =?utf-8?B?STlkVVBFQ3JlY09KdDR6NHNvZGt1dUZDTWkxMXFHcEpueWRWZHhvbHl2SGVZ?=
 =?utf-8?B?SXh6ZHpTT0lvT05QaDVlVThOQ0J1dnF3ZDF5MmhnVTlaNURadFkrcDVGNWFR?=
 =?utf-8?B?MTQ0MW9NbFVERDQ5eHhPTGRnUjdJbEM3Slg0WlVVbVpTVmJzWVBVcUs0cnpV?=
 =?utf-8?B?RVdXK2JQMFJ0Ykd6ekNJNU4wMUhpNlJHUXNkaXN1VDU0S3pUVXdQSHhGa04r?=
 =?utf-8?B?N2t5NGczeWJSUVF0aVdRZFZHUEFLTUFiSC8xVW5sOWRZelhOV1V3Z3JBL3VU?=
 =?utf-8?B?N0c1ek1RRVRwMVdaakhOZTViTmYrc0V4cGFjYk84MXFtSit1T2JHKy95QXBt?=
 =?utf-8?B?aElJOTkyenp5SEM2bjBFTWwrbitwdkQvUTlSRlEvMklKYlUyOXZodUdtYTI1?=
 =?utf-8?B?TUVtUXpiU0hOTHZqM1hnOU1TVEk5b2VUeDFzQ3hYeG9oL2Z5TmxMQWhSeVM2?=
 =?utf-8?B?YVdxczBlZllsOUpZQWNFelhlRnd1OFViSUZmUDhSa0JaY05QYk5LOXZaVnlp?=
 =?utf-8?B?MlhpQncvRjh3Qms3cTQySExNMWdmdnRYU0JHUXZLQVMxa0hveTltRnVNWnN3?=
 =?utf-8?B?RldJS1dYTkFHZDNsZ1VGM0Q0NnEvRkUzNzE4b0xpN0h0dDMxRVJTUCt0elA2?=
 =?utf-8?B?QmQ1U25wUkE0dUhtOVZCaWpjTFhhUkNwODhFNXNNQkdVMUxNQzQzUXp5N2o4?=
 =?utf-8?B?VUpQVkF5Z2xCZEM1UnNKTVdxQjlWOW5uTnlBaFY3eGJEK2lkTnJTOTNZYjE0?=
 =?utf-8?B?bDhjNW5zRFpWUEI5eFQrME1tQU9QdG9adlhxQ3B2TFROYTRMSnFTQWdWaTFN?=
 =?utf-8?B?WUdGbnNJcDNLTnp6MFdLbjVGdDVwYVFEZTROUFF3bEFUWDZ1dmdobUpvZmFj?=
 =?utf-8?B?R0xiK3JwQTFTdGgxSk9QRFQ0akgzRlk5RGdVZGlCdXdoenJUbWlkWVRsbit1?=
 =?utf-8?B?TVlXdTRJR2dVMU9sTWpYU2JibzZQUndmUmQ4WWR5OFk2WmZrQ3RnWEs5bjRn?=
 =?utf-8?B?TUV1WlgxMFRNV2Rva1kvcTRmc0cxNVhPZUE0ZXVxQXZLVG9JWk9XdGVkNXNo?=
 =?utf-8?Q?JPnGgJeualHvk5al33stF+o07?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 03d9511c-8cf2-4a1e-21a7-08dab7f24a81
X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3370.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Oct 2022 08:07:28.7541
 (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: VafEGYWrbeI4AsZ7/FxVxkaaUra6yb8IfzGw68ZKUF9pgAryeGg/bJl1JITW+e68
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN0PR12MB6102
X-Spam-Status: No, score=-2.1 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 2022-10-27 03:01, Luben Tuikov wrote:
> On 2022-10-25 13:50, Luben Tuikov wrote:
>> Looking...
>>
>> Regards,
>> Luben
>>
>> On 2022-10-25 09:35, Alex Deucher wrote:
>>> + Luben
>>>
>>> On Tue, Oct 25, 2022 at 2:55 AM brolerliew <brolerliew@gmail.com> wrote:
>>>>
>>>> When entity move from one rq to another, current_entity will be set to NULL
>>>> if it is the moving entity. This make entities close to rq head got
>>>> selected more frequently, especially when doing load balance between
>>>> multiple drm_gpu_scheduler.
>>>>
>>>> Make current_entity to next when removing from rq.
>>>>
>>>> Signed-off-by: brolerliew <brolerliew@gmail.com>
>>>> ---
>>>>  drivers/gpu/drm/scheduler/sched_main.c | 5 +++--
>>>>  1 file changed, 3 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c
>>>> index 2fab218d7082..00b22cc50f08 100644
>>>> --- a/drivers/gpu/drm/scheduler/sched_main.c
>>>> +++ b/drivers/gpu/drm/scheduler/sched_main.c
>>>> @@ -168,10 +168,11 @@ void drm_sched_rq_remove_entity(struct drm_sched_rq *rq,
>>>>         spin_lock(&rq->lock);
>>>>
>>>>         atomic_dec(rq->sched->score);
>>>> -       list_del_init(&entity->list);
>>>>
>>>>         if (rq->current_entity == entity)
>>>> -               rq->current_entity = NULL;
>>>> +               rq->current_entity = list_next_entry(entity, list);
>>>> +
>>>> +       list_del_init(&entity->list);
>>>>
>>>>         if (drm_sched_policy == DRM_SCHED_POLICY_FIFO)
>>>>                 drm_sched_rq_remove_fifo_locked(entity);
>>>> --
>>>> 2.34.1
>>>>
>>
> 
> Looks good. I'll pick it up into some other changes I've in tow, and repost
> along with my changes, as they're somewhat related.

Actually, the more I look at it, the more I think that we do want to set
rq->current_entity to NULL in that function, in order to pick the next best entity
(or scheduler for that matter), the next time around. See sched_entity.c,
and drm_sched_rq_select_entity() where we start evaluating from the _next_
entity.

So, it is best to leave it to set it to NULL, for now.

Regards,
Luben