From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 98398C433F5 for ; Thu, 10 Mar 2022 09:35:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E51AA8D0002; Thu, 10 Mar 2022 04:34:59 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id E00828D0001; Thu, 10 Mar 2022 04:34:59 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id D165B8D0002; Thu, 10 Mar 2022 04:34:59 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.a.hostedemail.com [64.99.140.24]) by kanga.kvack.org (Postfix) with ESMTP id C33988D0001 for ; Thu, 10 Mar 2022 04:34:59 -0500 (EST) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 9300B22CE9 for ; Thu, 10 Mar 2022 09:34:59 +0000 (UTC) X-FDA: 79227967518.05.82E0209 Received: from alexa-out-sd-01.qualcomm.com (alexa-out-sd-01.qualcomm.com [199.106.114.38]) by imf29.hostedemail.com (Postfix) with ESMTP id 8445A12001F for ; Thu, 10 Mar 2022 09:34:58 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=quicinc.com; i=@quicinc.com; q=dns/txt; s=qcdkim; t=1646904898; x=1678440898; h=message-id:date:mime-version:subject:to:cc:references: from:in-reply-to:content-transfer-encoding; bh=/IalRAFcuzbyyT+wYoMtKv0yxgtlvFF8P0B3U3hiXSQ=; b=QBjoxIIc4ojZRenK5MwHLocJpx6CziHa/9wyyQn0zfr6wO+oa5rjZq9I HyVw6ot3hgP6fN8hrJ3v/6PPyxKZW9mo7wVDevrYaQLO72XA92mHZOPya +RcnFzaC42zIoCe1zg4Qw5h/duxqcsvJlBoMjuuuLdCmfRym8+BrYwsZg w=; Received: from unknown (HELO ironmsg04-sd.qualcomm.com) ([10.53.140.144]) by alexa-out-sd-01.qualcomm.com with ESMTP; 10 Mar 2022 01:34:57 -0800 X-QCInternal: smtphost Received: from nasanex01c.na.qualcomm.com ([10.47.97.222]) by ironmsg04-sd.qualcomm.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Mar 2022 01:34:56 -0800 Received: from nalasex01a.na.qualcomm.com (10.47.209.196) by nasanex01c.na.qualcomm.com (10.47.97.222) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.15; Thu, 10 Mar 2022 01:34:34 -0800 Received: from [10.216.27.16] (10.80.80.8) by nalasex01a.na.qualcomm.com (10.47.209.196) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.2.986.15; Thu, 10 Mar 2022 01:34:30 -0800 Message-ID: <3846d4ff-c8ab-3c44-1974-fee451894c0d@quicinc.com> Date: Thu, 10 Mar 2022 15:04:26 +0530 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.6.0 Subject: Re: [PATCH] mm: madvise: return correct bytes advised with process_madvise Content-Language: en-US To: Nadav Amit , Minchan Kim CC: Andrew Morton , , Stephen Rothwell , David Rientjes , , Michal Hocko , Linux-MM , Linux Kernel Mailing List References: <1646803679-11433-1-git-send-email-quic_charante@quicinc.com> From: Charan Teja Kalla In-Reply-To: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [10.80.80.8] X-ClientProxiedBy: nasanex01b.na.qualcomm.com (10.46.141.250) To nalasex01a.na.qualcomm.com (10.47.209.196) X-Rspamd-Queue-Id: 8445A12001F X-Stat-Signature: qkpfzyfw97qrfr18cty5tkyfiuj9z1na X-Rspam-User: Authentication-Results: imf29.hostedemail.com; dkim=pass header.d=quicinc.com header.s=qcdkim header.b=QBjoxIIc; spf=pass (imf29.hostedemail.com: domain of quic_charante@quicinc.com designates 199.106.114.38 as permitted sender) smtp.mailfrom=quic_charante@quicinc.com; dmarc=pass (policy=none) header.from=quicinc.com X-Rspamd-Server: rspam07 X-HE-Tag: 1646904898-142478 X-Bogosity: Ham, tests=bogofilter, spamicity=0.004002, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Thanks Amit for the inputs!! On 3/10/2022 12:20 AM, Nadav Amit wrote: > --- > mm/madvise.c | 12 +++++++++--- > 1 file changed, 9 insertions(+), 3 deletions(-) > > diff --git a/mm/madvise.c b/mm/madvise.c > index 38d0f51..d3b49b3 100644 > --- a/mm/madvise.c > +++ b/mm/madvise.c > @@ -1426,15 +1426,21 @@ SYSCALL_DEFINE5(process_madvise, int, pidfd, const struct iovec __user *, vec, > > while (iov_iter_count(&iter)) { > iovec = iov_iter_iovec(&iter); > + /* > + * Even when [start, end) passed to do_madvise covers > + * some unmapped addresses, it continues processing with > + * returning ENOMEM at the end. Thus consider the range > + * as processed when do_madvise() returns ENOMEM. > + * This makes process_madvise() never returns ENOMEM. > + */ > > I fully understand and relate to the basic motivation of this > patch. > > The ENOMEM that this patch checks for, IIUC, is the ENOMEM that is > returned on unmapped holes. Such ENOMEM does not appear, according to > the man page, to be a valid reason to return ENOMEM to userspace. > Presumably process_madvise() is expected to skip unmapped holes > and not to fail because of them> True, that ENOMEM represents the VMA passed contains the unmapped holes. Pasting the Documentation of do_madvise(): * -ENOMEM - addresses in the specified range are not currently * mapped, or are outside the AS of the process. Internally process_madvise() calls do_madvise() in a loop by passing the vma it received in 'struct iovec'. And I too agree here that process_madvise() is expected to process the unmapped holes. > Having said that, I do not think that the check that the patch does > is clean or clearly documented. If it is about the Documentation, how about adding: "Since process_madvise() is expected to process unmapped holes, never return ENOMEM received from do_madvise() to user". If the code changes can be made further cleaner, please suggest. > > In addition, this patch (and some work on process_madvise()) raise > in my mind a couple of questions: > > 1. There are other errors that process_madvise might encounter > and can be propagated back to userspace, but are not > documented. For instance if can_madv_lru_vma() fails on > MADV_COLD, userspace will get EINVAL. EINVAL is not documented > as a valid error-code for such case in either madvise() and > process_madvise() man pages. I agree here with the man page documentations too and felt the same while going through them. For the mentioned case too, in the madvise[1] man page, EINVAL return type is only talked for MADV_DONTNEED and MADV_REMOVE. It should also contains for MADV_PAGEOUT, MADV_COLD and as well for MADV_FREE. The other missing return types, which I came across, in process_madvise are: EINVAL - return from process_madvise_behavior_valid(). EINTR - from mm_access() EACCES - from mm_access() Thanks, Charan