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 EE6DAC46467 for ; Tue, 10 Jan 2023 14:21:33 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 68A348E0005; Tue, 10 Jan 2023 09:21:33 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 63A688E0001; Tue, 10 Jan 2023 09:21:33 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 502618E0005; Tue, 10 Jan 2023 09:21:33 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 418568E0001 for ; Tue, 10 Jan 2023 09:21:33 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 0642A40B54 for ; Tue, 10 Jan 2023 14:21:33 +0000 (UTC) X-FDA: 80339102466.08.F379C0F Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf05.hostedemail.com (Postfix) with ESMTP id EC0EB10001C for ; Tue, 10 Jan 2023 14:21:30 +0000 (UTC) Authentication-Results: imf05.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=Tvi8Xfad; spf=pass (imf05.hostedemail.com: domain of bfoster@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1673360491; 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=I09tRp6zdZi/ub/P22Gc5q2mZ7cWTO2lX7+uoGwzZqg=; b=j1/NvSJ3ikLRB0xRBuJOaANHrwpO2cRWMrGH8bSxlPPWmjUx8qcqMN1JWX0O6CJ05Aqdhv 91aTjNGB0EoPMamJmRwXGvGeqmMfs7PH84T9UuXGPf4ed0rfFnfoXrk4Hj9yIOs0wKCjiU 0hiMitfHfy3jX/b4I8eyKmZXmCINovo= ARC-Authentication-Results: i=1; imf05.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=Tvi8Xfad; spf=pass (imf05.hostedemail.com: domain of bfoster@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1673360491; a=rsa-sha256; cv=none; b=Cq0hfMHfKatqO7sJoueC3uvnGKO8fI1tzzWsguoubgvT2fRhMgXrNHnRYP5iAiEHoLJ8Gc mY/ekv9E/q970upUFmSboJ9gqBijsCsgBfAv/clQZjyzqkLHesmwQLUSpfovOf7fKjMCAg hJWwe7R53dI9NKer2S1EtIv46yTUx6k= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1673360490; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=I09tRp6zdZi/ub/P22Gc5q2mZ7cWTO2lX7+uoGwzZqg=; b=Tvi8XfadsCtMPo6mFWPA7DRmfQfuN9sc0qBpEdTXMDrn/tsZrcaA8zD5mCSGx9LXXV7cBC CFZeMoEqvCcQZC+wDd3M+8SwOZPHP5gVNdwkIieANXbirkKVI/TEsiUhKT4/Lnju79lwvY B8hoJyPqbDIE2/d54Enr/dyBajp8rr0= Received: from mail-yb1-f198.google.com (mail-yb1-f198.google.com [209.85.219.198]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-148-NsuegvA6Oneoc3EuP_ZrYA-1; Tue, 10 Jan 2023 09:21:29 -0500 X-MC-Unique: NsuegvA6Oneoc3EuP_ZrYA-1 Received: by mail-yb1-f198.google.com with SMTP id w15-20020a05690204ef00b007b966ba4410so9717960ybs.5 for ; Tue, 10 Jan 2023 06:21:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=I09tRp6zdZi/ub/P22Gc5q2mZ7cWTO2lX7+uoGwzZqg=; b=6lWZp+xss9qU03JQlCHep4yQbP2evVVv5b0YmE82uBq0qh8iug+IoWNBosz9H+ux/g i5NNUsgQiIETGzYYOma6tEYJGoxqgesrS+6WcDMoMEF0DBOygUijgw3cT4k6PWhC8va3 DRtB7qi34RxK1nRHriI/W5LMSi8eSp84U02kI+NeMUNs+EdqC7Z0zL8zoiutj/W8NZRh WisE2tpQdVBRf3rHrAmyGeyQ+QWBfhg7aXAIBM6VtTHMllTOZ7FUgNWYf++WM5lTP0U8 +0wv3ADcqCVzytfj5NvUAY7y/JbouzZpGLWMrVOSwtYsSo1/pZQc8nWsS+6Vv/xh3Hs6 yEUw== X-Gm-Message-State: AFqh2krru4VwHciSyv4u2PXfHIHw4Zc9SdJ6NUWGjKbxrBSnMJl0teDv nz3//XGgI3sprRtw4jSOC0v0DHkumG+wcUWyYmj/iEr9PAnemUFxSRqw06q+WzXP+SPmHyaZo1H hSKfzEfLlJFM= X-Received: by 2002:a05:7500:5c92:b0:f0:401b:6d0e with SMTP id fh18-20020a0575005c9200b000f0401b6d0emr595493gab.22.1673360488504; Tue, 10 Jan 2023 06:21:28 -0800 (PST) X-Google-Smtp-Source: AMrXdXu0suqD6JUejg/bP+LLxlqm+8/NhlapFl+BQRsKWPzq2pn6o1g8ft2fkGiYEy6xXkAbSWzAKA== X-Received: by 2002:a05:7500:5c92:b0:f0:401b:6d0e with SMTP id fh18-20020a0575005c9200b000f0401b6d0emr595479gab.22.1673360488073; Tue, 10 Jan 2023 06:21:28 -0800 (PST) Received: from bfoster (c-24-61-119-116.hsd1.ma.comcast.net. [24.61.119.116]) by smtp.gmail.com with ESMTPSA id y22-20020a37f616000000b006fef61300fesm7143852qkj.16.2023.01.10.06.21.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Jan 2023 06:21:27 -0800 (PST) Date: Tue, 10 Jan 2023 09:22:28 -0500 From: Brian Foster To: Nhat Pham Cc: akpm@linux-foundation.org, hannes@cmpxchg.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, willy@infradead.org, kernel-team@meta.com Subject: Re: [PATCH v5 2/3] cachestat: implement cachestat syscall Message-ID: References: <20230104231127.2634648-1-nphamcs@gmail.com> <20230104231127.2634648-3-nphamcs@gmail.com> MIME-Version: 1.0 In-Reply-To: <20230104231127.2634648-3-nphamcs@gmail.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Stat-Signature: 995d6irkuchht4dmuw8ebgnxkf7a5eq4 X-Rspam-User: X-Rspamd-Queue-Id: EC0EB10001C X-Rspamd-Server: rspam06 X-HE-Tag: 1673360490-390419 X-HE-Meta: U2FsdGVkX1+mW3Uwby1qtUbwipRzBP0CkfGkHcTULp9Zn8jxC6KI9ZCqW37c4C5BB2/g5McNz5S6EtUyx6Z0f/xCJ3qX5jH0m5uj3DdAXvUovPRd4n9zTXpBoB1bAJwcHfjVll+B3ztY4fH/c4OBK3P/OwWcKXf+lP4hV+h2Hi6Bp6n58M0wkBH4ZlSQ+/3SpS3YZtlt/xwKDRAwOekYiMfd2BtndP+zlCPbn+seZIThMSRZiEqMRoqyE6hDXsbjbDGydFR4Tke/mt/24A2ur/V9t6nw9Kt0b6bZXz16wThBWHufRb9UpKj4CZQvM1wMkg0Br0SIHeyXmqH6STv3HUBKYpUYXt38Jc2Rpyi8gKvgMFRTUtlApATIKlVrxFIBAIscDXvwxOFS6IbjSCjtY/gNf2oL0yW0IYIaxvzkg/A89W7Zou3vDPEheOFniiClu6MeKaOd0fN4ZLOXSQtTF2xlObMfaPlWEaoqfllHPl65a5IKetc4cDcJKi/M3j6QwFlha0v/c259Hyw+bi0axD3QD0vljbVvRx939CleWwe1Je8xxz3ZVK0C/p+22Tb+TY41RIsSOAiUrB03CXAijPADhSxOuWljI+/yRmOdyM4hU0yTIOi5dIRABqlQ0ud44OoHw3+Yh9kU6WZBfl6BhF8vRJ3ucNrlGStZ7FCU1CE6XV4+sTOqdeW0XKJfm+WcOBMgVuAzTjyN0Mq3CT0PvfX4xi28a9sokFC6fVQ1PaX53rlpUes/hIZRztYLr0JjrRCFJtmJFOr15T10NGcTYu7WSmbEiWoSXZtcgeulU5L21GUTT1KbW/ydXRVCIiYCGSn7e42ZYnov2rcymZuwZtdwCJ/ZgkgO32pJNRdcD3avkg59cwnE5aiCq561hR8qozxEfrSBIyynueP5o7qzExROOaCB0lbm1+IY6miW3GMaIn+O1GGmQok2PFTJAdIqDLrfRShFr8HD1Cc016n kdPr4a+d lXk+cZBH8DUpM3YlQ5SR8XL1qpXULw6wdHZZWaCXwCw1Mq13xGQh/BuWMkr89gWtKq6xgwGxxIwliE4WjTlbMG7YgAu41nJ9C83qqQulrqzSX/0c= 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 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. 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? 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 >