Date:   Tue, 24 Jan 2023 10:41:28 -0400
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     Alistair Popple <apopple@nvidia.com>
Cc:     linux-mm@kvack.org, cgroups@vger.kernel.org,
        linux-kernel@vger.kernel.org, jhubbard@nvidia.com,
        tjmercier@google.com, hannes@cmpxchg.org, surenb@google.com,
        mkoutny@suse.com, daniel@ffwll.ch,
        Christian Benvenuti <benve@cisco.com>,
        Nelson Escobar <neescoba@cisco.com>,
        Leon Romanovsky <leon@kernel.org>, linux-rdma@vger.kernel.org
Subject: Re: [RFC PATCH 06/19] RDMA/usnic: convert to use vm_account
Message-ID: <Y8/uGL+TA7ow4Zmu@nvidia.com>
References: <cover.f52b9eb2792bccb8a9ecd6bc95055705cfe2ae03.1674538665.git-series.apopple@nvidia.com>
 <03ed2d166826cd7055810c66a175e20fa2153c52.1674538665.git-series.apopple@nvidia.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <03ed2d166826cd7055810c66a175e20fa2153c52.1674538665.git-series.apopple@nvidia.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?aYxYE4MVFfZUfhz6HQeV/K+4L1TejtqI/CWsU/t5iW/ztdkW1uoNXFxmgFjK?=
 =?us-ascii?Q?AyYDyNkEv0ZhakGaYA4sJiye0BonykOJUjLWYbS+DBU+6l4wsckxPrdIavFH?=
 =?us-ascii?Q?2ceV4Q1b9G3X9Tw4oXLupafdMRsL9ciJeievDPEWf1EnxXXwysMX6P9HPEYB?=
 =?us-ascii?Q?nhxrnrZfSCrYyWXMyTxZKmp0gsNjQHhsSPGldqbX83XBdqtAh+ESwuJoSngJ?=
 =?us-ascii?Q?76p9XtormxRndwFF2OAaOdidJ0jfEcT1y/4tYZVnVefUhpt9q4dgfam1VJ1I?=
 =?us-ascii?Q?ZGe3958jvuiejXO2hXteR0465gVXh649YIpqZOrMh3ILc3nkCFPAghPOTOLc?=
 =?us-ascii?Q?5J3RXWU+FDnsEVzTBcTrmd/9fh+huVGmjjXJ8BuFB0Zu6x4ivESHLo5Vjkay?=
 =?us-ascii?Q?eXi4Mo01jf/g8Yxd8XeoLZExVvs/3yNMU+rf2FXYDdawMrREccDrO7WxSel8?=
 =?us-ascii?Q?nuvFrxx4z14NqGPwudvfZFws+2yBS5dnUUcYofgYddYxH4MmX3mbwfYW4Agr?=
 =?us-ascii?Q?biygpKyxlUZqag1eE8mve8JMe42IXJRjBhQoqhdGlk7No8h8aE/gH4KEwUQt?=
 =?us-ascii?Q?QmP/kZLB7zgyQQNsDWQdgyRWx1w9ML5MH1IvM7Pek8ophVh+v9c7IeZs5+XN?=
 =?us-ascii?Q?RCv3VKDR+5UqVEPNFbFzW+571JUbARqq7mRJuQMuW2aaiuJ047wFSl6IHb9I?=
 =?us-ascii?Q?0D6Dhkd/+ydFCqqMibpKcKuxWXk/k1Yldy/agDrGD5l8FdsSN3uC4TI0fnLx?=
 =?us-ascii?Q?eZRw2pjdDqsyJ4D/J0lAeonyLahm4kftqFjqIC4d53h6BkmWJ8W9kGVvjxBS?=
 =?us-ascii?Q?TR/G99LAkT0iq4L5bwnYcviNoHfhc1QMPscqMw24TMDCpbg8khjFtzgwG7ZJ?=
 =?us-ascii?Q?alp/DZYcAgTwTPbVbe+bvua4IXALFJ59tfYMJGXKzg7ElBG2k6EXGlUCxVff?=
 =?us-ascii?Q?Z9fXOuo1GMYE8ByFUeSi3sF/1N6TRQwRylZ16p/l2pZuodcdm+RZQRv4Jrvs?=
 =?us-ascii?Q?1lLQNKmUKGBf0E3nIaOIbnfrvu9NGkT/wEnLl9yaM/G3iQbgYUPd7VgLM/2b?=
 =?us-ascii?Q?AOonnWB5VdX3mWkRgjX0nM6ry7xWQH/1ZvQoARtGSvbf1X3qiF1t8ii9UtVO?=
 =?us-ascii?Q?Ppkru0jD99z6/6w6Tbhxz71QJfWEae0pSmFZR5pKKffjnsgLsPNrMZgxczW5?=
 =?us-ascii?Q?r1vVRIrzbCGGdjSVi2lWpjaPFYXrqpFzkDFIE94bymV5olDL2hJFd28PiVij?=
 =?us-ascii?Q?1Z/Ko/3ubXZlVlljVhSwZS3ut3XLYHajw7jq8WGqqAMtbW5Wucch5FooALd9?=
 =?us-ascii?Q?PvwtXBV7eRA292l18bTtGwDyRY4YOPsQwUEjexZLbhI4MYA6/kygyQz86Yqf?=
 =?us-ascii?Q?wMLyiX8XCHVwtkRIxylxCZzHVC7henlL6NqqDwEnsiYBkR/0he5OhJPjGUTl?=
 =?us-ascii?Q?ypdRIrRMUhukWqGUztBNHEjWqW0RKkRvclCtYvsIKiIdHUdccut0ZDRw08cb?=
 =?us-ascii?Q?bwGoxB7beNTYzFvQ3CodDX2B/XRrLOukhAo0mD0sRcTY4MMMUcE9dzgQGOKV?=
 =?us-ascii?Q?msePoMZ1znk3dajlrhmSyiTSxPazO8c4SArdfnxY?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 10a20138-2937-44af-6be0-08dafe1913fc
X-MS-Exchange-CrossTenant-AuthSource: LV2PR12MB5869.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Jan 2023 14:41:29.1636
 (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: Ptw+XL35iN6kXublrCFnBFlKf38ALQ8ITH30j7Cpz2s+PGZ20mYwNjS1MV+70tUP
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR12MB7909
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Jan 24, 2023 at 04:42:35PM +1100, Alistair Popple wrote:
> diff --git a/drivers/infiniband/hw/usnic/usnic_uiom.c b/drivers/infiniband/hw/usnic/usnic_uiom.c
> index c301b3b..250276e 100644
> --- a/drivers/infiniband/hw/usnic/usnic_uiom.c
> +++ b/drivers/infiniband/hw/usnic/usnic_uiom.c
> @@ -89,8 +89,6 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable,
>  	struct page **page_list;
>  	struct scatterlist *sg;
>  	struct usnic_uiom_chunk *chunk;
> -	unsigned long locked;
> -	unsigned long lock_limit;
>  	unsigned long cur_base;
>  	unsigned long npages;
>  	int ret;
> @@ -123,10 +121,8 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable,
>  	uiomr->owning_mm = mm = current->mm;
>  	mmap_read_lock(mm);
>  
> -	locked = atomic64_add_return(npages, &current->mm->pinned_vm);
> -	lock_limit = rlimit(RLIMIT_MEMLOCK) >> PAGE_SHIFT;
> -
> -	if ((locked > lock_limit) && !capable(CAP_IPC_LOCK)) {
> +	vm_account_init_current(&uiomr->vm_account);
> +	if (vm_account_pinned(&uiomr->vm_account, npages)) {
>  		ret = -ENOMEM;
>  		goto out;
>  	}

Is this error handling right? This driver tried to avoid the race by
using atomic64_add_return() but it means that the out label undoes the add:


> @@ -178,7 +174,8 @@ static int usnic_uiom_get_pages(unsigned long addr, size_t size, int writable,
>  out:
>  	if (ret < 0) {
>  		usnic_uiom_put_pages(chunk_list, 0);
> -		atomic64_sub(npages, &current->mm->pinned_vm);

Here

> +		vm_unaccount_pinned(&uiomr->vm_account, npages);
> +		vm_account_release(&uiomr->vm_account);

But with the new API we shouldn't call vm_unaccount_pinned() if
vm_account_pinned() doesn't succeed?

Jason