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 5F9B0EB64D7 for ; Tue, 20 Jun 2023 22:05:21 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 9E0398D0002; Tue, 20 Jun 2023 18:05:20 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 990648D0001; Tue, 20 Jun 2023 18:05:20 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 80A428D0002; Tue, 20 Jun 2023 18:05:20 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id 6C3C28D0001 for ; Tue, 20 Jun 2023 18:05:20 -0400 (EDT) Received: from smtpin18.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay10.hostedemail.com (Postfix) with ESMTP id 39560C0408 for ; Tue, 20 Jun 2023 22:05:20 +0000 (UTC) X-FDA: 80924508000.18.F94C41B Received: from mail-ed1-f50.google.com (mail-ed1-f50.google.com [209.85.208.50]) by imf18.hostedemail.com (Postfix) with ESMTP id 567B81C0020 for ; Tue, 20 Jun 2023 22:05:18 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=B8QESZcP; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf18.hostedemail.com: domain of emmir@google.com designates 209.85.208.50 as permitted sender) smtp.mailfrom=emmir@google.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1687298718; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=6QnZ16+cR3SKz8OZyy5d1CsXcXjkIRyncSh8yNxkG1M=; b=xjmcVBUjCikzGgBPWI2qT0d8IPhLnMWznGg3vU3hdz1KyemzolIWqw0UEwBPVSkPRffPaE tawAVDqEDSWGiy9ewFqLGPsajuKhRcfoK8urdDxZeCyOrkGSx4WO6dK33w0bNskhNuEQ25 5LEyif5jK3kemKB+usp1/ZrotJhX0g8= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=pass header.d=google.com header.s=20221208 header.b=B8QESZcP; dmarc=pass (policy=reject) header.from=google.com; spf=pass (imf18.hostedemail.com: domain of emmir@google.com designates 209.85.208.50 as permitted sender) smtp.mailfrom=emmir@google.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1687298718; a=rsa-sha256; cv=none; b=rkJxJNWVTZPbv9Nh8i9q+re6Z/yAFTLjbIyXmEdcuImIozV0WZB80u8T35y9jrvDbu3qUU wbxgbZL2Jrrz/PTVuCKCWK/yJsrpJk54Cr2lWIRB8a9tmzSKikZ1fOponkENEq5I6B0Mv6 4aHd7GwGiXhCGTSjlKBbF3QDh0fMCxg= Received: by mail-ed1-f50.google.com with SMTP id 4fb4d7f45d1cf-514ad92d1e3so75a12.1 for ; Tue, 20 Jun 2023 15:05:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20221208; t=1687298717; x=1689890717; 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=6QnZ16+cR3SKz8OZyy5d1CsXcXjkIRyncSh8yNxkG1M=; b=B8QESZcPS8IOqAQ9veug7dutrAkK9Ty1tsh2jQp6IbIscVsQ3171n8gu4ay4YA1jka SkkY8idwFT4BOwqktrmuEbxD0Q/eYeyQGlApPqQBEFgsOCmzBHnvaRCdjry6zWAgVZ4G yyFUmiO18E4j8x97g+1K1kjmm85SQxOEFP0ccV/TiJVtZ2muvDpMiypubUXTcZdN/Hk6 vQ570hXUuFVnwEmSHLwMjj9UoRrL3kKGlkVcqEUvhhOT2ok9Y2mDTVOnKKEQQvCbbpHD b9luoM6mpvHQG2q9UDVJ10SdF0+qSH5L0gjwTYjn8fi3DenuPM9N1G70ZZFBE3/zqrBq ZVaQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687298717; x=1689890717; 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=6QnZ16+cR3SKz8OZyy5d1CsXcXjkIRyncSh8yNxkG1M=; b=AOq5qKqIjk3C6hwHjm/B8N7yZWZqvXuMGYx5+ADayjiblRar49ynLR737NT7Hs9j69 xOBKrwrd4lNCQ/e6OlUvxdr44r6V69u62Oi/K7YDl68nypRjariffHADeBpS3gWz/VG0 IkckKrI0XVi7gbKrltRPrJ5ig6a/IjLw2Kv6IIA3mZsjy8ox2L+MchQLzdipqfmYbvfK N9Iq28rMFojWebh10q+uDJYMWYvCiBf3STB3ULBJeOMHjka20BsoMX8ApK8bG6M12a22 NgpL+2mjwaMeUfULd+eevp0JZZ39yEWm62xxkyAzrIiRLTG0dpXBZrevINkbD37ChBaC R9/Q== X-Gm-Message-State: AC+VfDxppf5JGydB/WyFy52jNo8Dvf/V5FR1vZYh2RaLuokpid65EB28 2puJ0kD0McrdJEjRVsov/JciCtXUlVS4VrWZde4NeQ== X-Google-Smtp-Source: ACHHUZ59qq5aWENB/BjU7unJQ0/yUrpV9cMPMC3H9gw6cRwjKcqMmpGu7hrOaqCTn7aGXRAjC3awXkHyEymz3yE+18M= X-Received: by 2002:a50:9e07:0:b0:50b:f6ce:2f3d with SMTP id z7-20020a509e07000000b0050bf6ce2f3dmr574426ede.0.1687298716727; Tue, 20 Jun 2023 15:05:16 -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> <444ed144-a2ee-cb16-880a-128383c83a08@collabora.com> In-Reply-To: <444ed144-a2ee-cb16-880a-128383c83a08@collabora.com> From: =?UTF-8?B?TWljaGHFgiBNaXJvc8WCYXc=?= Date: Wed, 21 Jun 2023 00:05:05 +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 X-Rspamd-Queue-Id: 567B81C0020 X-Rspam-User: X-Rspamd-Server: rspam02 X-Stat-Signature: o69yp1dyyx85gs5mocpgsw5dj7tex9wn X-HE-Tag: 1687298718-610941 X-HE-Meta: U2FsdGVkX1+h9+OBddpvh8oq/gUko+QqbhvJB6cJiO+BDsCXJbBFN7xp2MeRElihHszUGeuTYP6/3Zi7RqzzjPJf2lC9i6/HZG6kldK2k6s2j9SpwOf/AGntnWOkv55Dff3rApfIOJWaHuNH8qq2zWcnmyLQv8InuZotpWTj6kcuUBcTwpkZbiPcPDsjU2LFupds2DWLdp1sLm39+8y4hTEyu4CFTAGmCbXRYNaNWaAZ3TSSf7zaQW3fWjGsjFN5gzT8Zb8mzhVfLI3r2EmSfAnJWu6TUprjvgLSrRBIe4Ga8fh/SHstIc12/2+7io6sXxWfJUws16SXvaMmIFJRkEhEJuFVnsbrdR9lQpZA4kkBlLLOFTUhKrsz5kZX2WGrszOku6PqY5tgQ4Ne9qktGVRT0Bva5IsGz4YYAtuCnnJfSXKfw2iZMne1k8QHv7VGlkcbYU54S9kA1iw49RhKPzxjM3n0uJ7egQ8oF9mg9Q1q00kI3abt5VdB4ZSaATekZLyiOFstx416ygIvcZH2X9n0tG4IvlMoDANPmppY+5XNgy9kkVFLS423N3tHWQ+FL8m02Mz/RC4HUVBECVEDN42iaqXmi+MIu2Opsl9IgzrjL6+L4ZjuvdaG/yxI/V9/xWOLxQ/hZ5DV8RDj9S2UIl+Keeoq+SEMkgE0SUxwE0AUQxGUek4/ckRz+vG/bOgbWwtjehkGuqJHtU4m2wImrY0jbgFnRlAGG1SSikeG4Se1A1xLy8VG1b/8EoYfTT+PcswieNH9oSogMrogZPkgrblzbJp5FeG+E15wULquxmRONIUvZErcQk3YR+pISq4VUIDe4ZL1xZ39rvlwYJJt+7sNU3pniqFbSpIBjPCh3A+G912J8x+Lvc8dG98ANjdle1m6IZj1tX4GNl2imK0Psx8wZUObVQVohjQeOBTMbJdVOkRHbdqi1Q73bVHwal2Lg7+VjluBuIejVS7qQAX Jv2NvdXD QMjFosOfdwtTpnBt/4jtdAw3TZYKb2XTI4LBKW6K6o8tpnliuKcoDoj2kUPB40aI+5Mbr2X5ak6tUIS8yOIrADNPPBrV5JHb+gAc99I45OmhoeANjv5ZETAdhN6wFFf+2Y50nhFkRLuDYpLcqOzfhDbgbj2e0wmCqUNxKz2MFNinczhK1d2idl0mLC8oySY3bTUsNbgZ38apq1RZPfIRH1qXF70EpkD2Hmm0GwjLaxrQ972QP3f0+wMiDwbA4ltqAFFUy8tcxy1b8QH3BD6f6wzeH4h2k0iqjv+/5fs314DQ9lcBdM9KcUx8hEbX4QfrN+pIe7MMkXN3cQIEG1+FDwfz4b1JlwlpmKhmF+zMrYlqlyvBfTGKBWgtB5tEPpGGmStK/xVWp+D1yKr2mupHdohI1niOrg79UDqM7Kn+5vr05f2x+5mYPBgJe8xVFj6HrX2+y 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: On Tue, 20 Jun 2023 at 13:16, Muhammad Usama Anjum wrote: > On 6/19/23 1:16=E2=80=AFPM, Micha=C5=82 Miros=C5=82aw wrote: > > 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 nex= t version.) > > [...] > >>>>>>> I guess this will be reworked anyway, but I'd prefer this didn't = need > >>>>>>> custom errors etc. If we agree to decoupling the selection and GE= T > >>>>>>> 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 = special > >>>>>>> cases). > >>>>>> Right now, pagemap_scan_output() performs the work of both of thes= e 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= case. > >>>>>> 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_out= put > >>>>> uses max_pages properly to limit n_out). > >>>>> Isn't it that when the buffer is full we want to abort the scan alw= ays > >>>>> (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_outpu= t() we > >>>> check if we have space before putting data inside it. I'm using this= same > >>>> 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 returnin= g > >>> 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; > You are missing the optimization here that we check for full buffer every > time adding to user buffer. This was added to remove extra iteration of > page walk if buffer is full already. The way you are suggesting will remo= ve it. > > So you are returning remaining pages to be found now. This doesn't seem > right. If max_pages is 520, found_pages is 0 and n_pages is 512 before > calling pagemap_scan_output(). found_pages would become 512 after adding > 512 pages to output buffer. But n_pages would return 8 instead of 512. Yo= u > were saying we should return the number of pages added to the output buff= er. Ok, if we want this optimization, then i'd rework it so that we have: bool pagemap_scan_output(..., int *n_pages) { limit n_pages; ... return have_more_room_in_output; } The compiler should remove the pointer and memory storage for `n_pages` when inlining the function. Best Regards Micha=C5=82 Miros=C5=82aw