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 CFA0AC4332F for ; Tue, 20 Dec 2022 14:38:00 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 66BCC8E0007; Tue, 20 Dec 2022 09:38:00 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 61DA88E0006; Tue, 20 Dec 2022 09:38:00 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 4BC9C8E0007; Tue, 20 Dec 2022 09:38:00 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 3A4898E0006 for ; Tue, 20 Dec 2022 09:38:00 -0500 (EST) Received: from smtpin29.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id EFA87120CCE for ; Tue, 20 Dec 2022 14:37:59 +0000 (UTC) X-FDA: 80262939078.29.C589423 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf11.hostedemail.com (Postfix) with ESMTP id 7A6D440003 for ; Tue, 20 Dec 2022 14:37:57 +0000 (UTC) Authentication-Results: imf11.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=W2qgXrT5; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf11.hostedemail.com: domain of bfoster@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1671547077; 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=j/FjbEpoRdR1NHbN3/m88Q4muAb1AJ3S0g9UVwS9euM=; b=S6R2tOC2Ec/Mz570Nulv850kqg7p7rBPIQ1RyanIytr+Pg9DqtiSLRcuU7fxFqyExrMETT LVlUeFL63bAiUqBOmpWSho7OnwzGs8EBfvoFQlv61/E1GTmnEjyhXfYL2v88biCFwGhHCr lM19I5XB72stjHPNJ3N3wXUJspp41uQ= ARC-Authentication-Results: i=1; imf11.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=W2qgXrT5; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf11.hostedemail.com: domain of bfoster@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=bfoster@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1671547077; a=rsa-sha256; cv=none; b=sDzcNlvRjsUuEq4ta/IvVvX4SPVKnQ9XHdeEIWufjFpgnI7+/ErY8BxUnxMCouIM/UPiOf 9FIdoDAqaOf6rl8DxQgMPBdA41krePWIiUSQ/tiOcXnYN6Lw7N70v+xIEHFCmkXbot3HEv Q4aWkCOkhGtsj9UQlM4U2p+phOvqNf4= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1671547076; 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=j/FjbEpoRdR1NHbN3/m88Q4muAb1AJ3S0g9UVwS9euM=; b=W2qgXrT5XZM2mX5VYskoTEvw33HfmNE42G2oUfOF3jlXq78vhuTqN4t8QfGl3YDJnMM8s+ 8QZEMLOKNTuIZvp285awWKbH7qQKzMCfJadZftjJTHjw6koh7GqIqDgOXZtoMKmnZNVqcL DfqBisvZcZbIkfpJx/nHAowirkD50/E= Received: from mail-qv1-f71.google.com (mail-qv1-f71.google.com [209.85.219.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_128_GCM_SHA256) id us-mta-453-5g9tqHG3P9qm662eh7CZ_A-1; Tue, 20 Dec 2022 09:37:53 -0500 X-MC-Unique: 5g9tqHG3P9qm662eh7CZ_A-1 Received: by mail-qv1-f71.google.com with SMTP id w1-20020a056214012100b004c6ecf32001so7121744qvs.8 for ; Tue, 20 Dec 2022 06:37:53 -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=j/FjbEpoRdR1NHbN3/m88Q4muAb1AJ3S0g9UVwS9euM=; b=ERVHK20fhWLpiGkr8YCP7nQYex+emmToqceRHfxgit9rHryvmkrjtpFmAYQ+IH9nSJ 2JnjXpF2qqQM4d1E5VOr2v2F8cNDI04Q4M/2xNgar7CnfnadXg2ykkJm6rHfJzuQCR52 ddI4IJ6bAO/HVZ9zPH5MBStgds9Xp4NuyouCmDhyEIbXdFNteNye8KMXqOoHM6sRDUM1 IbBaKyet3MLWaKbWrBklk181Gz4Wb2zwKbtKl9D3Ovv7wQZCubW9Es8A36++sq09uCRv D2YszFkUVKZBH7YtIc70aY8m2l66q3troQQ+RuokQnCx2R71j3noPfIRCWIqcmKdiz3j Yxtg== X-Gm-Message-State: ANoB5pldF37aMLtiWmE8sRGUdoK2LvgxpwtnQouQrkHspdZOzCVr4DT7 9TL3JZCbmyUTpn5RIF1L617O7HFURiKUDjM2VfItYbZWNHL9P8xRwfq7oJXNXpyjR9J/rmExGwb PtIjdNfJ64AA= X-Received: by 2002:ac8:6786:0:b0:3a8:1a6d:3db3 with SMTP id b6-20020ac86786000000b003a81a6d3db3mr40391446qtp.62.1671547072770; Tue, 20 Dec 2022 06:37:52 -0800 (PST) X-Google-Smtp-Source: AA0mqf5+a7lOWncpxetwm3ZN/RkP79KblTgc/MGgVtmr6Ks5QqdoyhMfQdfI5E19yqSSxH7CJPq0cw== X-Received: by 2002:ac8:6786:0:b0:3a8:1a6d:3db3 with SMTP id b6-20020ac86786000000b003a81a6d3db3mr40391423qtp.62.1671547072443; Tue, 20 Dec 2022 06:37:52 -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 i6-20020a05620a248600b006fc2f74ad12sm9095307qkn.92.2022.12.20.06.37.51 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 20 Dec 2022 06:37:51 -0800 (PST) Date: Tue, 20 Dec 2022 09:37:58 -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 v4 3/4] cachestat: implement cachestat syscall Message-ID: References: <20221216192149.3902877-1-nphamcs@gmail.com> <20221216192149.3902877-4-nphamcs@gmail.com> MIME-Version: 1.0 In-Reply-To: <20221216192149.3902877-4-nphamcs@gmail.com> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline X-Rspamd-Queue-Id: 7A6D440003 X-Rspamd-Server: rspam09 X-Rspam-User: X-Stat-Signature: cx57mnz4mx5osc1jrj6puko9ftx1anfs X-HE-Tag: 1671547077-944796 X-HE-Meta: U2FsdGVkX181mglskgQiPLnfoCTDSEmEstfG7FgXKBETRA9FnsFxAvY0zz+388dnBRpbKVAIMBdgpyeSUqCBMPKF7Dhmq5tfvGTVCpXpM/O92Lb/TO8G4Nz1DLYHKR024oIy5oE+jFsfLhdFRBTlt0zJJq5lU6vnKqdQFut5GI0KfqZRLw6Fxzm0pyfNZTz7nd7hP1TZV5Cnuyy8ZmWQGhNWid53ltyC3eb4hllU6tSySeJi8cV3TvGU0Z0SMr1Vmh+5ahdoC/xrRD3vW9tJG5In91RfD5aIrg26IYJKUMyER4ojwWOBBthpEoIlP0rdxc4NS8YKV9QNJZL3THV4JloeI7TUvYiZ+Z19pq8F/8bzdNtFPWt2FdNiqHdzrkHKSmWlNKsPvR+khRSBeiRAh7sGYryhDTskQRV2fFswfK2U3lpT56jL/NxmenZp+LMQAzRKDSuJyMzaQRvmOYXkLcKbeYtmi3Y4K9iTJvWADcds/Lm1+gNXf6YIcwzSxzsDBJs7xtyspG7o61udL+DcStaQvb9AfgqqS9y+sc0+LyWEhy+lk4M+wP5vt4OfKg36LEr+IRK5PTqsRnZj8Hnncxh20AYnfXR6TvX3qQa+To+5nuqFv14a8eOxsLF8CVyO8nNd3OPCOSwmCnGs8+uR+rNDvbLognhGUypR5qXI26i5wohBzYYS+tZWd89Dm4eGjyR2NPfSTW3IDuyhiwAKo9ExCOGSGK5qzqRqxRa0KvX/RMeVmbMIxPJmkulnHa4ENS06fJH7Z2ZNKTEoCn7htHp4hnfwdgBvEO/3UhAQoxWnKT/XN8s5zDwb622FpNUUiazBp26SzTEGBc6K1HuL8zdIUs5KDdV2f3Gk4Qh/0s8Xsc+JlyHmu9DcbVgWFs8slnTbADxcwP/dUf2JXk+nCjCHOlYekD4bd2tELUD2ya3w+63juy+hxCUtIwsRz+1stiClZUA/Iv+g2Zmpxzs gRBfOks8 +vH9J9x+jyk3KVqLDZCvKV2aAhDEi1kBkhoxIvb9aR/WygHJK+v9ywombkIty1g0qV1qSEpn9Ppj31T47rbLWVT27ISRELb1PfGhVfGiuokyepUs= 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 Fri, Dec 16, 2022 at 11:21:48AM -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 status 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 (recently) > evicted pages, in the bytes range given by `off` and `len`. > > 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 > --- Hi Nhat, Thanks for the tweaks. FWIW, this by and large looks reasonable to me. Just a couple more random nitty things, if you happen to care about them.. > 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 | 137 ++++++++++++++++++++ > 20 files changed, 180 insertions(+), 1 deletion(-) > ... > diff --git a/mm/filemap.c b/mm/filemap.c > index 08341616ae7a..29ffb906caa4 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c ... > @@ -3949,3 +3953,136 @@ bool filemap_release_folio(struct folio *folio, gfp_t gfp) > return try_to_free_buffers(folio); > } > EXPORT_SYMBOL(filemap_release_folio); > + > +#ifdef CONFIG_CACHESTAT > +/** > + * filemap_cachestat() - compute the page cache statistics of a mapping > + * @mapping: The mapping to compute the statistics for. > + * @first_index: The starting page cache index. > + * @last_index: The final page index (inclusive). > + * @cs: the cachestat struct to write the result to. > + * > + * This will query the page cache statistics of a mapping in the > + * page range of [first_index, last_index] (inclusive). THe statistics > + * queried include: number of dirty pages, number of pages marked for > + * writeback, and the number of (recently) evicted pages. > + */ > +void filemap_cachestat(struct address_space *mapping, pgoff_t first_index, > + pgoff_t last_index, struct cachestat *cs) > +{ Not sure why the internal helper needs to be wrapped in a config option? Perhaps it makes more sense to define CONFIG_CACHESTAT_SYSCALL and just wrap the syscall bits with that..? I would also think it might make things simpler to split out all the syscall bits into a separate patch from filemap_cachestat(). > + XA_STATE(xas, &mapping->i_pages, first_index); > + struct folio *folio; > + > + rcu_read_lock(); > + xas_for_each(&xas, folio, last_index) { > + unsigned long nr_pages; > + pgoff_t folio_first_index, folio_last_index; > + > + if (xas_retry(&xas, folio)) > + continue; > + > + nr_pages = folio_nr_pages(folio); > + folio_first_index = folio_pgoff(folio); > + folio_last_index = folio_first_index + nr_pages - 1; > + > + /* Folios might straddle the range boundaries, only count covered subpages */ > + if (folio_first_index < first_index) > + nr_pages -= first_index - folio_first_index; > + > + if (folio_last_index > last_index) > + nr_pages -= folio_last_index - last_index; > + > + if (xa_is_value(folio)) { > + /* page is evicted */ > + void *shadow = (void *)folio; > + bool workingset; /* not used */ > + > + cs->nr_evicted += nr_pages; > + > +#ifdef CONFIG_SWAP /* implies CONFIG_MMU */ > + if (shmem_mapping(mapping)) { > + /* shmem file - in swap cache */ > + swp_entry_t swp = radix_to_swp_entry(folio); > + > + shadow = get_shadow_from_swap_cache(swp); > + } > +#endif > + if (workingset_test_recent(shadow, true, &workingset)) > + cs->nr_recently_evicted += nr_pages; > + > + continue; > + } > + > + /* page is in cache */ > + cs->nr_cache += nr_pages; > + > + if (folio_test_dirty(folio)) > + cs->nr_dirty += nr_pages; > + > + if (folio_test_writeback(folio)) > + cs->nr_writeback += nr_pages; I'm not sure this is an issue right now (or if it will ever be for your use cases), but from a filesystem perspective it is possible to have large or variable sized folios in cache. At the moment I think the fs (or XFS+iomap at least) manages dirty/writeback state at folio granularity, but that may change in the near future if/when iomap sub-page dirty tracking comes along. I suspect that means it may become possible to have a large folio of some N number of pages where only a subset of those pages are actually in dirty/writeback state, and thus introduces some inaccuracy here because this assumes that folio state applies to folio_nr_pages() worth of pages. Just something to be aware of.. Brian > + } > + rcu_read_unlock(); > +} > +EXPORT_SYMBOL(filemap_cachestat); > + > +/* > + * The cachestat(5) system call. > + * > + * cachestat() returns the page cache statistics of a file in the > + * bytes specified by `off` and `len`: number of cached pages, > + * number of dirty pages, number of pages marked for writeback, > + * number of (recently) evicted pages. > + * > + * `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; > + > + 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 */ > -- > 2.30.2 >