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 705E5C54EBC for ; Tue, 10 Jan 2023 21:16:44 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 946318E0002; Tue, 10 Jan 2023 16:16:43 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 8CF638E0001; Tue, 10 Jan 2023 16:16:43 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 749BC8E0002; Tue, 10 Jan 2023 16:16:43 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id 5BB748E0001 for ; Tue, 10 Jan 2023 16:16:43 -0500 (EST) Received: from smtpin25.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 35466A2E1D for ; Tue, 10 Jan 2023 21:16:43 +0000 (UTC) X-FDA: 80340148686.25.8A8DA03 Received: from mail-wm1-f41.google.com (mail-wm1-f41.google.com [209.85.128.41]) by imf20.hostedemail.com (Postfix) with ESMTP id 9A7631C0016 for ; Tue, 10 Jan 2023 21:16:40 +0000 (UTC) Authentication-Results: imf20.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=J6bsJ+qd; spf=pass (imf20.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.128.41 as permitted sender) smtp.mailfrom=nphamcs@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1673385400; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=rm3QR70vsvtk0GdrQxO+T3mTyRnc34PXABTeiDAN3aU=; b=WMZhE8BuHOCMg59/Nkb75pOV92FBskdAO2ZoiPe6vUC/oxuLWfYtFPehQhUmaNJAMKlt6m xrsKvnFutmmUpRZSsUFQdRK8smcm9rMOd5+gc/49yBCaavOP9FgOYulkd0cey0IsfIXsti 9XgSvipRGkXVDOXPtdHej6DbUpWaVHA= ARC-Authentication-Results: i=1; imf20.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=J6bsJ+qd; spf=pass (imf20.hostedemail.com: domain of nphamcs@gmail.com designates 209.85.128.41 as permitted sender) smtp.mailfrom=nphamcs@gmail.com; dmarc=pass (policy=none) header.from=gmail.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1673385400; a=rsa-sha256; cv=none; b=M2k5vKVCTAztXkUrSYF73DMQuB18aTpD98rZHBP8ljH1qzKJ+ZbRcLpEWAAByLxg3JA45A gc/ZU6dFz1YHgsdLka8nMPoZtHHAQaJ1kYJ6ngXOwlJJZOX+1KoRWfEEdoJwWWqopBCJNe PY2LIzrNHSef8feMGnOwAjmCvl0WHQQ= Received: by mail-wm1-f41.google.com with SMTP id k26-20020a05600c1c9a00b003d972646a7dso12907983wms.5 for ; Tue, 10 Jan 2023 13:16:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=rm3QR70vsvtk0GdrQxO+T3mTyRnc34PXABTeiDAN3aU=; b=J6bsJ+qdaNDxxbNulA8aTN9nXH3IutM50tiJ5BjyYjYfGVJb5R+O19fG0rRUReO6GR mLjpF2Kf9ICtutODhBvNHZxSrXfjfBtK/OtLFDr4jNkF7Js5VDHb1C0svKOgiTnu5oyL 1y9qXWMQ1O0C9YmFzGhsloo6j61NYsU9y5h190SIqCK+ULiConv3u4L5NNGZb8BUayPv ktWG/fet59yas18GqJET0PmLvOV9Z0sloEiOptIMMjz32S7Fu53+XAgoLDgOlf8hDvoH tbGRPUGuW5P3GV7pnnk6jglZI/X4HiE9SW8MbVfQB5z95+UWcGGZ9K2UTVPdyd/jDDhg 7g3A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=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=rm3QR70vsvtk0GdrQxO+T3mTyRnc34PXABTeiDAN3aU=; b=fQd/8s/qNmH2K9JfEhDa/H7ftqeiC70PVHtsq+5fyx9m1AnsTSEC69d+vmBQEif2fi Yx9248BvYiv9wsudublKn2XdiZxCwINoxi77pZZX0MEmp8aZkhlaIzcdx4meiwUSiXB4 g75R74HR6l1yABb5NFS2HFK8y1y0agaw6rZtSAni16Udy7DdAiPqlujFzrRpgdTHkpYj QX++VIIxFjfa1rJ2HSJjYbWjOiq8a9Uz2ycQdQ5k7r+m8u6Ypu1qub35r8HsLmTLNFcn EZpNFltS9fqdHCa0xdygIfMjld8aVDZVyHrOaiKrzg6xCYVgEDjKTANeLMc1qpTYShDX xhvg== X-Gm-Message-State: AFqh2kr1BHE8nA1cKeiHW+FIOyFlqXzcx1BXJbGFXvqRsz1Wg2RoFRvt e2t1y9r1bn4/m0ysJD0x+jMsW6sb2OB4sxdyk7o= X-Google-Smtp-Source: AMrXdXuPNpcSOQohf7grr4ojrQziXGy7FM8/oN0RxQE7DoM80GPa1c3u6edBIVq+XJWnAd7jnyKuiaTSWBDRQdd99jQ= X-Received: by 2002:a05:600c:3d99:b0:3d0:8c4b:b76f with SMTP id bi25-20020a05600c3d9900b003d08c4bb76fmr3038411wmb.131.1673385398899; Tue, 10 Jan 2023 13:16:38 -0800 (PST) MIME-Version: 1.0 References: <20230104231127.2634648-1-nphamcs@gmail.com> <20230104231127.2634648-3-nphamcs@gmail.com> In-Reply-To: From: Nhat Pham Date: Tue, 10 Jan 2023 13:16:27 -0800 Message-ID: Subject: Re: [PATCH v5 2/3] cachestat: implement cachestat syscall To: Brian Foster Cc: akpm@linux-foundation.org, hannes@cmpxchg.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, willy@infradead.org, kernel-team@meta.com Content-Type: text/plain; charset="UTF-8" X-Rspamd-Queue-Id: 9A7631C0016 X-Stat-Signature: b9cwx8fgdunegrosr63tzuwrb77cz619 X-Rspam-User: X-Rspamd-Server: rspam08 X-HE-Tag: 1673385400-31284 X-HE-Meta: U2FsdGVkX19InPFoJ6f0BZ86dGZrMnxxTwoTylUOWsexYy/MB0PDlBinM+tKRm3euf7wlzxuGJ49bIXgcHqTiI8FhEQQ1BzNkv+mKT4sGOyiJItYWn894hXFK1MOfNYNAJJG4Fk9OskbONA+DaqtYgMIXHZ2ign5CZNILcTZfiGkS8JKHOIyZG8Q9ftgUJabiWlwWBUaM+h3W1qJ6NRfA8iYNwSkCVoxiKzJthZs8jvs1NV82ofQ0zt5Ju2DP0LhBuk15seoebrXy4PXl+ZA3CNNQnOBGXRLczovMRy84ttf0bf8glQyXTO8qdTsCFWM6DhUkEtFpnqCUYFDl5lwMSB78OPlru/lOsRIovDnZINVQiNWUDY0SIqNhOAxtliZNOYS5g6WZ+K/KylF4usdy1eF/H24XfTyDU+GDZjCg7GXlNnUCQ0kHpqRH0XpHBYSiE6h7pY+GDuL2lzJF2BZSjzu+BERg5+QA0LghEzmnTb7Zn6N/Ndf8grxpUQgrM51B1OOpzZSd6s3XUrwBuME48py9H2jBKv3YPQWzOo4DXZRI7LgYP+pX7lt+olUrlQR4eartcaPfZyF5mQuZQh9sEHGuOHX0UQSiZ22873qVXJ+rBhX9Cs2f4cypFzb450dvbcy06NH2jwatsrwhSzq8pIB++MnZjOEPulTLo3DH0XD9Y7iiqtmlkfB1GDlTod3/ZzNUwTHY2bf0VF4g+wpAXCN/2w4zFR3SywDXdlBRvfcobcRvBNSl/2BvfaeYIa/H7eb+xfVPpPG2NVGDUpqhGseQnDWYxWyoXRjgEyhgVhsCoo48H832m3Xju1rbqkx2mIK43Y7JKVWI0KjRQUOFji/+y5vca14Lo0QPcRnXof6EOoyEGKSyhkdpCOe+hLI3qTlT8SclPAywsINimBqJm1tHqsXbVb9LuZBBwNuVzviDcr4vTzSvLVPjH/62OazSDBgfLvagCHmMaRuFY2 LAA2aPOA arHvcYlMumk49IKlzeq5trvxXbVFBlh3loRyeipjdND2pTdttBKPkof3Enw== 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, Jan 10, 2023 at 6:21 AM Brian Foster wrote: > > On Wed, Jan 04, 2023 at 03:11:26PM -0800, Nhat Pham wrote: > > Implement a new syscall that queries cache state of a file and > > summarizes the number of cached pages, number of dirty pages, number of > > pages marked for writeback, number of (recently) evicted pages, etc. in > > a given range. > > > > NAME > > cachestat - query the page cache statistics of a file. > > > > SYNOPSIS > > #include > > > > struct cachestat { > > __u64 nr_cache; > > __u64 nr_dirty; > > __u64 nr_writeback; > > __u64 nr_evicted; > > __u64 nr_recently_evicted; > > }; > > > > int cachestat(unsigned int fd, off_t off, size_t len, > > size_t cstat_size, struct cachestat *cstat, > > unsigned int flags); > > > > DESCRIPTION > > cachestat() queries the number of cached pages, number of dirty > > pages, number of pages marked for writeback, number of evicted > > pages, number of recently evicted pages, in the bytes range given by > > `off` and `len`. > > > > An evicted page is a page that is previously in the page cache but > > has been evicted since. A page is recently evicted if its last > > eviction was recent enough that its reentry to the cache would > > indicate that it is actively being used by the system, and that > > there is memory pressure on the system. > > > > These values are returned in a cachestat struct, whose address is > > given by the `cstat` argument. > > > > The `off` and `len` arguments must be non-negative integers. If > > `len` > 0, the queried range is [`off`, `off` + `len`]. If `len` == > > 0, we will query in the range from `off` to the end of the file. > > > > `cstat_size` allows users to obtain partial results. The syscall > > will copy the first `csstat_size` bytes to the specified userspace > > memory. `cstat_size` must be a non-negative value that is no larger > > than the current size of the cachestat struct. > > > > The `flags` argument is unused for now, but is included for future > > extensibility. User should pass 0 (i.e no flag specified). > > > > RETURN VALUE > > On success, cachestat returns 0. On error, -1 is returned, and errno > > is set to indicate the error. > > > > ERRORS > > EFAULT cstat points to an invalid address. > > > > EINVAL invalid `cstat_size` or `flags` > > > > EBADF invalid file descriptor. > > > > Signed-off-by: Nhat Pham > > --- > > arch/alpha/kernel/syscalls/syscall.tbl | 1 + > > arch/arm/tools/syscall.tbl | 1 + > > arch/ia64/kernel/syscalls/syscall.tbl | 1 + > > arch/m68k/kernel/syscalls/syscall.tbl | 1 + > > arch/microblaze/kernel/syscalls/syscall.tbl | 1 + > > arch/parisc/kernel/syscalls/syscall.tbl | 1 + > > arch/powerpc/kernel/syscalls/syscall.tbl | 1 + > > arch/s390/kernel/syscalls/syscall.tbl | 1 + > > arch/sh/kernel/syscalls/syscall.tbl | 1 + > > arch/sparc/kernel/syscalls/syscall.tbl | 1 + > > arch/x86/entry/syscalls/syscall_32.tbl | 1 + > > arch/x86/entry/syscalls/syscall_64.tbl | 1 + > > arch/xtensa/kernel/syscalls/syscall.tbl | 1 + > > include/linux/fs.h | 3 + > > include/linux/syscalls.h | 3 + > > include/uapi/asm-generic/unistd.h | 5 +- > > include/uapi/linux/mman.h | 9 ++ > > init/Kconfig | 10 ++ > > kernel/sys_ni.c | 1 + > > mm/filemap.c | 143 ++++++++++++++++++++ > > 20 files changed, 186 insertions(+), 1 deletion(-) > > > ... > > diff --git a/mm/filemap.c b/mm/filemap.c > > index 08341616ae7a..d70d47b20700 100644 > > --- a/mm/filemap.c > > +++ b/mm/filemap.c > ... > > @@ -3949,3 +3953,142 @@ bool filemap_release_folio(struct folio *folio, gfp_t gfp) > > return try_to_free_buffers(folio); > > } > > EXPORT_SYMBOL(filemap_release_folio); > ... > > +#ifdef CONFIG_CACHESTAT_SYSCALL > > +/* > > + * The cachestat(5) system call. > > + * > > + * cachestat() returns the page cache statistics of a file in the > > + * bytes range specified by `off` and `len`: number of cached pages, > > + * number of dirty pages, number of pages marked for writeback, > > + * number of evicted pages, and number of recently evicted pages. > > + * > > + * An evicted page is a page that is previously in the page cache > > + * but has been evicted since. A page is recently evicted if its last > > + * eviction was recent enough that its reentry to the cache would > > + * indicate that it is actively being used by the system, and that > > + * there is memory pressure on the system. > > + * > > + * `off` and `len` must be non-negative integers. If `len` > 0, > > + * the queried range is [`off`, `off` + `len`]. If `len` == 0, > > + * we will query in the range from `off` to the end of the file. > > + * > > + * `cstat_size` allows users to obtain partial results. The syscall > > + * will copy the first `csstat_size` bytes to the specified userspace > > + * memory. It also makes the cachestat struct extensible - new fields > > + * can be added in the future without breaking existing usage. > > + * `cstat_size` must be a non-negative value that is no larger than > > + * the current size of the cachestat struct. > > + * > > + * The `flags` argument is unused for now, but is included for future > > + * extensibility. User should pass 0 (i.e no flag specified). > > + * > > + * Because the status of a page can change after cachestat() checks it > > + * but before it returns to the application, the returned values may > > + * contain stale information. > > + * > > + * return values: > > + * zero - success > > + * -EFAULT - cstat points to an illegal address > > + * -EINVAL - invalid arguments > > + * -EBADF - invalid file descriptor > > + */ > > +SYSCALL_DEFINE6(cachestat, unsigned int, fd, off_t, off, size_t, len, > > + size_t, cstat_size, struct cachestat __user *, cstat, > > + unsigned int, flags) > > +{ > > + struct fd f = fdget(fd); > > + struct address_space *mapping; > > + struct cachestat cs; > > + pgoff_t first_index = off >> PAGE_SHIFT; > > + pgoff_t last_index = > > + len == 0 ? ULONG_MAX : (off + len - 1) >> PAGE_SHIFT; > > + > > + if (off < 0 || cstat_size > sizeof(struct cachestat) || flags != 0) > > + return -EINVAL; > > + > > + if (!f.file) > > + return -EBADF; > > + > > It looks like we miss an fdput() before returning via the above error > checks. Ooops yeah I missed that. I'll fix it. > > The only other thing that stands out as a bit odd to me is the > cstat_size check and associated ability to return a partial cachestat > struct. Do other syscalls do anything like that? I'd think we'd want to > ensure we always at least return a fully populated cachestat struct, > even if it happened to be an old/compat version if the size does ever > increase. Hm? Not that I know of, but the idea is that the user might expect a smaller struct cachestat in their code (and allocate memory accordingly). With this cstat_size, we can make sure that the expansion of cachestat struct (with new fields) does not break existing user's code - we only copy part of the struct. > > Brian > > > + memset(&cs, 0, sizeof(struct cachestat)); > > + mapping = f.file->f_mapping; > > + filemap_cachestat(mapping, first_index, last_index, &cs); > > + fdput(f); > > + > > + if (copy_to_user(cstat, &cs, cstat_size)) > > + return -EFAULT; > > + > > + return 0; > > +} > > +#endif /* CONFIG_CACHESTAT_SYSCALL */ > > -- > > 2.30.2 > > >