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 X-Spam-Level: X-Spam-Status: No, score=-4.3 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B3440C433DB for ; Mon, 29 Mar 2021 22:05:40 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 3533F6192C for ; Mon, 29 Mar 2021 22:05:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 3533F6192C Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 9D7206B007E; Mon, 29 Mar 2021 18:05:39 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 987C06B0080; Mon, 29 Mar 2021 18:05:39 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 7FF6C6B0081; Mon, 29 Mar 2021 18:05:39 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0008.hostedemail.com [216.40.44.8]) by kanga.kvack.org (Postfix) with ESMTP id 652EE6B007E for ; Mon, 29 Mar 2021 18:05:39 -0400 (EDT) Received: from smtpin15.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay01.hostedemail.com (Postfix) with ESMTP id 20CE8180AD820 for ; Mon, 29 Mar 2021 22:05:39 +0000 (UTC) X-FDA: 77974294398.15.98E452F Received: from mail-wr1-f49.google.com (mail-wr1-f49.google.com [209.85.221.49]) by imf22.hostedemail.com (Postfix) with ESMTP id E4B75C0007C3 for ; Mon, 29 Mar 2021 22:05:35 +0000 (UTC) Received: by mail-wr1-f49.google.com with SMTP id c8so14264553wrq.11 for ; Mon, 29 Mar 2021 15:05:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=Mjz+YD2Tx5DyOSE35KploiTIrbyo0/nx0vHxTLevzjo=; b=u4MQp80YbJspUlArV+2AMcNlbbrSB0ygmvrACJSMzfcmbKJ76RPn40s9Ax3m6ZW4aP 9+q+4+mJFePQe1RCR0HZSEHgyDVWL2KUA+Aq+/McNCP1/W1RoK7d3yjHF/7+Ds0ALB3B CplGxGA4HU4MiUoNWBOvj9szAIC9Nv2iv9HxwjPoGECUHbg+U+LZ4eCTbD0kMaKYDF/t qz9/EfZc428woz9t5yuCb55P1sWK73HVMizLuepav2ejy0cMRjufJVx6ziPbviJHympF bUJBc5Ioi96yeINSOvcYAA6tdYRZ+o/9iA1lapU0YC5xA7DNyV7bmInPUIMb7xxBrxid SX4A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=Mjz+YD2Tx5DyOSE35KploiTIrbyo0/nx0vHxTLevzjo=; b=fHk4CwoswCtjapIumtHQNL43nlSkiepwyMs+9FT1Oh09OXZFJ3Ai2rwZD2pMuZbvQ8 eRO62gsJs0tymyWrfv+3eXSGZIbrnQdtjrE5tZ60yPdxWb9UOwi7zCCIxkNDZfEbc9Q5 TOehf4R6TlR1MF5XTHT9eAVoDP8hEly7Da4Lt92NOMom/n2yo7dPexMyzGqvHjdXlvcS 57Ly3FWgSpRg/hB0Jr4kiHORsIwIbtjypsX0jOYZPOUixMnI4NznAizZxql6t0tsx1uJ W7MIv8QBLJfbJMem0VKbgYkmnlkADWwldOmGxi8H49PAY9vPBeFNw0QZHRCRKtoR7sri z5og== X-Gm-Message-State: AOAM533nDAz9HRSqpkmedMahdV7vmAn1IIjWZ8cgKqZKERdGgUN80BLQ 9NlQVtwdQi/kuGXxlbAebM4= X-Google-Smtp-Source: ABdhPJySeTzypB3koxOyvmgY9zf9P7ZJV9vOyS8o1ib16/QGqeMcX3j3f0lyiHn+CndLD3mUE24Tzg== X-Received: by 2002:adf:c752:: with SMTP id b18mr30301620wrh.233.1617055537072; Mon, 29 Mar 2021 15:05:37 -0700 (PDT) Received: from [192.168.0.160] ([170.253.36.171]) by smtp.gmail.com with ESMTPSA id p10sm32803994wrw.33.2021.03.29.15.05.36 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 29 Mar 2021 15:05:36 -0700 (PDT) Subject: Re: [PATCH v4 4/4] ioctl_userfaultfd.2: Add write-protect mode docs To: Peter Xu Cc: linux-man@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Mike Rapoport , Andrea Arcangeli , Axel Rasmussen , Michael Kerrisk , Nadav Amit , Andrew Morton References: <20210322220848.52162-1-peterx@redhat.com> <20210322220848.52162-5-peterx@redhat.com> <20210323191618.GJ6486@xz-x1> <20210329215145.GE429942@xz-x1> From: "Alejandro Colomar (man-pages)" Message-ID: <5fc2ff4e-15f8-8d50-ab7c-1bb401a99ead@gmail.com> Date: Tue, 30 Mar 2021 00:05:35 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 MIME-Version: 1.0 In-Reply-To: <20210329215145.GE429942@xz-x1> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Rspamd-Server: rspam03 X-Rspamd-Queue-Id: E4B75C0007C3 X-Stat-Signature: wczcdhrz5pafu5g6roko4d6o6697ezws Received-SPF: none (gmail.com>: No applicable sender policy available) receiver=imf22; identity=mailfrom; envelope-from=""; helo=mail-wr1-f49.google.com; client-ip=209.85.221.49 X-HE-DKIM-Result: pass/pass X-HE-Tag: 1617055535-940777 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Hi Peter, On 3/29/21 11:51 PM, Peter Xu wrote: > On Thu, Mar 25, 2021 at 10:32:20PM +0100, Alejandro Colomar (man-pages) wrote: >>>>> +.TP >>>>> +.B ENOENT >>>>> +The range specified in >>>>> +.I range >>>>> +is not valid. >>>> >>>> I'm not sure how this is different from the wording above in EINVAL. An >>>> "otherwise invalid range" was already giving EINVAL? >>> >>> This can be returned when vma is not found (mwriteprotect_range()): >>> >>> err = -ENOENT; >>> dst_vma = find_dst_vma(dst_mm, start, len); >>> >>> if (!dst_vma) >>> goto out_unlock; >>> >>> I think maybe I could simply remove this entry, because from an user app >>> developer pov I'd only be interested in specific error that I'd be able to >>> detect and (even better) recover from. For such error I'd say there's not much >>> to do besides failing the app. >> >> If there's any possibility that the error can happen, it should be >> documented, even if it's to say "Fatal error; abort!". Just try to explain >> the causes and how to avoid causing them and/or possibly what to do when >> they happen (abort?). > > Okay. Would you mind me keeping my original wording? Because IMHO that > exactly does what you said as "trying to explain the causes" and so on: > > .B ENOENT > The range specified in > .I range > is not valid. > For example, the virtual address does not exist, > or not registered with userfaultfd write-protect mode. > > It's indeed slightly duplicated with EINVAL, but if you don't agree with the > wording meanwhile if you don't agree on overlapping of the errors, then what I > need is not reworking this patchset, but proposing a kernel patch to change the > error retval to make them match. I am not against proposing a kernel patch, but > I just don't see it extremely necessary. > > For my own experience on working with the kernel, the return value sometimes is > not that strict - say, it's hard to control every single bit of the possible > return code of a syscall/ioctl to reflect everything matching the document. We > should always try to do it accurate but it seems not easy to me. It's also > hard to write up the document that 100% matching the kernel code, because at > least that'll require a full-path workthrough of every single piece of kernel > code that the syscall/ioctl has called, so as to collect all the errors, then > summarize their meanings. That could be a lot of work. Yes, That's fine. I was only curious about the overlap, but if they do overlap, that's it. >>>>> +For example, the virtual address does not exist, >>>>> +or not registered with userfaultfd write-protect mode. >>>>> +.TP >>>>> +.B EFAULT >>>>> +Encountered a generic fault during processing. >>>> >>>> What is a "generic fault"? >>> >>> For example when the user copy failed due to some reason. See >>> userfaultfd_writeprotect(): >>> >>> if (copy_from_user(&uffdio_wp, user_uffdio_wp, >>> sizeof(struct uffdio_writeprotect))) >>> return -EFAULT; >>> >>> But I didn't check other places, generally I'd return -EFAULT if I can't find a >>> proper other replacement which has a clearer meaning. >>> >>> I don't think this is really helpful to user app too because no user app would >>> start to read this -EFAULT to do anything useful.. how about I drop it too if >>> you think the description is confusing? >> >> Same as above. > > Above copy_from_user() is the only place that could trigger -EFAULT so far I > can find. So either I can change above into: > > .TP > .B EFAULT > Failure on copying ioctl parameters into the kernel. > > Would you think it okay (before I repost)? I'd still prefer my original > wording because I bet 90% user developer may not even know what does it mean > when the kernel cannot copy the user parameter, and what he/she can do with > it.. However if you think it's proper I'll use it. Okay, I'll take your original words. Maybe all this "extra" info could go into the commit message. I'll wait for your resend with the a-b and the minor changes :-) Thanks, Alex -- Alejandro Colomar Linux man-pages comaintainer; https://www.kernel.org/doc/man-pages/ http://www.alejandro-colomar.es/