Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Liam Howlett <liam.howlett@oracle.com>
To:     Michel Lespinasse <michel@lespinasse.org>
CC:     Linux-MM <linux-mm@kvack.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        "kernel-team@fb.com" <kernel-team@fb.com>,
        Laurent Dufour <ldufour@linux.ibm.com>,
        Jerome Glisse <jglisse@google.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Michal Hocko <mhocko@suse.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Davidlohr Bueso <dave@stgolabs.net>,
        Matthew Wilcox <willy@infradead.org>,
        Rik van Riel <riel@surriel.com>,
        Paul McKenney <paulmck@kernel.org>,
        Song Liu <songliubraving@fb.com>,
        Suren Baghdasaryan <surenb@google.com>,
        Minchan Kim <minchan@google.com>,
        Joel Fernandes <joelaf@google.com>,
        David Rientjes <rientjes@google.com>,
        Axel Rasmussen <axelrasmussen@google.com>,
        Andy Lutomirski <luto@kernel.org>
Subject: Re: [PATCH v2 03/35] mmap locking API: name the return values
Thread-Topic: [PATCH v2 03/35] mmap locking API: name the return values
Thread-Index: AQHYFEhlS0G23Foc5UywdAOJNFKkp6x9U46A
Date:   Mon, 31 Jan 2022 16:17:43 +0000
Message-ID: <20220131161737.cclyx2ntil3opted@revolver>
References: <20220128131006.67712-1-michel@lespinasse.org>
 <20220128131006.67712-4-michel@lespinasse.org>
In-Reply-To: <20220128131006.67712-4-michel@lespinasse.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?L28n83qYkFVKll8PmI3khTQe+dJ/dqfcxj0rLq6UpQi5ltBy+w4Zon5CkeuO?=
 =?us-ascii?Q?OkXSrv4W9SymfRDXL2i39Qk7+KzwJMbBl27bs5AcXOISU5H8e1Bkxv31fryj?=
 =?us-ascii?Q?xhWv1Q9j/CEJoG/XHHV5VTUmtjTZg6RDo07SRHVjfyubADfxdlwriwKg1SvG?=
 =?us-ascii?Q?ZZYYcudf7oW39g0uP30CuflmW0dtrc7GzlHRPcaDvfNMPp4YIkG3WOoD2cvS?=
 =?us-ascii?Q?izRSrzdkSjXcGzGdVozywj1TqXHfflkr/H6AVnrpy/ObCPx45Vx42dUAT6tp?=
 =?us-ascii?Q?RZaaLU7eDqz40Qz+i1idt0/TRFYArTCmQKwaWvunN1yZMzVkhsc7hd/8P9eP?=
 =?us-ascii?Q?lPwucqim9Tr/WtmBzf9mDNNOFEAEc+YFQWPuwVCUrxkOP7VYoHmybe5eChC0?=
 =?us-ascii?Q?fEyG6gkEa/JD2tigw8Ysa4e4P3jvrCUk9JNqONn9alnRVEIujAmvXrJUONi7?=
 =?us-ascii?Q?3fT33P+9gqJXLUKwUHo0x20dCZYi7ifRjk9MQtm1ucK9dGPQVfnUldWkW+m9?=
 =?us-ascii?Q?2XwXddSq4lZ1INsIizw+q2x1sN1G2g8xhRrTlCnz8RQa07vPO/NcDMWRuqSC?=
 =?us-ascii?Q?n4SLWyjbPISXHknIUJX28iolfUWIvrxgX03kvRUHzCY8zTOUeek+Hx/h93pi?=
 =?us-ascii?Q?Emz0EkGW43MbUcsvxvWh0cdRh7OYIiLJ+agBySL7mj95en9i/C/hWgA+LTZK?=
 =?us-ascii?Q?+mkFEl/4I7dY8uAt15qqL9INpnGScdu2Ooi4YF9YfKl1RuMXs0ZqAa/LTeXl?=
 =?us-ascii?Q?1+rMblBs4PMzZds2ppAOLFciTdSgoJ7kB4lAiq78JEakCxrEjTtsixhOsR/x?=
 =?us-ascii?Q?U0MjmENzRdqPnMIeBDxycv6pThHQ1cHYeiJlsQ0qVDuxwF0lz8eypoiya7NX?=
 =?us-ascii?Q?FxPTtHTNUo9sabVsI+ZfHRyW5w2bZJQFeWxMGpDz/K7xlenemrrby14B+uZv?=
 =?us-ascii?Q?CkFm8fHz28CPOH+xMJYl7BAfbzkMgNP+mMEvF7b7METifjP1xzF+5DOiWdiI?=
 =?us-ascii?Q?0KLF/woF+5Ew0DMb3Hr4gJ8a3IITCq2d+O1KRySC2k//RBSURZnmPULNW0F9?=
 =?us-ascii?Q?oYdZHDGEYXa1KYK3g/vTaGEz/O4HlGaXdkVVNUd3pOxsdWqoiI7JQPKPqILk?=
 =?us-ascii?Q?9alo41KYCJzuHmvb5CwZXZaKXgwPqek6DVQhsgxS6oc3ZYVeAnq7qRW9ExO5?=
 =?us-ascii?Q?eOs1HX7qlITQpvJ5M3YTmVt0J0PWkl0uIECIDFX8yOvCqlc0ktdDlN2kEOfd?=
 =?us-ascii?Q?kT0coHKLau/iXQfsW05zz9w90v1Slbz/CpBf8CqM7N42q6biq8X2f/6pgblf?=
 =?us-ascii?Q?HeZIw1hHr8RWFwhDKIAWsqSxNJyONDrowG/eN66kvi3mOMskTjCAtHjyqh20?=
 =?us-ascii?Q?eEKLWFeD8Yy0ISj4RD22Vdd7e6G2VdfOsb1vXtuRAxJgFCovmkqTOs5kKWi3?=
 =?us-ascii?Q?a0XIq5AYx6A8r8CrMmlhEL1ELsuZ3s3yG3AQED0t/nrvLvDz33w9zJytZLIK?=
 =?us-ascii?Q?HmDsK427/U6v0Kjc7WDCp0I9QlezkLZ115kr5QJfDftNV5bxOqgK1sHQWJ9P?=
 =?us-ascii?Q?vs3oju1mIntkXNIjjeOH6FK3mcLDXcdPbd3WTPa388Y+SpV7fEgMD11R8Q81?=
 =?us-ascii?Q?zEW6XQLZ/7Mag6rg4VK45NY=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <699B7732D962904FACF071F0C3045786@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SN6PR10MB3022.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: aeaf5a03-2b89-4089-1e8c-08d9e4d535ec
X-MS-Exchange-CrossTenant-originalarrivaltime: 31 Jan 2022 16:17:43.2844
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: /XAtWibhFjhpFmf50CDg99JRH5YQQf/4buIrCZb3TQ25RXMxXlRICgM+M9LXHF8aNJdBHm+ss6jtUjbEKlizmQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB4615
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10244 signatures=673430
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 malwarescore=0 phishscore=0
 mlxscore=0 adultscore=0 suspectscore=0 mlxlogscore=908 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2201110000
 definitions=main-2201310106
X-Proofpoint-ORIG-GUID: BZshXYOJbcftEV9ZH-grOFpGXlwtSuYO
X-Proofpoint-GUID: BZshXYOJbcftEV9ZH-grOFpGXlwtSuYO
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

* Michel Lespinasse <michel@lespinasse.org> [220128 08:10]:
> In the mmap locking API, the *_killable() functions return an error
> (or 0 on success), and the *_trylock() functions return a boolean
> (true on success).
>=20
> Rename the return values "int error" and "bool ok", respectively,
> rather than using "ret" for both cases which I find less readable.

Would it be better to add function documentation in regards to return
types?  I think changing the variables does help, but putting a block
with Return: <what's above> would work best.

>=20
> Signed-off-by: Michel Lespinasse <michel@lespinasse.org>
> ---
>  include/linux/mmap_lock.h | 32 ++++++++++++++++----------------
>  1 file changed, 16 insertions(+), 16 deletions(-)
>=20
> diff --git a/include/linux/mmap_lock.h b/include/linux/mmap_lock.h
> index db9785e11274..1b14468183d7 100644
> --- a/include/linux/mmap_lock.h
> +++ b/include/linux/mmap_lock.h
> @@ -81,22 +81,22 @@ static inline void mmap_write_lock_nested(struct mm_s=
truct *mm, int subclass)
> =20
>  static inline int mmap_write_lock_killable(struct mm_struct *mm)
>  {
> -	int ret;
> +	int error;
> =20
>  	__mmap_lock_trace_start_locking(mm, true);
> -	ret =3D down_write_killable(&mm->mmap_lock);
> -	__mmap_lock_trace_acquire_returned(mm, true, ret =3D=3D 0);
> -	return ret;
> +	error =3D down_write_killable(&mm->mmap_lock);
> +	__mmap_lock_trace_acquire_returned(mm, true, !error);
> +	return error;
>  }
> =20
>  static inline bool mmap_write_trylock(struct mm_struct *mm)
>  {
> -	bool ret;
> +	bool ok;
> =20
>  	__mmap_lock_trace_start_locking(mm, true);
> -	ret =3D down_write_trylock(&mm->mmap_lock) !=3D 0;
> -	__mmap_lock_trace_acquire_returned(mm, true, ret);
> -	return ret;
> +	ok =3D down_write_trylock(&mm->mmap_lock) !=3D 0;
> +	__mmap_lock_trace_acquire_returned(mm, true, ok);
> +	return ok;
>  }
> =20
>  static inline void mmap_write_unlock(struct mm_struct *mm)
> @@ -120,22 +120,22 @@ static inline void mmap_read_lock(struct mm_struct =
*mm)
> =20
>  static inline int mmap_read_lock_killable(struct mm_struct *mm)
>  {
> -	int ret;
> +	int error;
> =20
>  	__mmap_lock_trace_start_locking(mm, false);
> -	ret =3D down_read_killable(&mm->mmap_lock);
> -	__mmap_lock_trace_acquire_returned(mm, false, ret =3D=3D 0);
> -	return ret;
> +	error =3D down_read_killable(&mm->mmap_lock);
> +	__mmap_lock_trace_acquire_returned(mm, false, !error);
> +	return error;
>  }
> =20
>  static inline bool mmap_read_trylock(struct mm_struct *mm)
>  {
> -	bool ret;
> +	bool ok;
> =20
>  	__mmap_lock_trace_start_locking(mm, false);
> -	ret =3D down_read_trylock(&mm->mmap_lock) !=3D 0;
> -	__mmap_lock_trace_acquire_returned(mm, false, ret);
> -	return ret;
> +	ok =3D down_read_trylock(&mm->mmap_lock) !=3D 0;
> +	__mmap_lock_trace_acquire_returned(mm, false, ok);
> +	return ok;
>  }
> =20
>  static inline void mmap_read_unlock(struct mm_struct *mm)
> --=20
> 2.20.1
> =