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: <7398324a-2134-3501-ca81-67a56a4c8629@nvidia.com>
Date:   Sun, 21 Aug 2022 17:14:01 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.12.0
Subject: Re: [PATCH] mm/gup.c: Fix return value for __gup_longterm_locked()
Content-Language: en-US
To:     Shigeru Yoshida <syoshida@redhat.com>, akpm@linux-foundation.org
Cc:     linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        Alistair Popple <apopple@nvidia.com>,
        syzbot+616ff0452fec30f4dcfd@syzkaller.appspotmail.com
References: <20220821183547.950370-1-syoshida@redhat.com>
From:   John Hubbard <jhubbard@nvidia.com>
In-Reply-To: <20220821183547.950370-1-syoshida@redhat.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?aGNVdnl0YjZoZkpRM3hkUkpRNERpbjM2MnhVcGxpNFJaTDBvWVNUKzcvNXFa?=
 =?utf-8?B?QmM4TGR5U3FybkxkZEVLZWpDd25teXo5WGZPcmluYjlRTjRHTjZtUkVJWXk2?=
 =?utf-8?B?aGY0MzlTSDlvLy9KWXo4TXhGUFR5cmUvZ3g1Uy9TN3hQZVBsOUVVbkt6RWpQ?=
 =?utf-8?B?akxsWDl4eEU4S2VhQnVRR1piUW5jL0F5aUtEcW5rMFJMc2h0Q2FiT1RUQjYw?=
 =?utf-8?B?eVRXTlJRd3d2MDVTRUc0SDZhS3lmWER2bjRSTzM3Z0M5S2F3ZVI4QzVYTTcy?=
 =?utf-8?B?RVdiY1huSmZYQmFsd0tWSzk2WWVJdTE1aXREMGtLSmlEb2tNM0VDcTdFWnlY?=
 =?utf-8?B?bUNvZnBRMjNnL3RDbm1qbGl6Kzk5OWQ0a3FsT0ZOWFJUb3R2RGI0dEF4RGR2?=
 =?utf-8?B?UVRXZXNWMURJS3NUL0h6ZDhYRnlKKy81OFVOQ1QraVRWNXZyK2wydW4rOUtB?=
 =?utf-8?B?cUYwRng2YTNCZWFaWXJWOXpTZUZkcWdNUUxweWEzMlgxOVlhWnhCZCtuVlBO?=
 =?utf-8?B?T3hRNU8yTmxRdVZxRmFCK290RDR5VjdWUVY0TUtGTzdKdDZWa3cvRjFCSFZX?=
 =?utf-8?B?YkJMSkpPbFpDMEtFY0RHRnBIS2pmdmpFWHptYVRsNzVCYkZISytyQVkwZ0V2?=
 =?utf-8?B?ekR1bVBVa2ZVeCtIYVhYVWJyL0pObU1yWnVZZ09taDdSUEQwbC9uMGJsZGNs?=
 =?utf-8?B?Q1MrV2FKcXppNjZNcWV0Qm5YV3ZjSFcrMjB1dTJWNk9wdW91VGFnakM5cnY4?=
 =?utf-8?B?clY5V3JFYUUxR1FrTHhlUFpTUVlJVndnVjliLzBFYnRBNGtOM2dHaHdrUjRH?=
 =?utf-8?B?c3F4SGdSODVIdzIyajhleFpXK3FPWENEc3RUc2ExMDhZek4zQU93dzd1ZEFB?=
 =?utf-8?B?Z2lEUXpUVGI2alRNRlIxZDI4ZzFrbTZWQUtnamR6NlpvZEI1UEFnQU9sYlAy?=
 =?utf-8?B?K3R6UHA5TTFDUU1vQmRTRHB2YTJWek43cXduUG9iUVQ2OUxwTE9qUHdFdW9K?=
 =?utf-8?B?Tm0yRGNNSmVnOEJ2OS9vWXREM1BsN3FhdEs4WXNrTHFZM1ZWMjVJdWZmSncr?=
 =?utf-8?B?SHQ2QVV6SVM5dUFLM28yQ0ZDTFppTGZHaGpHRE52L21YNjBBa0R3Sk9XNWl1?=
 =?utf-8?B?WnhrZTJHNkZ3RjRhWllkNm1CUVRvdmNRUk5hVHB3YXpURU0zVDVUL2lvZmN5?=
 =?utf-8?B?VmNLMTlTeEliSzI2V2Z1Qm5TY1RyZ01iWCtPbFRoa0szYkllaWc5RTM2NDlG?=
 =?utf-8?B?NG5HRFJSVVZGdkQxT3JDSVQ1anBMTS9xejNIeVRTQ01LQ3E4b0ErV3BZTHJh?=
 =?utf-8?B?ZFZYb3UyUCtxNzg0aUNqSUhIdEZUODd0TDl2a1M4WE1JMkplN1FBZTlGa3Zp?=
 =?utf-8?B?Q2dVR0Z6d05YaWQ1aUI4WTVxZVdxbHJqZFA0TjFlUU5OcnVNRnRzbTdWK1pq?=
 =?utf-8?B?YkowaTRVYjdkY1NwVjNueDhKZm5rV1NZclhLZGUyRWZJalNjNFUxM1ZlTXR4?=
 =?utf-8?B?UkRWakV4MXdGS3hzNmp5UzAzMjJ2cjhnKzFSU0NuUEJhTFRtWkEvYlRhRXJz?=
 =?utf-8?B?c0hKNXJ1T3l1ajVyTWhEd2tFQkpsVTJSVTFKTXY1MlZOcFIzRk11SzVUbHVv?=
 =?utf-8?B?ZzhWcXJlUXFtNCtwcktTWEZHdmNJN3dQZ2hzSzFNdEoxaTVZb3dkTGlabFha?=
 =?utf-8?B?STBQcFk2ZVNWZXY5dDJhQXpRcE9vYllKUXd3M25JMk9qekhFM3YwYVRtOWwx?=
 =?utf-8?B?Uk8rT1lySlBmeEtzYklvZytNMWd6REQwbUZ1ZjhnU2Nmd01WNndubVFWTHFH?=
 =?utf-8?B?MmIvdlJCMHcwRlNoZ0UzVTZJTjQ2eFBwUG5sWHlWelJXK1UzZU1kV2g4WFF5?=
 =?utf-8?B?cHFKNDkxVnAwMnF2Z0kzcDhRTUdScU5LR2tBYTBzdWR4RVpXVkF3aHFtc2gw?=
 =?utf-8?B?cEZUbnpWakpPMDVTZnRna2k3aEg2Y09XSWNMZ3Bna2pYMHp4R3ZwRXdqaXhT?=
 =?utf-8?B?NkZlUTVvUUYrODBlQmc2ZlVaWGxtbnd3MU1LRGx4cHR6VER2TXkxb1lHUGph?=
 =?utf-8?B?N0tLSCtjNXpVSG9VVFNJV1dXMTFtSG5WTzkxN0tWYWtrMExqNTA1MGtKNVBF?=
 =?utf-8?Q?Rc7Vus4BGpGse5ZYlzdlixcbK?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: c43a3618-eb7e-492c-e4a4-08da83d338ae
X-MS-Exchange-CrossTenant-AuthSource: BY5PR12MB4130.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 22 Aug 2022 00:14:03.9609
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 2FqLmMWxc6w4zzqeA5dP0SvY20FkMWZTPYuI4lDCarCu6gxyTixqCapTPs4N6c3WvG94PzuYQ1Wxi9oa9x07qw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR12MB6826
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        NICE_REPLY_A,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,
        SPF_NONE,T_SCC_BODY_TEXT_LINE 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 8/21/22 11:35, Shigeru Yoshida wrote:
> __get_user_pages_locked() may return the number of pages less than
> nr_pages.  So __gup_longterm_locked() have to return the number of
> pages __get_user_pages_locked() returns if it succeeded, not nr_pages
> passed.

s/passed/requested/

> 
> Fixes: 61c63c2076d9 (mm/gup.c: simplify and fix check_and_migrate_movable_pages() return codes)
> CC: Alistair Popple <apopple@nvidia.com>
> Reported-by: syzbot+616ff0452fec30f4dcfd@syzkaller.appspotmail.com
> Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
> ---
>   mm/gup.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)

Reviewed-by: John Hubbard <jhubbard@nvidia.com>

(with a couple of nits about line length, below)

> 
> diff --git a/mm/gup.c b/mm/gup.c
> index 5aa7531a703b..542cf74c59b0 100644
> --- a/mm/gup.c
> +++ b/mm/gup.c
> @@ -2068,22 +2068,22 @@ static long __gup_longterm_locked(struct mm_struct *mm,
>   				  unsigned int gup_flags)
>   {
>   	unsigned int flags;
> -	long rc;
> +	long rc, nr_pinned_pages;
>   
>   	if (!(gup_flags & FOLL_LONGTERM))
>   		return __get_user_pages_locked(mm, start, nr_pages, pages, vmas,
>   					       NULL, gup_flags);
>   	flags = memalloc_pin_save();
>   	do {
> -		rc = __get_user_pages_locked(mm, start, nr_pages, pages, vmas,
> -					     NULL, gup_flags);
> -		if (rc <= 0)
> +		nr_pinned_pages = __get_user_pages_locked(mm, start, nr_pages, pages, vmas,
> +							  NULL, gup_flags);

Can you please wrap at 80 cols, though?

> +		if (nr_pinned_pages <= 0)
>   			break;
> -		rc = check_and_migrate_movable_pages(rc, pages, gup_flags);
> +		rc = check_and_migrate_movable_pages(nr_pinned_pages, pages, gup_flags);

Also here.

>   	} while (rc == -EAGAIN);
>   	memalloc_pin_restore(flags);
>   
> -	return rc ? rc : nr_pages;
> +	return rc ? rc : nr_pinned_pages;
>   }
>   
>   static bool is_valid_gup_flags(unsigned int gup_flags)

thanks,
-- 
John Hubbard
NVIDIA