by John Groves

[permalink] [raw]

Subject: [RFC PATCH 16/20] famfs: Add fault counters

One of the key requirements for famfs is that it service vma faults
efficiently. Our metadata helps - the search order is n for n extents,
and n is usually 1. But we can still observe gnarly lock contention
in mm if PTE faults are happening. This commit introduces fault counters
that can be enabled and read via /sys/fs/famfs/...

These counters have proved useful in troubleshooting situations where
PTE faults were happening instead of PMD. No performance impact when
disabled.

Signed-off-by: John Groves <[email protected]>
---
fs/famfs/famfs_file.c | 97 +++++++++++++++++++++++++++++++++++++++
fs/famfs/famfs_internal.h | 73 +++++++++++++++++++++++++++++
2 files changed, 170 insertions(+)

diff --git a/fs/famfs/famfs_file.c b/fs/famfs/famfs_file.c
index fd42d5966982..a626f8a89790 100644
--- a/fs/famfs/famfs_file.c
+++ b/fs/famfs/famfs_file.c
@@ -19,6 +19,100 @@
#include <uapi/linux/famfs_ioctl.h>
#include "famfs_internal.h"

+/***************************************************************************************
+ * filemap_fault counters
+ *
+ * The counters and the fault_count_enable file live at
+ * /sys/fs/famfs/
+ */
+struct famfs_fault_counters ffc;
+static int fault_count_enable;
+
+static ssize_t
+fault_count_enable_show(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%d\n", fault_count_enable);
+}
+
+static ssize_t
+fault_count_enable_store(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ int value;
+ int rc;
+
+ rc = sscanf(buf, "%d", &value);
+ if (rc != 1)
+ return 0;
+
+ if (value > 0) /* clear fault counters when enabling, but not when disabling */
+ famfs_clear_fault_counters(&ffc);
+
+ fault_count_enable = value;
+ return count;
+}
+
+/* Individual fault counters are read-only */
+static ssize_t
+fault_count_pte_show(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%llu", famfs_pte_fault_ct(&ffc));
+}
+
+static ssize_t
+fault_count_pmd_show(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%llu", famfs_pmd_fault_ct(&ffc));
+}
+
+static ssize_t
+fault_count_pud_show(struct kobject *kobj,
+ struct kobj_attribute *attr,
+ char *buf)
+{
+ return sprintf(buf, "%llu", famfs_pud_fault_ct(&ffc));
+}
+
+static struct kobj_attribute fault_count_enable_attribute = __ATTR(fault_count_enable,
+ 0660,
+ fault_count_enable_show,
+ fault_count_enable_store);
+static struct kobj_attribute fault_count_pte_attribute = __ATTR(pte_fault_ct,
+ 0440,
+ fault_count_pte_show,
+ NULL);
+static struct kobj_attribute fault_count_pmd_attribute = __ATTR(pmd_fault_ct,
+ 0440,
+ fault_count_pmd_show,
+ NULL);
+static struct kobj_attribute fault_count_pud_attribute = __ATTR(pud_fault_ct,
+ 0440,
+ fault_count_pud_show,
+ NULL);
+
+
+static struct attribute *attrs[] = {
+ &fault_count_enable_attribute.attr,
+ &fault_count_pte_attribute.attr,
+ &fault_count_pmd_attribute.attr,
+ &fault_count_pud_attribute.attr,
+ NULL,
+};
+
+struct attribute_group famfs_attr_group = {
+ .attrs = attrs,
+};
+
+/* End fault counters */
+
/**
* famfs_map_meta_alloc() - Allocate famfs file metadata
* @mapp: Pointer to an mcache_map_meta pointer
@@ -525,6 +619,9 @@ __famfs_filemap_fault(
if (IS_DAX(inode)) {
pfn_t pfn;

+ if (fault_count_enable)
+ famfs_inc_fault_counter_by_order(&ffc, pe_size);
+
ret = dax_iomap_fault(vmf, pe_size, &pfn, NULL, &famfs_iomap_ops);
if (ret & VM_FAULT_NEEDDSYNC)
ret = dax_finish_sync_fault(vmf, pe_size, pfn);
diff --git a/fs/famfs/famfs_internal.h b/fs/famfs/famfs_internal.h
index af3990d43305..987cb172a149 100644
--- a/fs/famfs/famfs_internal.h
+++ b/fs/famfs/famfs_internal.h
@@ -50,4 +50,77 @@ struct famfs_fs_info {
char *rootdev;
};

+/*
+ * filemap_fault counters
+ */
+extern struct attribute_group famfs_attr_group;
+
+enum famfs_fault {
+ FAMFS_PTE = 0,
+ FAMFS_PMD,
+ FAMFS_PUD,
+ FAMFS_NUM_FAULT_TYPES,
+};
+
+static inline int valid_fault_type(int type)
+{
+ if (unlikely(type < 0 || type > FAMFS_PUD))
+ return 0;
+ return 1;
+}
+
+struct famfs_fault_counters {
+ atomic64_t fault_ct[FAMFS_NUM_FAULT_TYPES];
+};
+
+extern struct famfs_fault_counters ffc;
+
+static inline void famfs_clear_fault_counters(struct famfs_fault_counters *fc)
+{
+ int i;
+
+ for (i = 0; i < FAMFS_NUM_FAULT_TYPES; i++)
+ atomic64_set(&fc->fault_ct[i], 0);
+}
+
+static inline void famfs_inc_fault_counter(struct famfs_fault_counters *fc,
+ enum famfs_fault type)
+{
+ if (valid_fault_type(type))
+ atomic64_inc(&fc->fault_ct[type]);
+}
+
+static inline void famfs_inc_fault_counter_by_order(struct famfs_fault_counters *fc, int order)
+{
+ int pgf = -1;
+
+ switch (order) {
+ case 0:
+ pgf = FAMFS_PTE;
+ break;
+ case PMD_ORDER:
+ pgf = FAMFS_PMD;
+ break;
+ case PUD_ORDER:
+ pgf = FAMFS_PUD;
+ break;
+ }
+ famfs_inc_fault_counter(fc, pgf);
+}
+
+static inline u64 famfs_pte_fault_ct(struct famfs_fault_counters *fc)
+{
+ return atomic64_read(&fc->fault_ct[FAMFS_PTE]);
+}
+
+static inline u64 famfs_pmd_fault_ct(struct famfs_fault_counters *fc)
+{
+ return atomic64_read(&fc->fault_ct[FAMFS_PMD]);
+}
+
+static inline u64 famfs_pud_fault_ct(struct famfs_fault_counters *fc)
+{
+ return atomic64_read(&fc->fault_ct[FAMFS_PUD]);
+}
+
#endif /* FAMFS_INTERNAL_H */
--
2.43.0

2024-03-11 01:29:42

by Dave Chinner

[permalink] [raw]

Subject: Re: [RFC PATCH 00/20] Introduce the famfs shared-memory file system

On Thu, Feb 29, 2024 at 08:52:48AM -0600, John Groves wrote:
> On 24/02/29 01:15PM, Dave Chinner wrote:
> > On Mon, Feb 26, 2024 at 08:05:58PM -0600, John Groves wrote:
> > > bw ( MiB/s): min= 5085, max=27367, per=100.00%, avg=14361.95, stdev=165.61, samples=719
> > > iops : min= 2516, max=13670, avg=7160.17, stdev=82.88, samples=719
> > > lat (usec) : 4=0.05%, 10=0.72%, 20=2.23%, 50=2.48%, 100=3.02%
> > > lat (usec) : 250=1.54%, 500=2.37%, 750=1.34%, 1000=0.75%
> > > lat (msec) : 2=3.20%, 4=43.10%, 10=23.05%, 20=14.81%, 50=1.25%
> >
> > Most of the IO latencies are up round the 4-20ms marks. That seems
> > kinda high for a 2MB IO. With a memcpy speed of 10GB/s, the 2MB
> > should only take a couple of hundred microseconds. For Famfs, the
> > latencies appear to be around 1-4ms.
> >
> > So where's all that extra time coming from?
>
> Below, you will see two runs with performance and latency distribution
> about the same as famfs (the answer for that was --fallocate=native).

Ah, that is exactly what I suspected, and was wanting profiles
because that will show up in them clearly.

> > > lat (msec) : 100=0.08%
> > > cpu : usr=10.18%, sys=0.79%, ctx=67227, majf=0, minf=38511
> >
> > And why is system time reporting at almost zero instead of almost
> > all the remaining cpu time (i.e. up at 80-90%)?
>
> Something weird is going on with the cpu reporting. Sometimes sys=~0, but other times
> it's about what you would expect. I suspect some sort of measurement error,
> like maybe the method doesn't work with my cpu model? (I'm grasping, but with
> a somewhat rational basis...)
>
> I pasted two xfs runs below. The first has the wonky cpu sys value, and
> the second looks about like what one would expect.
>
> >
> > Can you run call-graph kernel profiles for XFS and famfs whilst
> > running this workload so we have some insight into what is behaving
> > differently here?
>
> Can you point me to an example of how to do that?

perf record --call-graph ...
pref report --call-graph ...

> I'd been thinking about the ~2x gap for a few days, and the most obvious
> difference is famfs files must be preallocated (like fallocate, but works
> a bit differently since allocation happens in user space). I just checked
> one of the xfs files, and it had maybe 80 extents (whereas the famfs
> files always have 1 extent here).

Which is about 4MB per extent. Extent size is not the problem for
zero-seek-latency storage hardware, though.

Essentially what you are seeing is interleaving extent allocation
between all the files because they are located in the same
directory. The locality algorithm is trying to place the data
extents close to the owner inode, but the indoes are also all close
together because they are located in the same AG as the parent
directory inode. Allocation concurrency is created by placing new
directories in different allocation groups, so we end up with
workloads in different directories being largely isolated from each
other.

However, that means when you are trying to write to many files in
the same directory at the same time, they are largely all competing
for the same AG lock to do block allocation during IO submission.
That creates interleaving of write() sized extents between different
files. We use speculative preallocation for buffered IO to avoid
this, and for direct IO the application needs to use extent size hints
or preallocation to avoid this contention based interleaving.

IOWs, by using fallocate() to preallocate all the space there will
be no allocation during IO submission and so the serialisation that
occurs due to competing allocations just goes away...

> FWIW I ran xfs with and without io_uring, and there was no apparent
> difference (which makes sense to me because it's not block I/O).
>
> The prior ~2x gap still seems like a lot of overhead for extent list
> mapping to memory, but adding --fallocate=native to the xfs test brought
> it into line with famfs:

As I suspected. :)

As for CPU usage accounting, the number of context switches says it
all.

"Bad":

> cpu : usr=15.48%, sys=1.17%, ctx=62654, majf=0, minf=22801

"good":

> cpu : usr=14.43%, sys=78.18%, ctx=5272, majf=0, minf=15708

I'd say that in the "bad" case most of the kernel work is being
shuffled off to kernel threads to do the work and so it doesn't get
accounted to the submission task. In comparison, in the "good" case
the work is being done in the submission thread and hence there's a
lot fewer context switches and the system time is correctly
accounted to the submission task.

Perhaps an io_uring task accounting problem?

Cheers,

Dave.
--
Dave Chinner
[email protected]