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: <fcc77463-10e7-7fd4-b675-7adcb13219ed@amd.com>
Date:   Thu, 20 Oct 2022 12:47:28 -0400
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.2.2
Subject: Re: [PATCH] drm/amdkfd: use vma_lookup() instead of find_vma()
Content-Language: en-US
To:     Deming Wang <wangdeming@inspur.com>, airlied@gmail.com,
        daniel@ffwll.ch, alexander.deucher@amd.com,
        christian.koenig@amd.com, Xinhui.Pan@amd.com
Cc:     amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
        linux-kernel@vger.kernel.org
References: <20221007022651.4816-1-wangdeming@inspur.com>
From:   Felix Kuehling <felix.kuehling@amd.com>
In-Reply-To: <20221007022651.4816-1-wangdeming@inspur.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?c212c1BKSUh5UFZtWDQyeVpzMHZnZ2FSTWFrZ2gzTnhvcFBxYnIvcmQyaUV2?=
 =?utf-8?B?S1JqYzk3NW5rZDFodkwyMWJDUlh5S2UySG5TeVhzSXRlemN1aVZ3LzBLMm1z?=
 =?utf-8?B?ZkVmVFF4Wjkwdktwc3dtc0g5ZkxjNXpsTnpFSUQxMVV4TmtOZWhGK2NITjdU?=
 =?utf-8?B?aFJiRXRoL3FXaVl5TDVFS1JxWmozRVJBakkrYitpZ1VHWGhYcGozMWswRWZi?=
 =?utf-8?B?N3lRaWdaUzhTSzZFTTRHcUh3bTJZbCtkTHJ3enlPdHdQc3VUeWw1Skc2SHlW?=
 =?utf-8?B?Q0oyWTBiZkltZnc2YnQ0VDBMR09INXNnd0pwSUFDdG1ldm5TZHlqcWwxcWFL?=
 =?utf-8?B?ZVQ5TmtIOU5PSDNZdTNnNi92cTJVZVVRRFphRmFwT0l4bHd3WGJGYmk2WlVB?=
 =?utf-8?B?NHZXa3VXa0N6TDZVSDB1MkhRVzdiRXhBSUNESjlJOXpyaFRaR1JJcnRXODBY?=
 =?utf-8?B?aEx3emdaK0pNTDRzRExpZWFQUnJEc0dKdEQyMFhxcXUxaVkzZkRMalVxWHYw?=
 =?utf-8?B?UG1Kd0lvTWU5cDBxUjlzaGhycUwyR0tmY0FYcDlZNHNIdS9sY2orcHlORjhj?=
 =?utf-8?B?bys4akdQQzB3S3FNTnByaEFMZlQrZ0F3c3BIdjIwYWtBRkt3Rm1CSnB0TE1B?=
 =?utf-8?B?cHJzbmtUNERVSXlqQUVPWHBTUjNCditpRmJlTnIreUhBYVpYTDFlUTZYbDdQ?=
 =?utf-8?B?MXl2MVVkd2cxVGR6NXViL0ZtSGdSY2Y0VDZNRDFQVFN6MThmQ2VTTVNYTEk4?=
 =?utf-8?B?MnU2OXNxY256UllaRGk5dlQrK2ptdkhQbjBHM3JRVjVLaTNNMkRQdVZZcjla?=
 =?utf-8?B?Ym1uK1FEOGdYZUF6aGJOeEdscEl5N0d3bHd5cE5LbTFlbHJRTXVFajNray9F?=
 =?utf-8?B?QjNQYWlvWWhmbFZGR3hYUHJJVmFvNmIyR2lJdmZMc09EakNzdExFNU4zKzVn?=
 =?utf-8?B?K1ByTWZiMFR3cVRrbHEyNUFJSm8rZEVvTmltTC9ocUFZR3JqcVhDeWh0ZkJY?=
 =?utf-8?B?b0tpSk1TQ243K1ZoYklZK0JvTEs1YWdsVU9HNnNnUmw4dGZHaUlqZXlVbjN5?=
 =?utf-8?B?NGs4Ukg3NmRDanFleE0ya3F3RTJNL2lXK21rd01VK0t5WEhwaiswRzY2ODlL?=
 =?utf-8?B?TElCV0NUY2NkcW1TUjU5b2ZGTkhnR2grY3J4Z2pFSmhqOUgvR3FUTVFLNjVO?=
 =?utf-8?B?bXVIdGk0MVdYQUppYldkVnBLY1JSZXBsVWEvY25WWDhnS1Z2aC91M2RjdWth?=
 =?utf-8?B?SU0yZTRCc2xVOWlZUEw2NkRnYWZ0T3AyUnNXMWVYOTM1aGUyQXp1Yjd5NzRs?=
 =?utf-8?B?c3lCNWRNVUpKUUc0UW9GQktjYlRxWDgyMDZOVzhkSkZ5ZDFTRTk3MW4rY2c3?=
 =?utf-8?B?Zzd4SloyaEhRelE2czNYbGd1dWFZbnFmQzBDNy9weDIwTzdCMWVPTmozbCtY?=
 =?utf-8?B?SDFpeHNnd0E0SVVIVkpRZEoxMGtkdHBjOTE1Qk9XS2hSdEQ0VnE0ZVg3TlNy?=
 =?utf-8?B?VWFFZ2RhSTVVNmt3MTAvMXZaVlFWWFFwZklVWW8yVU1EN3V0VEY1Mnl2MENE?=
 =?utf-8?B?RDFCbWpPTkxMb2x3MnhKYWVycERrL3ZCd3VIWmVuQ05mVWhEK080ZHYrUlBD?=
 =?utf-8?B?U2F6TDFtQVNVSWI3Z0JGOVQ1ajYyQkNJdnNMdVl4Tnp5cFovVDYzSjQra1RF?=
 =?utf-8?B?L2NBeko5UW43OEx0OVhRRlNEb0Z5YlljNkVPMzV1ZzVuU0dtRWxVTzZhckls?=
 =?utf-8?B?TFUxc2tCYzJDaVJSNHZncElhRmNQeVNhSkhhY3ZhTGlZa0VKNHZ3aXBOMXND?=
 =?utf-8?B?WGM3Nld0LzBFdVlPNG80VUVYRzFMVklGL29vazJkRmJrNXQvcDJMWHBuaEFK?=
 =?utf-8?B?azBKbjFVcHlmZlMwM2FWM3VuUDh0U1ZGczlCZmFRVmhXeUVZT2JXOWdqMHhq?=
 =?utf-8?B?dHpSYUY5OURwVjg2VXo2ZFR1WHRFNHgwT2R5VlpnZFgvTlhxZnIxUkxNTSs2?=
 =?utf-8?B?M1NyWlpwcUZXUUZUc3BnVG8xbE5PR1NVZTZ3QkJtZHZ6SCtQTzQyTHBBTkdl?=
 =?utf-8?B?YlUxeDRGb3dBMGkxVGJHNExZTXVuSEhSQTdLRUNsM1BKZnRNeWpjRzd4QTlJ?=
 =?utf-8?Q?2nAEkH6SlUpq2U8bpHxd+h+q0?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c20a39f5-9647-49d0-f293-08dab2bac72a
X-MS-Exchange-CrossTenant-AuthSource: BN9PR12MB5115.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 20 Oct 2022 16:47:30.4144
 (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: 2oQ8ZNodjiI0OLLk2VHEofuGbBinHXn1ZtV+cegshJGKYiiYDPIDJd1QK8E7yZExbWzP4Ru0P+Y59OKVgpedqA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH8PR12MB7253
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,URIBL_BLOCKED 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

Am 2022-10-06 um 22:26 schrieb Deming Wang:
> Using vma_lookup() verifies the start address is contained in the found
> vma.  This results in easier to read the code.
>
> Signed-off-by: Deming Wang <wangdeming@inspur.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_migrate.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> index 2797029bd500..3599cc931b0a 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_migrate.c
> @@ -529,8 +529,8 @@ svm_migrate_ram_to_vram(struct svm_range *prange, uint32_t best_loc,
>   	for (addr = start; addr < end;) {
>   		unsigned long next;
>   
> -		vma = find_vma(mm, addr);
> -		if (!vma || addr < vma->vm_start)
> +		vma = vma_lookup(mm, addr);
> +		if (!vma)
>   			break;
>   
>   		next = min(vma->vm_end, end);
> @@ -798,8 +798,8 @@ int svm_migrate_vram_to_ram(struct svm_range *prange, struct mm_struct *mm,
>   	for (addr = start; addr < end;) {
>   		unsigned long next;
>   
> -		vma = find_vma(mm, addr);
> -		if (!vma || addr < vma->vm_start) {
> +		vma = vma_lookup(mm, addr)

There is a semicolon missing here. I will fix this before I submit the 
patch.

Please do at least a minimum amount of due diligence before posting 
patches. That would include a test to make sure your code compiles.

Regards,
   Felix


> +		if (!vma) {
>   			pr_debug("failed to find vma for prange %p\n", prange);
>   			r = -EFAULT;
>   			break;