Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 23 Jul 2019 22:01:18 -0600
From:   Sultan Alsawaf <sultan@kerneltoast.com>
To:     Andreas Dilger <adilger@dilger.ca>
Cc:     Alexander Viro <viro@zeniv.linux.org.uk>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] mbcache: Speed up cache entry creation
Message-ID: <20190724040118.GA31214@sultan-box.localdomain>
References: <20190723053549.14465-1-sultan@kerneltoast.com>
 <5EDDA127-031C-4F16-9B9B-8DBC94C7E471@dilger.ca>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <5EDDA127-031C-4F16-9B9B-8DBC94C7E471@dilger.ca>
User-Agent: Mutt/1.12.1 (2019-06-15)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Jul 23, 2019 at 10:56:05AM -0600, Andreas Dilger wrote:
> Do you have any kind of performance metrics that show this is an actual
> improvement in performance?  This would be either macro-level benchmarks
> (e.g. fio, but this seems unlikely to show any benefit), or micro-level
> measurements (e.g. flame graph) that show a net reduction in CPU cycles,
> lock contention, etc. in this part of the code.

Hi Andreas,

Here are some basic micro-benchmark results:

Before:
[    3.162896] mb_cache_entry_create: AVG cycles: 75
[    3.054701] mb_cache_entry_create: AVG cycles: 78
[    3.152321] mb_cache_entry_create: AVG cycles: 77

After:
[    3.043380] mb_cache_entry_create: AVG cycles: 68
[    3.194321] mb_cache_entry_create: AVG cycles: 71
[    3.038100] mb_cache_entry_create: AVG cycles: 69

The performance difference is probably more drastic when free memory is low,
since an unnecessary call to kmem_cache_alloc() can result in a long wait for
pages to be freed.

The micro-benchmark code is attached.

Thanks,
Sultan
---
diff --git a/fs/mbcache.c b/fs/mbcache.c
index 289f3664061e..e0f22ff8fab8 100644
--- a/fs/mbcache.c
+++ b/fs/mbcache.c
@@ -82,7 +82,7 @@ static inline struct mb_bucket *mb_cache_entry_bucket(struct mb_cache *cache,
  * -EBUSY if entry with the same key and value already exists in cache.
  * Otherwise 0 is returned.
  */
-int mb_cache_entry_create(struct mb_cache *cache, gfp_t mask, u32 key,
+static int __mb_cache_entry_create(struct mb_cache *cache, gfp_t mask, u32 key,
 			  u64 value, bool reusable)
 {
 	struct mb_cache_entry *entry, *dup;
@@ -148,6 +148,29 @@ int mb_cache_entry_create(struct mb_cache *cache, gfp_t mask, u32 key,
 
 	return 0;
 }
+
+int mb_cache_entry_create(struct mb_cache *cache, gfp_t mask, u32 key,
+			  u64 value, bool reusable)
+{
+	static unsigned long count, sum;
+	static DEFINE_MUTEX(lock);
+	volatile cycles_t start, delta;
+	int ret;
+
+	mutex_lock(&lock);
+	local_irq_disable();
+	start = get_cycles();
+	ret = __mb_cache_entry_create(cache, mask, key, value, reusable);
+	delta = get_cycles() - start;
+	local_irq_enable();
+
+	sum += delta;
+	if (++count == 1000)
+		printk("%s: AVG cycles: %lu\n", __func__, sum / count);
+	mutex_unlock(&lock);
+
+	return ret;
+}
 EXPORT_SYMBOL(mb_cache_entry_create);
 
 void __mb_cache_entry_free(struct mb_cache_entry *entry)