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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id BC330EB64DA for ; Mon, 19 Jun 2023 08:16:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230016AbjFSIQR (ORCPT ); Mon, 19 Jun 2023 04:16:17 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55962 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229704AbjFSIQQ (ORCPT ); Mon, 19 Jun 2023 04:16:16 -0400 Received: from mail-il1-x12a.google.com (mail-il1-x12a.google.com [IPv6:2607:f8b0:4864:20::12a]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DBCD2A8 for ; Mon, 19 Jun 2023 01:16:14 -0700 (PDT) Received: by mail-il1-x12a.google.com with SMTP id e9e14a558f8ab-33d928a268eso279235ab.0 for ; Mon, 19 Jun 2023 01:16:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1687162574; x=1689754574; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=nUxNLC+gaxAx6q4yk9Zr89shiO34QlAjv1HZ696Wg1U=; b=6jB0Vaksw7QYyR3aieZrQMpKBSoFdhf1SglIAWLr1IUfcUcsjNjZT047OxQ/zQDxn/ Q9VLzY0piRJkaAmLa8H4wzNvh8yfWhfZ2IWwJ+gHskDnk2EqS3ffbkHr01hFU11k/zF0 xgsVP5PPjtZeLsInnU48wBXWitsB5Cg42pRR7NSKqCaGZsK8UpHqajQTimFQ0q5vd/9r acH2E9d4CQJy/awvToQdq8wrpNqwUmb65hWO8yhrGnfXzNPlCCI+qzY7RPDt1S85CFV0 QZ8a7hay4Wk37tQGl8s76uSpDIge/MKmH10WPIqNLvcYLXmP9kohSEeLvCiG8udvDi4l XIRw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687162574; x=1689754574; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=nUxNLC+gaxAx6q4yk9Zr89shiO34QlAjv1HZ696Wg1U=; b=kP3rqrE/syBkdpnBtjJd62NPHIoFoIcwy63NNfyQ8L+eB8mQRzdbCW+b0Xhy6dP++O ztwPm2VOKx+po92kamsEV/DACc4dZe2L2WBvndXGL/u0dHMpsjubEpjIRN85TcaVakTO yX7cmjXNc0PdQyXisvcl2bWHWJwCCYkr5OqjKRAhTuIAEcuiwn6DKPV+99d2jDoIqmJA 8LKbELsPCayvp36K2O60eAvAYrfMgCAwQy6W+u7phaFgcIYthJT3ZQEXFcUVnYOOKadc jrfgr8bhLSCBn8S13vfn1oUbdW0yxuZHAVwsym8NO/J0j8z9s/9EF82ShZkxzAmqeWGh A7sw== X-Gm-Message-State: AC+VfDz4/WT/bnNjH2Fs39g9vNW9UoVJKxJXGzs9EVQMFG4/vt5Ko042 ZnzMHmHbBW5TbIw8XGOuxQmzPAFn2VeNgJK+RMX54g== X-Google-Smtp-Source: ACHHUZ7pPgmfxfjpyVDu/lfSWd41l6pU01tdM9sEKUPUeywUHyKDUU+RDI/ym0egdMQruqWyQjzGX8yWcNRZAY/X9kE= X-Received: by 2002:a92:c243:0:b0:340:f76:4292 with SMTP id k3-20020a92c243000000b003400f764292mr894272ilo.0.1687162574121; Mon, 19 Jun 2023 01:16:14 -0700 (PDT) MIME-Version: 1.0 References: <20230613102905.2808371-1-usama.anjum@collabora.com> <20230613102905.2808371-3-usama.anjum@collabora.com> <0db01d90-09d6-08a4-bbb8-70670d3baa94@collabora.com> <34203acf-7270-7ade-a60e-ae0f729dcf70@collabora.com> <96b7cc00-d213-ad7d-1b48-b27f75b04d22@collabora.com> <39bc8212-9ee8-dbc1-d468-f6be438b683b@collabora.com> <2e1b80f1-0385-0674-ae5f-9703a6ef975d@collabora.com> In-Reply-To: <2e1b80f1-0385-0674-ae5f-9703a6ef975d@collabora.com> From: =?UTF-8?B?TWljaGHFgiBNaXJvc8WCYXc=?= Date: Mon, 19 Jun 2023 10:16:01 +0200 Message-ID: Subject: Re: [PATCH v18 2/5] fs/proc/task_mmu: Implement IOCTL to get and optionally clear info about PTEs To: Muhammad Usama Anjum Cc: Peter Xu , David Hildenbrand , Andrew Morton , Andrei Vagin , Danylo Mocherniuk , Paul Gofman , Cyrill Gorcunov , Mike Rapoport , Nadav Amit , Alexander Viro , Shuah Khan , Christian Brauner , Yang Shi , Vlastimil Babka , "Liam R . Howlett" , Yun Zhou , Suren Baghdasaryan , Alex Sierra , Matthew Wilcox , Pasha Tatashin , Axel Rasmussen , "Gustavo A . R . Silva" , Dan Williams , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kselftest@vger.kernel.org, Greg KH , kernel@collabora.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 16 Jun 2023 at 08:57, Muhammad Usama Anjum wrote: > > On 6/16/23 1:07=E2=80=AFAM, Micha=C5=82 Miros=C5=82aw wrote: > > On Thu, 15 Jun 2023 at 17:11, Muhammad Usama Anjum > > wrote: > >> On 6/15/23 7:52=E2=80=AFPM, Micha=C5=82 Miros=C5=82aw wrote: > >>> On Thu, 15 Jun 2023 at 15:58, Muhammad Usama Anjum > >>> wrote: > >>>> I'll send next revision now. > >>>> On 6/14/23 11:00=E2=80=AFPM, Micha=C5=82 Miros=C5=82aw wrote: > >>>>> (A quick reply to answer open questions in case they help the next = version.) [...] > >>>>> I guess this will be reworked anyway, but I'd prefer this didn't ne= ed > >>>>> custom errors etc. If we agree to decoupling the selection and GET > >>>>> output, it could be: > >>>>> > >>>>> bool is_interesting_page(p, flags); // this one does the > >>>>> required/anyof/excluded match > >>>>> size_t output_range(p, start, len, flags); // this one fills the > >>>>> output vector and returns how many pages were fit > >>>>> > >>>>> In this setup, `is_interesting_page() && (n_out =3D output_range())= < > >>>>> n_pages` means this is the final range, no more will fit. And if > >>>>> `n_out =3D=3D 0` then no pages fit and no WP is needed (no other sp= ecial > >>>>> cases). > >>>> Right now, pagemap_scan_output() performs the work of both of these = two > >>>> functions. The part can be broken into is_interesting_pages() and we= can > >>>> leave the remaining part as it is. > >>>> > >>>> Saying that n_out < n_pages tells us the buffer is full covers one c= ase. > >>>> But there is case of maximum pages have been found and walk needs to= be > >>>> aborted. > >>> > >>> This case is exactly what `n_out < n_pages` will cover (if scan_outpu= t > >>> uses max_pages properly to limit n_out). > >>> Isn't it that when the buffer is full we want to abort the scan alway= s > >>> (with WP if `n_out > 0`)? > >> Wouldn't it be duplication of condition if buffer is full inside > >> pagemap_scan_output() and just outside it. Inside pagemap_scan_output(= ) we > >> check if we have space before putting data inside it. I'm using this s= ame > >> condition to indicate that buffer is full. > > > > I'm not sure what do you mean? The buffer-full conditions would be > > checked in ..scan_output() and communicated to the caller by returning > > N less than `n_pages` passed in. This is exactly how e.g. read() > > works: if you get less than requested you've hit the end of the file. > > If the file happens to have size that is equal to the provided buffer > > length, the next read() will return 0. > Right now we have: > > pagemap_scan_output(): > if (p->vec_buf_index >=3D p->vec_buf_len) > return PM_SCAN_BUFFER_FULL; > if (p->found_pages =3D=3D p->max_pages) > return PM_SCAN_FOUND_MAX_PAGES; Why do you need to differentiate between those cases? > pagemap_scan_pmd_entry(): > ret =3D pagemap_scan_output(bitmap, p, start, n_pages); > if (ret >=3D 0) // success > make_UFFD_WP and flush > else > buffer_error > > You are asking me to do: > > pagemap_scan_output(): > if (p->vec_buf_index >=3D p->vec_buf_len) > return 0; > if (p->found_pages =3D=3D p->max_pages) > return PM_SCAN_FOUND_MAX_PAGES; This should be instead: n_pages =3D min(p->max_pags - p_found_pages, n_pages) ... return n_pages; > pagemap_scan_pmd_entry(): > ret =3D pagemap_scan_output(bitmap, p, start, n_pages); > if (ret > 0) // success > make_UFFD_WP and flush > else if (ret =3D=3D 0) // buffer full > return PM_SCAN_BUFFER_FULL; > else //other errors > buffer_error And this would be: if (ret > 0 && WP) WP + flush if (ret < n_pages) return -ENOSPC; > So you are asking me to go from consie code to write more lines of code. = I > would write more lines without any issue if it improves readability and > logical sense. But I don't see here any benefit. Please see the clarifications above. Best Regards Micha=C5=82 Miros=C5=82aw