From: Amir Goldstein <[email protected]>
There are a lot of arguments to fsnotify() and the handle_event() method.
Pass them in a const struct instead of on the argument list.
Apart from being more tidy, this helps with passing error reports to the
backend. __fsnotify_parent() argument list was intentionally left
untouched, because its argument list is still short enough and because
most of the event info arguments are initialized inside
__fsnotify_parent().
Signed-off-by: Amir Goldstein <[email protected]>
[ Fix kernelbot warnings]
[avoid __fsnotify version when not needed]
Signed-off-by: Gabriel Krisman Bertazi <[email protected]>
---
fs/notify/fanotify/fanotify.c | 59 ++++++++++++--------------
fs/notify/fsnotify.c | 73 +++++++++++++++++---------------
include/linux/fsnotify_backend.h | 73 +++++++++++++++++++++++---------
3 files changed, 120 insertions(+), 85 deletions(-)
diff --git a/fs/notify/fanotify/fanotify.c b/fs/notify/fanotify/fanotify.c
index c3eefe3f6494..6875d4d34c0c 100644
--- a/fs/notify/fanotify/fanotify.c
+++ b/fs/notify/fanotify/fanotify.c
@@ -253,21 +253,22 @@ static int fanotify_get_response(struct fsnotify_group *group,
* been included within the event mask, but have not been explicitly
* requested by the user, will not be present in the returned mask.
*/
-static u32 fanotify_group_event_mask(struct fsnotify_group *group,
- struct fsnotify_iter_info *iter_info,
- u32 event_mask, const void *data,
- int data_type, struct inode *dir)
+static u32 fanotify_group_event_mask(
+ struct fsnotify_group *group, u32 event_mask,
+ const struct fsnotify_event_info *event_info,
+ struct fsnotify_iter_info *iter_info)
{
__u32 marks_mask = 0, marks_ignored_mask = 0;
__u32 test_mask, user_mask = FANOTIFY_OUTGOING_EVENTS |
FANOTIFY_EVENT_FLAGS;
- const struct path *path = fsnotify_data_path(data, data_type);
+ const struct path *path = fsnotify_event_info_path(event_info);
unsigned int fid_mode = FAN_GROUP_FLAG(group, FANOTIFY_FID_BITS);
struct fsnotify_mark *mark;
int type;
pr_debug("%s: report_mask=%x mask=%x data=%p data_type=%d\n",
- __func__, iter_info->report_mask, event_mask, data, data_type);
+ __func__, iter_info->report_mask, event_mask,
+ event_info->data, event_info->data_type);
if (!fid_mode) {
/* Do we have path to open a file descriptor? */
@@ -278,7 +279,7 @@ static u32 fanotify_group_event_mask(struct fsnotify_group *group,
return 0;
} else if (!(fid_mode & FAN_REPORT_FID)) {
/* Do we have a directory inode to report? */
- if (!dir && !(event_mask & FS_ISDIR))
+ if (!event_info->dir && !(event_mask & FS_ISDIR))
return 0;
}
@@ -427,13 +428,13 @@ static int fanotify_encode_fh(struct fanotify_fh *fh, struct inode *inode,
* FS_ATTRIB reports the child inode even if reported on a watched parent.
* FS_CREATE reports the modified dir inode and not the created inode.
*/
-static struct inode *fanotify_fid_inode(u32 event_mask, const void *data,
- int data_type, struct inode *dir)
+static struct inode *fanotify_fid_inode(u32 event_mask,
+ const struct fsnotify_event_info *event_info)
{
if (event_mask & ALL_FSNOTIFY_DIRENT_EVENTS)
- return dir;
+ return event_info->dir;
- return fsnotify_data_inode(data, data_type);
+ return fsnotify_event_info_inode(event_info);
}
/*
@@ -444,18 +445,18 @@ static struct inode *fanotify_fid_inode(u32 event_mask, const void *data,
* reported to parent.
* Otherwise, do not report dir fid.
*/
-static struct inode *fanotify_dfid_inode(u32 event_mask, const void *data,
- int data_type, struct inode *dir)
+static struct inode *fanotify_dfid_inode(u32 event_mask,
+ const struct fsnotify_event_info *event_info)
{
- struct inode *inode = fsnotify_data_inode(data, data_type);
+ struct inode *inode = fsnotify_event_info_inode(event_info);
if (event_mask & ALL_FSNOTIFY_DIRENT_EVENTS)
- return dir;
+ return event_info->dir;
if (S_ISDIR(inode->i_mode))
return inode;
- return dir;
+ return event_info->dir;
}
static struct fanotify_event *fanotify_alloc_path_event(const struct path *path,
@@ -563,17 +564,17 @@ static struct fanotify_event *fanotify_alloc_name_event(struct inode *id,
return &fne->fae;
}
-static struct fanotify_event *fanotify_alloc_event(struct fsnotify_group *group,
- u32 mask, const void *data,
- int data_type, struct inode *dir,
- const struct qstr *file_name,
- __kernel_fsid_t *fsid)
+static struct fanotify_event *fanotify_alloc_event(
+ struct fsnotify_group *group, u32 mask,
+ const struct fsnotify_event_info *event_info,
+ __kernel_fsid_t *fsid)
{
struct fanotify_event *event = NULL;
gfp_t gfp = GFP_KERNEL_ACCOUNT;
- struct inode *id = fanotify_fid_inode(mask, data, data_type, dir);
- struct inode *dirid = fanotify_dfid_inode(mask, data, data_type, dir);
- const struct path *path = fsnotify_data_path(data, data_type);
+ struct inode *id = fanotify_fid_inode(mask, event_info);
+ struct inode *dirid = fanotify_dfid_inode(mask, event_info);
+ const struct path *path = fsnotify_event_info_path(event_info);
+ const struct qstr *file_name = event_info->name;
unsigned int fid_mode = FAN_GROUP_FLAG(group, FANOTIFY_FID_BITS);
struct mem_cgroup *old_memcg;
struct inode *child = NULL;
@@ -712,9 +713,7 @@ static void fanotify_insert_event(struct fsnotify_group *group,
}
static int fanotify_handle_event(struct fsnotify_group *group, u32 mask,
- const void *data, int data_type,
- struct inode *dir,
- const struct qstr *file_name, u32 cookie,
+ const struct fsnotify_event_info *event_info,
struct fsnotify_iter_info *iter_info)
{
int ret = 0;
@@ -744,8 +743,7 @@ static int fanotify_handle_event(struct fsnotify_group *group, u32 mask,
BUILD_BUG_ON(HWEIGHT32(ALL_FANOTIFY_EVENT_BITS) != 19);
- mask = fanotify_group_event_mask(group, iter_info, mask, data,
- data_type, dir);
+ mask = fanotify_group_event_mask(group, mask, event_info, iter_info);
if (!mask)
return 0;
@@ -767,8 +765,7 @@ static int fanotify_handle_event(struct fsnotify_group *group, u32 mask,
return 0;
}
- event = fanotify_alloc_event(group, mask, data, data_type, dir,
- file_name, &fsid);
+ event = fanotify_alloc_event(group, mask, event_info, &fsid);
ret = -ENOMEM;
if (unlikely(!event)) {
/*
diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c
index 30d422b8c0fc..7c783c9df1dd 100644
--- a/fs/notify/fsnotify.c
+++ b/fs/notify/fsnotify.c
@@ -240,13 +240,11 @@ int __fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data,
EXPORT_SYMBOL_GPL(__fsnotify_parent);
static int fsnotify_handle_inode_event(struct fsnotify_group *group,
- struct fsnotify_mark *inode_mark,
- u32 mask, const void *data, int data_type,
- struct inode *dir, const struct qstr *name,
- u32 cookie)
+ struct fsnotify_mark *inode_mark, u32 mask,
+ const struct fsnotify_event_info *event_info)
{
- const struct path *path = fsnotify_data_path(data, data_type);
- struct inode *inode = fsnotify_data_inode(data, data_type);
+ const struct path *path = fsnotify_event_info_path(event_info);
+ struct inode *inode = fsnotify_event_info_inode(event_info);
const struct fsnotify_ops *ops = group->ops;
if (WARN_ON_ONCE(!ops->handle_inode_event))
@@ -260,16 +258,17 @@ static int fsnotify_handle_inode_event(struct fsnotify_group *group,
if (!(mask & inode_mark->mask & ALL_FSNOTIFY_EVENTS))
return 0;
- return ops->handle_inode_event(inode_mark, mask, inode, dir, name, cookie);
+ return ops->handle_inode_event(inode_mark, mask, inode, event_info->dir,
+ event_info->name, event_info->cookie);
}
static int fsnotify_handle_event(struct fsnotify_group *group, __u32 mask,
- const void *data, int data_type,
- struct inode *dir, const struct qstr *name,
- u32 cookie, struct fsnotify_iter_info *iter_info)
+ const struct fsnotify_event_info *event_info,
+ struct fsnotify_iter_info *iter_info)
{
struct fsnotify_mark *inode_mark = fsnotify_iter_inode_mark(iter_info);
struct fsnotify_mark *parent_mark = fsnotify_iter_parent_mark(iter_info);
+ struct fsnotify_event_info child_event_info;
int ret;
if (WARN_ON_ONCE(fsnotify_iter_sb_mark(iter_info)) ||
@@ -284,8 +283,8 @@ static int fsnotify_handle_event(struct fsnotify_group *group, __u32 mask,
* interested in this event?
*/
if (parent_mark->mask & FS_EVENT_ON_CHILD) {
- ret = fsnotify_handle_inode_event(group, parent_mark, mask,
- data, data_type, dir, name, 0);
+ ret = fsnotify_handle_inode_event(group, parent_mark,
+ mask, event_info);
if (ret)
return ret;
}
@@ -302,18 +301,22 @@ static int fsnotify_handle_event(struct fsnotify_group *group, __u32 mask,
* The child watcher is expecting an event without a file name
* and without the FS_EVENT_ON_CHILD flag.
*/
+ if (WARN_ON_ONCE(!event_info->inode))
+ return 0;
+
mask &= ~FS_EVENT_ON_CHILD;
- dir = NULL;
- name = NULL;
+ child_event_info = *event_info;
+ child_event_info.dir = NULL;
+ child_event_info.name = NULL;
+ event_info = &child_event_info;
}
- return fsnotify_handle_inode_event(group, inode_mark, mask, data, data_type,
- dir, name, cookie);
+ return fsnotify_handle_inode_event(group, inode_mark, mask, event_info);
}
-static int send_to_group(__u32 mask, const void *data, int data_type,
- struct inode *dir, const struct qstr *file_name,
- u32 cookie, struct fsnotify_iter_info *iter_info)
+static int send_to_group(__u32 mask,
+ const struct fsnotify_event_info *event_info,
+ struct fsnotify_iter_info *iter_info)
{
struct fsnotify_group *group = NULL;
__u32 test_mask = (mask & ALL_FSNOTIFY_EVENTS);
@@ -351,18 +354,18 @@ static int send_to_group(__u32 mask, const void *data, int data_type,
pr_debug("%s: group=%p mask=%x marks_mask=%x marks_ignored_mask=%x data=%p data_type=%d dir=%p cookie=%d\n",
__func__, group, mask, marks_mask, marks_ignored_mask,
- data, data_type, dir, cookie);
+ event_info->data, event_info->data_type, event_info->dir,
+ event_info->cookie);
if (!(test_mask & marks_mask & ~marks_ignored_mask))
return 0;
if (group->ops->handle_event) {
- return group->ops->handle_event(group, mask, data, data_type, dir,
- file_name, cookie, iter_info);
+ return group->ops->handle_event(group, mask, event_info,
+ iter_info);
}
- return fsnotify_handle_event(group, mask, data, data_type, dir,
- file_name, cookie, iter_info);
+ return fsnotify_handle_event(group, mask, event_info, iter_info);
}
static struct fsnotify_mark *fsnotify_first_mark(struct fsnotify_mark_connector **connp)
@@ -448,21 +451,22 @@ static void fsnotify_iter_next(struct fsnotify_iter_info *iter_info)
* in whatever means they feel necessary.
*
* @mask: event type and flags
+ * Input args in struct fsnotify_event_info:
* @data: object that event happened on
* @data_type: type of object for fanotify_data_XXX() accessors
* @dir: optional directory associated with event -
- * if @file_name is not NULL, this is the directory that
- * @file_name is relative to
- * @file_name: optional file name associated with event
+ * if @name is not NULL, this is the directory that
+ * @name is relative to
+ * @name: optional file name associated with event
* @inode: optional inode associated with event -
* either @dir or @inode must be non-NULL.
* if both are non-NULL event may be reported to both.
* @cookie: inotify rename cookie
*/
-int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
- const struct qstr *file_name, struct inode *inode, u32 cookie)
+int __fsnotify(__u32 mask, const struct fsnotify_event_info *event_info)
{
- const struct path *path = fsnotify_data_path(data, data_type);
+ const struct path *path = fsnotify_event_info_path(event_info);
+ struct inode *inode = event_info->inode;
struct fsnotify_iter_info iter_info = {};
struct super_block *sb;
struct mount *mnt = NULL;
@@ -475,13 +479,13 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
if (!inode) {
/* Dirent event - report on TYPE_INODE to dir */
- inode = dir;
+ inode = event_info->dir;
} else if (mask & FS_EVENT_ON_CHILD) {
/*
* Event on child - report on TYPE_PARENT to dir if it is
* watching children and on TYPE_INODE to child.
*/
- parent = dir;
+ parent = event_info->dir;
}
sb = inode->i_sb;
@@ -538,8 +542,7 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
* That's why this traversal is so complicated...
*/
while (fsnotify_iter_select_report_types(&iter_info)) {
- ret = send_to_group(mask, data, data_type, dir, file_name,
- cookie, &iter_info);
+ ret = send_to_group(mask, event_info, &iter_info);
if (ret && (mask & ALL_FSNOTIFY_PERM_EVENTS))
goto out;
@@ -552,7 +555,7 @@ int fsnotify(__u32 mask, const void *data, int data_type, struct inode *dir,
return ret;
}
-EXPORT_SYMBOL_GPL(fsnotify);
+EXPORT_SYMBOL_GPL(__fsnotify);
static __init int fsnotify_init(void)
{
diff --git a/include/linux/fsnotify_backend.h b/include/linux/fsnotify_backend.h
index 2b5fb9327a77..3c6fb43276ba 100644
--- a/include/linux/fsnotify_backend.h
+++ b/include/linux/fsnotify_backend.h
@@ -112,6 +112,28 @@ struct fsnotify_iter_info;
struct mem_cgroup;
+/*
+ * Event info args passed to fsnotify() and to backends on handle_event():
+ * @data: object that event happened on
+ * @data_type: type of object for fanotify_data_XXX() accessors
+ * @dir: optional directory associated with event -
+ * if @name is not NULL, this is the directory that
+ * @name is relative to
+ * @name: optional file name associated with event
+ * @inode: optional inode associated with event -
+ * either @dir or @inode must be non-NULL.
+ * if both are non-NULL event may be reported to both.
+ * @cookie: inotify rename cookie
+ */
+struct fsnotify_event_info {
+ const void *data;
+ int data_type;
+ struct inode *dir;
+ const struct qstr *name;
+ struct inode *inode;
+ u32 cookie;
+};
+
/*
* Each group much define these ops. The fsnotify infrastructure will call
* these operations for each relevant group.
@@ -119,13 +141,7 @@ struct mem_cgroup;
* handle_event - main call for a group to handle an fs event
* @group: group to notify
* @mask: event type and flags
- * @data: object that event happened on
- * @data_type: type of object for fanotify_data_XXX() accessors
- * @dir: optional directory associated with event -
- * if @file_name is not NULL, this is the directory that
- * @file_name is relative to
- * @file_name: optional file name associated with event
- * @cookie: inotify rename cookie
+ * @event_info: information attached to the event
* @iter_info: array of marks from this group that are interested in the event
*
* handle_inode_event - simple variant of handle_event() for groups that only
@@ -147,8 +163,7 @@ struct mem_cgroup;
*/
struct fsnotify_ops {
int (*handle_event)(struct fsnotify_group *group, u32 mask,
- const void *data, int data_type, struct inode *dir,
- const struct qstr *file_name, u32 cookie,
+ const struct fsnotify_event_info *event_info,
struct fsnotify_iter_info *iter_info);
int (*handle_inode_event)(struct fsnotify_mark *mark, u32 mask,
struct inode *inode, struct inode *dir,
@@ -262,6 +277,12 @@ static inline struct inode *fsnotify_data_inode(const void *data, int data_type)
}
}
+static inline struct inode *fsnotify_event_info_inode(
+ const struct fsnotify_event_info *event_info)
+{
+ return fsnotify_data_inode(event_info->data, event_info->data_type);
+}
+
static inline const struct path *fsnotify_data_path(const void *data,
int data_type)
{
@@ -273,6 +294,12 @@ static inline const struct path *fsnotify_data_path(const void *data,
}
}
+static inline const struct path *fsnotify_event_info_path(
+ const struct fsnotify_event_info *event_info)
+{
+ return fsnotify_data_path(event_info->data, event_info->data_type);
+}
+
enum fsnotify_obj_type {
FSNOTIFY_OBJ_TYPE_INODE,
FSNOTIFY_OBJ_TYPE_PARENT,
@@ -422,11 +449,11 @@ struct fsnotify_mark {
/* called from the vfs helpers */
/* main fsnotify call to send events */
-extern int fsnotify(__u32 mask, const void *data, int data_type,
- struct inode *dir, const struct qstr *name,
- struct inode *inode, u32 cookie);
-extern int __fsnotify_parent(struct dentry *dentry, __u32 mask, const void *data,
- int data_type);
+extern int __fsnotify(__u32 mask,
+ const struct fsnotify_event_info *event_info);
+extern int __fsnotify_parent(struct dentry *dentry, __u32 mask,
+ const void *data, int data_type);
+
extern void __fsnotify_inode_delete(struct inode *inode);
extern void __fsnotify_vfsmount_delete(struct vfsmount *mnt);
extern void fsnotify_sb_delete(struct super_block *sb);
@@ -605,16 +632,14 @@ static inline void fsnotify_init_event(struct fsnotify_event *event)
}
#else
-
-static inline int fsnotify(__u32 mask, const void *data, int data_type,
- struct inode *dir, const struct qstr *name,
- struct inode *inode, u32 cookie)
+static inline int __fsnotify(__u32 mask,
+ const struct fsnotify_event_info *event_info)
{
return 0;
}
static inline int __fsnotify_parent(struct dentry *dentry, __u32 mask,
- const void *data, int data_type)
+ const void *data, int data_type)
{
return 0;
}
@@ -641,6 +666,16 @@ static inline void fsnotify_unmount_inodes(struct super_block *sb)
#endif /* CONFIG_FSNOTIFY */
+static inline int fsnotify(__u32 mask, const void *data, int data_type,
+ struct inode *dir, const struct qstr *name,
+ struct inode *inode, u32 cookie)
+{
+ return __fsnotify(mask, &(struct fsnotify_event_info) {
+ .data = (data), .data_type = (data_type), .dir = (dir),
+ .name = (name), .inode = (inode), .cookie = (cookie)
+ });
+}
+
#endif /* __KERNEL __ */
#endif /* __LINUX_FSNOTIFY_BACKEND_H */
--
2.32.0
Greeting,
FYI, we noticed a -3.3% regression of unixbench.score due to commit:
commit: 4c40d6efc8b22b88a45c335ffd6d25b55d769f5b ("[PATCH v4 08/16] fsnotify: pass arguments of fsnotify() in struct fsnotify_event_info")
url: https://github.com/0day-ci/linux/commits/Gabriel-Krisman-Bertazi/File-system-wide-monitoring/20210721-001444
base: https://git.kernel.org/cgit/linux/kernel/git/jack/linux-fs.git fsnotify
in testcase: unixbench
on test machine: 96 threads 2 sockets Intel(R) Xeon(R) CPU @ 2.30GHz with 128G memory
with following parameters:
runtime: 300s
nr_task: 1
test: pipe
cpufreq_governor: performance
ucode: 0x4003006
test-description: UnixBench is the original BYTE UNIX benchmark suite aims to test performance of Unix-like system.
test-url: https://github.com/kdlucas/byte-unixbench
In addition to that, the commit also has significant impact on the following tests:
+------------------+-------------------------------------------------------------------------------------+
| testcase: change | will-it-scale: will-it-scale.per_thread_ops -1.3% regression |
| test machine | 192 threads 4 sockets Intel(R) Xeon(R) Platinum 9242 CPU @ 2.30GHz with 192G memory |
| test parameters | cpufreq_governor=performance |
| | mode=thread |
| | nr_task=100% |
| | test=eventfd1 |
| | ucode=0x5003006 |
+------------------+-------------------------------------------------------------------------------------+
If you fix the issue, kindly add following tag
Reported-by: kernel test robot <[email protected]>
Details are as below:
-------------------------------------------------------------------------------------------------->
To reproduce:
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp install job.yaml # job file is attached in this email
bin/lkp split-job --compatible job.yaml # generate the yaml file for lkp run
bin/lkp run generated-yaml-file
=========================================================================================
compiler/cpufreq_governor/kconfig/nr_task/rootfs/runtime/tbox_group/test/testcase/ucode:
gcc-9/performance/x86_64-rhel-8.3/1/debian-10.4-x86_64-20200603.cgz/300s/lkp-csl-2sp4/pipe/unixbench/0x4003006
commit:
263b74f276 ("fsnotify: Add helper to detect overflow_event")
4c40d6efc8 ("fsnotify: pass arguments of fsnotify() in struct fsnotify_event_info")
263b74f2761d777d 4c40d6efc8b22b88a45c335ffd6
---------------- ---------------------------
%stddev %change %stddev
\ | \
1554 -3.3% 1503 unixbench.score
7.551e+08 -3.4% 7.295e+08 unixbench.workload
0.00 ?158% +4075.0% 0.02 ? 37% perf-sched.wait_time.avg.ms.preempt_schedule_common.__cond_resched.copy_page_from_iter.pipe_write.new_sync_write
0.00 ?158% +4075.0% 0.02 ? 37% perf-sched.wait_time.max.ms.preempt_schedule_common.__cond_resched.copy_page_from_iter.pipe_write.new_sync_write
318916 ?169% -95.4% 14689 ? 5% perf-stat.i.dTLB-load-misses
5208687 ? 7% +26.3% 6579325 ? 2% perf-stat.i.iTLB-load-misses
991.91 ? 5% -17.0% 822.95 ? 2% perf-stat.i.instructions-per-iTLB-miss
0.02 ?170% -0.0 0.00 ? 5% perf-stat.overall.dTLB-load-miss-rate%
931.13 ? 7% -22.5% 721.29 ? 2% perf-stat.overall.instructions-per-iTLB-miss
2491 +1.8% 2535 perf-stat.overall.path-length
318091 ?169% -95.4% 14652 ? 5% perf-stat.ps.dTLB-load-misses
5195369 ? 7% +26.3% 6562257 ? 2% perf-stat.ps.iTLB-load-misses
0.82 ? 13% +0.2 1.02 ? 8% perf-profile.calltrace.cycles-pp.mutex_lock.pipe_write.new_sync_write.vfs_write.ksys_write
0.69 ? 8% +0.2 0.91 ? 11% perf-profile.calltrace.cycles-pp.security_file_permission.vfs_write.ksys_write.do_syscall_64.entry_SYSCALL_64_after_hwframe
1.05 ? 10% +0.3 1.33 ? 6% perf-profile.calltrace.cycles-pp.security_file_permission.vfs_read.ksys_read.do_syscall_64.entry_SYSCALL_64_after_hwframe
0.39 ? 63% +0.3 0.69 ? 11% perf-profile.calltrace.cycles-pp.common_file_perm.security_file_permission.vfs_write.ksys_write.do_syscall_64
0.16 ?158% +0.5 0.68 ? 6% perf-profile.calltrace.cycles-pp.common_file_perm.security_file_permission.vfs_read.ksys_read.do_syscall_64
0.64 ? 16% -0.2 0.46 ? 17% perf-profile.children.cycles-pp.anon_pipe_buf_release
0.34 ? 47% -0.2 0.19 ? 10% perf-profile.children.cycles-pp.wait_for_xmitr
0.35 ? 46% -0.2 0.20 ? 10% perf-profile.children.cycles-pp.serial8250_console_write
0.33 ? 47% -0.1 0.19 ? 9% perf-profile.children.cycles-pp.serial8250_console_putchar
0.21 ? 29% -0.1 0.13 ? 22% perf-profile.children.cycles-pp.enqueue_hrtimer
0.18 ? 31% -0.1 0.11 ? 31% perf-profile.children.cycles-pp.timerqueue_add
0.30 ? 14% -0.1 0.23 ? 15% perf-profile.children.cycles-pp.update_blocked_averages
0.10 ? 21% -0.1 0.04 ? 91% perf-profile.children.cycles-pp.menu_reflect
1.37 ? 8% +0.4 1.76 ? 6% perf-profile.children.cycles-pp.common_file_perm
2.27 ? 8% +0.5 2.82 ? 5% perf-profile.children.cycles-pp.security_file_permission
0.00 +1.5 1.47 ? 14% perf-profile.children.cycles-pp.__fsnotify
0.63 ? 16% -0.2 0.45 ? 17% perf-profile.self.cycles-pp.anon_pipe_buf_release
0.11 ? 32% -0.0 0.07 ? 23% perf-profile.self.cycles-pp.sysvec_apic_timer_interrupt
0.47 ? 9% +0.1 0.58 ? 12% perf-profile.self.cycles-pp.new_sync_write
0.43 ? 14% +0.1 0.58 ? 12% perf-profile.self.cycles-pp.security_file_permission
0.86 ? 17% +0.4 1.28 ? 8% perf-profile.self.cycles-pp.common_file_perm
0.00 +1.5 1.45 ? 14% perf-profile.self.cycles-pp.__fsnotify
unixbench.score
1580 +--------------------------------------------------------------------+
| |
1560 |-+ .+...+...+.. ..+ |
| ..+. +. |
1540 |-+ .+. |
|...+..+...+...+. |
1520 |-+ |
| O O O O O O |
1500 |-+ O O O O O O |
| |
1480 |-+ O |
| O O O O |
1460 |-+ O |
| |
1440 +--------------------------------------------------------------------+
[*] bisect-good sample
[O] bisect-bad sample
***************************************************************************************************
lkp-csl-2ap2: 192 threads 4 sockets Intel(R) Xeon(R) Platinum 9242 CPU @ 2.30GHz with 192G memory
=========================================================================================
compiler/cpufreq_governor/kconfig/mode/nr_task/rootfs/tbox_group/test/testcase/ucode:
gcc-9/performance/x86_64-rhel-8.3/thread/100%/debian-10.4-x86_64-20200603.cgz/lkp-csl-2ap2/eventfd1/will-it-scale/0x5003006
commit:
263b74f276 ("fsnotify: Add helper to detect overflow_event")
4c40d6efc8 ("fsnotify: pass arguments of fsnotify() in struct fsnotify_event_info")
263b74f2761d777d 4c40d6efc8b22b88a45c335ffd6
---------------- ---------------------------
%stddev %change %stddev
\ | \
3.014e+08 -1.3% 2.974e+08 will-it-scale.192.threads
1569651 -1.3% 1549107 will-it-scale.per_thread_ops
3.014e+08 -1.3% 2.974e+08 will-it-scale.workload
747.17 ? 37% +1300.3% 10462 ?145% softirqs.CPU13.NET_RX
1212 ? 41% +1341.6% 17474 ?133% interrupts.34:PCI-MSI.524292-edge.eth0-TxRx-3
1212 ? 41% +1341.6% 17474 ?133% interrupts.CPU13.34:PCI-MSI.524292-edge.eth0-TxRx-3
0.01 ? 8% +32.5% 0.02 ? 15% perf-sched.sch_delay.avg.ms.pipe_read.new_sync_read.vfs_read.ksys_read
2.86 ?162% +256.5% 10.20 ? 38% perf-sched.wait_time.avg.ms.preempt_schedule_common.__cond_resched.__do_fault.do_fault.__handle_mm_fault
38371 -1.5% 37785 proc-vmstat.nr_slab_reclaimable
71698 -1.8% 70437 proc-vmstat.nr_slab_unreclaimable
5621 ? 6% -12.4% 4926 ? 3% slabinfo.Acpi-State.active_objs
5621 ? 6% -12.4% 4926 ? 3% slabinfo.Acpi-State.num_objs
5395 ? 7% -13.4% 4670 ? 7% slabinfo.files_cache.active_objs
5395 ? 7% -13.4% 4670 ? 7% slabinfo.files_cache.num_objs
1070171 ? 2% +8.4% 1160265 ? 2% perf-stat.i.cache-misses
7414136 ? 2% +11.7% 8284549 ? 2% perf-stat.i.cache-references
741668 ? 3% -12.8% 647061 ? 4% perf-stat.i.cycles-between-cache-misses
1.598e+11 +1.5% 1.621e+11 perf-stat.i.dTLB-loads
165125 -2.3% 161301 perf-stat.i.dTLB-store-misses
5.501e+08 ? 3% +4.5% 5.746e+08 perf-stat.i.iTLB-load-misses
1005 ? 4% -4.3% 962.30 perf-stat.i.instructions-per-iTLB-miss
237876 ? 2% +10.3% 262365 ? 3% perf-stat.i.node-load-misses
91893 ? 2% +4.9% 96434 perf-stat.i.node-store-misses
0.01 ? 2% +11.4% 0.02 ? 2% perf-stat.overall.MPKI
497110 ? 3% -6.9% 462766 ? 2% perf-stat.overall.cycles-between-cache-misses
0.00 -0.0 0.00 perf-stat.overall.dTLB-store-miss-rate%
1004 ? 4% -4.5% 960.10 perf-stat.overall.instructions-per-iTLB-miss
551166 +1.3% 558404 perf-stat.overall.path-length
1107917 ? 3% +7.4% 1190240 ? 2% perf-stat.ps.cache-misses
7527216 ? 2% +11.4% 8382817 ? 2% perf-stat.ps.cache-references
1.592e+11 +1.5% 1.616e+11 perf-stat.ps.dTLB-loads
164932 -2.3% 161085 perf-stat.ps.dTLB-store-misses
5.483e+08 ? 3% +4.5% 5.729e+08 perf-stat.ps.iTLB-load-misses
237049 ? 2% +10.4% 261604 ? 4% perf-stat.ps.node-load-misses
91568 ? 2% +4.9% 96075 perf-stat.ps.node-store-misses
9.39 -0.3 9.05 perf-profile.calltrace.cycles-pp.eventfd_write.vfs_write.ksys_write.do_syscall_64.entry_SYSCALL_64_after_hwframe
11.74 -0.2 11.57 perf-profile.calltrace.cycles-pp.eventfd_read.new_sync_read.vfs_read.ksys_read.do_syscall_64
7.75 -0.1 7.65 perf-profile.calltrace.cycles-pp.__entry_text_start.__libc_read
7.76 -0.1 7.66 perf-profile.calltrace.cycles-pp.__entry_text_start.__libc_write
7.46 -0.1 7.37 perf-profile.calltrace.cycles-pp._copy_to_iter.eventfd_read.new_sync_read.vfs_read.ksys_read
1.54 -0.1 1.45 perf-profile.calltrace.cycles-pp.syscall_return_via_sysret.__libc_write
1.54 -0.1 1.46 perf-profile.calltrace.cycles-pp.syscall_return_via_sysret.__libc_read
2.59 -0.1 2.53 perf-profile.calltrace.cycles-pp.syscall_exit_to_user_mode.do_syscall_64.entry_SYSCALL_64_after_hwframe.__libc_read
1.82 -0.0 1.78 perf-profile.calltrace.cycles-pp.exit_to_user_mode_prepare.syscall_exit_to_user_mode.do_syscall_64.entry_SYSCALL_64_after_hwframe.__libc_read
2.58 -0.0 2.54 perf-profile.calltrace.cycles-pp.syscall_exit_to_user_mode.do_syscall_64.entry_SYSCALL_64_after_hwframe.__libc_write
1.81 -0.0 1.78 perf-profile.calltrace.cycles-pp.exit_to_user_mode_prepare.syscall_exit_to_user_mode.do_syscall_64.entry_SYSCALL_64_after_hwframe.__libc_write
1.06 -0.0 1.03 perf-profile.calltrace.cycles-pp.copy_user_enhanced_fast_string._copy_from_user.eventfd_write.vfs_write.ksys_write
1.74 -0.0 1.70 perf-profile.calltrace.cycles-pp._raw_spin_lock_irq.eventfd_read.new_sync_read.vfs_read.ksys_read
1.34 -0.0 1.32 perf-profile.calltrace.cycles-pp.fput_many.ksys_write.do_syscall_64.entry_SYSCALL_64_after_hwframe.__libc_write
0.55 -0.0 0.53 perf-profile.calltrace.cycles-pp.entry_SYSCALL_64_safe_stack.__libc_read
0.64 +0.0 0.67 perf-profile.calltrace.cycles-pp.__might_sleep.__might_fault._copy_from_user.eventfd_write.vfs_write
1.91 +0.0 1.95 perf-profile.calltrace.cycles-pp.__might_fault._copy_to_iter.eventfd_read.new_sync_read.vfs_read
1.99 +0.1 2.04 perf-profile.calltrace.cycles-pp.__might_fault._copy_from_user.eventfd_write.vfs_write.ksys_write
42.27 +0.1 42.38 perf-profile.calltrace.cycles-pp.entry_SYSCALL_64_after_hwframe.__libc_read
46.96 +0.1 47.07 perf-profile.calltrace.cycles-pp.__libc_write
0.68 ? 6% +0.1 0.82 ? 4% perf-profile.calltrace.cycles-pp.__x64_sys_write.do_syscall_64.entry_SYSCALL_64_after_hwframe.__libc_write
40.77 +0.2 40.94 perf-profile.calltrace.cycles-pp.do_syscall_64.entry_SYSCALL_64_after_hwframe.__libc_read
36.01 +0.3 36.27 perf-profile.calltrace.cycles-pp.ksys_read.do_syscall_64.entry_SYSCALL_64_after_hwframe.__libc_read
26.10 +0.3 26.36 perf-profile.calltrace.cycles-pp.ksys_write.do_syscall_64.entry_SYSCALL_64_after_hwframe.__libc_write
32.26 +0.3 32.55 perf-profile.calltrace.cycles-pp.entry_SYSCALL_64_after_hwframe.__libc_write
30.79 +0.3 31.13 perf-profile.calltrace.cycles-pp.do_syscall_64.entry_SYSCALL_64_after_hwframe.__libc_write
28.76 +0.6 29.33 perf-profile.calltrace.cycles-pp.vfs_read.ksys_read.do_syscall_64.entry_SYSCALL_64_after_hwframe.__libc_read
4.33 ? 4% +0.8 5.08 ? 2% perf-profile.calltrace.cycles-pp.security_file_permission.vfs_write.ksys_write.do_syscall_64.entry_SYSCALL_64_after_hwframe
18.63 ? 2% +0.8 19.40 perf-profile.calltrace.cycles-pp.vfs_write.ksys_write.do_syscall_64.entry_SYSCALL_64_after_hwframe.__libc_write
7.72 ? 3% +0.8 8.49 perf-profile.calltrace.cycles-pp.security_file_permission.vfs_read.ksys_read.do_syscall_64.entry_SYSCALL_64_after_hwframe
0.00 +1.7 1.66 ? 3% perf-profile.calltrace.cycles-pp.__fsnotify.vfs_write.ksys_write.do_syscall_64.entry_SYSCALL_64_after_hwframe
0.00 +2.0 1.96 perf-profile.calltrace.cycles-pp.__fsnotify.vfs_read.ksys_read.do_syscall_64.entry_SYSCALL_64_after_hwframe
0.00 +2.0 2.03 ? 2% perf-profile.calltrace.cycles-pp.__fsnotify.security_file_permission.vfs_read.ksys_read.do_syscall_64
7.05 ? 2% -7.0 0.00 perf-profile.children.cycles-pp.fsnotify
9.54 -0.3 9.22 perf-profile.children.cycles-pp.eventfd_write
8.66 -0.2 8.42 perf-profile.children.cycles-pp.syscall_return_via_sysret
11.96 -0.2 11.78 perf-profile.children.cycles-pp.eventfd_read
57.15 -0.2 56.98 perf-profile.children.cycles-pp.__libc_read
10.01 -0.1 9.88 perf-profile.children.cycles-pp.__entry_text_start
3.74 -0.1 3.66 perf-profile.children.cycles-pp.exit_to_user_mode_prepare
5.51 -0.1 5.43 perf-profile.children.cycles-pp.syscall_exit_to_user_mode
3.62 -0.1 3.56 perf-profile.children.cycles-pp._raw_spin_lock_irq
2.30 -0.0 2.26 perf-profile.children.cycles-pp.copy_user_enhanced_fast_string
3.27 -0.0 3.22 perf-profile.children.cycles-pp.copy_user_generic_unrolled
2.64 -0.0 2.62 perf-profile.children.cycles-pp.fput_many
2.06 +0.0 2.09 perf-profile.children.cycles-pp.___might_sleep
1.27 +0.0 1.32 perf-profile.children.cycles-pp.__might_sleep
4.17 +0.1 4.26 perf-profile.children.cycles-pp.__might_fault
47.14 +0.1 47.25 perf-profile.children.cycles-pp.__libc_write
0.72 ? 5% +0.1 0.84 ? 3% perf-profile.children.cycles-pp.__x64_sys_write
26.35 +0.2 26.59 perf-profile.children.cycles-pp.ksys_write
36.20 +0.3 36.48 perf-profile.children.cycles-pp.ksys_read
74.75 +0.4 75.16 perf-profile.children.cycles-pp.entry_SYSCALL_64_after_hwframe
0.48 ? 3% +0.5 1.00 ? 3% perf-profile.children.cycles-pp.apparmor_file_permission
71.83 +0.6 72.38 perf-profile.children.cycles-pp.do_syscall_64
18.86 ? 2% +0.7 19.59 perf-profile.children.cycles-pp.vfs_write
12.35 ? 3% +1.4 13.79 perf-profile.children.cycles-pp.security_file_permission
0.00 +5.8 5.80 perf-profile.children.cycles-pp.__fsnotify
6.73 ? 2% -6.7 0.00 perf-profile.self.cycles-pp.fsnotify
2.20 ? 2% -0.3 1.93 ? 2% perf-profile.self.cycles-pp.eventfd_write
8.57 -0.2 8.34 perf-profile.self.cycles-pp.syscall_return_via_sysret
2.96 -0.1 2.81 perf-profile.self.cycles-pp.entry_SYSCALL_64_after_hwframe
1.51 -0.1 1.41 ? 2% perf-profile.self.cycles-pp.ksys_write
3.32 -0.1 3.25 perf-profile.self.cycles-pp.exit_to_user_mode_prepare
4.50 -0.1 4.43 perf-profile.self.cycles-pp.__entry_text_start
2.57 -0.1 2.50 perf-profile.self.cycles-pp.eventfd_read
2.53 -0.1 2.46 perf-profile.self.cycles-pp._copy_to_iter
3.45 -0.1 3.39 perf-profile.self.cycles-pp._raw_spin_lock_irq
2.52 -0.0 2.47 perf-profile.self.cycles-pp.fput_many
0.83 -0.0 0.79 perf-profile.self.cycles-pp._copy_from_user
2.10 -0.0 2.06 perf-profile.self.cycles-pp.copy_user_enhanced_fast_string
0.92 -0.0 0.89 perf-profile.self.cycles-pp.syscall_exit_to_user_mode_prepare
2.01 +0.0 2.04 perf-profile.self.cycles-pp.___might_sleep
1.11 +0.0 1.16 perf-profile.self.cycles-pp.__might_sleep
0.70 ? 4% +0.1 0.80 ? 2% perf-profile.self.cycles-pp.__x64_sys_write
2.99 ? 2% +0.4 3.36 ? 2% perf-profile.self.cycles-pp.vfs_read
2.53 ? 3% +0.5 3.03 ? 4% perf-profile.self.cycles-pp.vfs_write
0.38 ? 4% +0.5 0.89 ? 3% perf-profile.self.cycles-pp.apparmor_file_permission
2.64 ? 13% +0.9 3.54 ? 9% perf-profile.self.cycles-pp.security_file_permission
0.00 +5.6 5.59 perf-profile.self.cycles-pp.__fsnotify
Disclaimer:
Results have been estimated based on internal Intel analysis and are provided
for informational purposes only. Any difference in system hardware or software
design or configuration may affect actual performance.
---
0DAY/LKP+ Test Infrastructure Open Source Technology Center
https://lists.01.org/hyperkitty/list/[email protected] Intel Corporation
Thanks,
Oliver Sang
On Sat, Jul 31, 2021 at 9:20 AM kernel test robot <[email protected]> wrote:
>
>
>
> Greeting,
>
> FYI, we noticed a -3.3% regression of unixbench.score due to commit:
>
>
> commit: 4c40d6efc8b22b88a45c335ffd6d25b55d769f5b ("[PATCH v4 08/16] fsnotify: pass arguments of fsnotify() in struct fsnotify_event_info")
> url: https://github.com/0day-ci/linux/commits/Gabriel-Krisman-Bertazi/File-system-wide-monitoring/20210721-001444
> base: https://git.kernel.org/cgit/linux/kernel/git/jack/linux-fs.git fsnotify
>
> in testcase: unixbench
> on test machine: 96 threads 2 sockets Intel(R) Xeon(R) CPU @ 2.30GHz with 128G memory
> with following parameters:
>
> runtime: 300s
> nr_task: 1
> test: pipe
> cpufreq_governor: performance
> ucode: 0x4003006
>
> test-description: UnixBench is the original BYTE UNIX benchmark suite aims to test performance of Unix-like system.
> test-url: https://github.com/kdlucas/byte-unixbench
>
> In addition to that, the commit also has significant impact on the following tests:
>
> +------------------+-------------------------------------------------------------------------------------+
> | testcase: change | will-it-scale: will-it-scale.per_thread_ops -1.3% regression |
> | test machine | 192 threads 4 sockets Intel(R) Xeon(R) Platinum 9242 CPU @ 2.30GHz with 192G memory |
> | test parameters | cpufreq_governor=performance |
> | | mode=thread |
> | | nr_task=100% |
> | | test=eventfd1 |
> | | ucode=0x5003006 |
> +------------------+-------------------------------------------------------------------------------------+
>
>
> If you fix the issue, kindly add following tag
> Reported-by: kernel test robot <[email protected]>
>
Gabriel,
It looks like my change throws away much of the performance gain for
small IO on pipes without any watches that was achieved by commit
71d734103edf ("fsnotify: Rearrange fast path to minimise overhead
when there is no watcher").
I think the way to fix it is to lift the optimization in __fsnotify()
to the fsnotify_parent() inline wrapper as Mel considered doing
but was not sure it was worth the effort at the time.
It's not completely trivial. I think it requires setting a flag
MNT_FSNOTIFY_WATCHED when there are watches on the
vfsmount. I will look into it.
Thanks,
Amir.
On Sat, Jul 31, 2021 at 12:27 PM Amir Goldstein <[email protected]> wrote:
>
> On Sat, Jul 31, 2021 at 9:20 AM kernel test robot <[email protected]> wrote:
> >
> >
> >
> > Greeting,
> >
> > FYI, we noticed a -3.3% regression of unixbench.score due to commit:
> >
> >
> > commit: 4c40d6efc8b22b88a45c335ffd6d25b55d769f5b ("[PATCH v4 08/16] fsnotify: pass arguments of fsnotify() in struct fsnotify_event_info")
> > url: https://github.com/0day-ci/linux/commits/Gabriel-Krisman-Bertazi/File-system-wide-monitoring/20210721-001444
> > base: https://git.kernel.org/cgit/linux/kernel/git/jack/linux-fs.git fsnotify
> >
> > in testcase: unixbench
> > on test machine: 96 threads 2 sockets Intel(R) Xeon(R) CPU @ 2.30GHz with 128G memory
> > with following parameters:
> >
> > runtime: 300s
> > nr_task: 1
> > test: pipe
> > cpufreq_governor: performance
> > ucode: 0x4003006
> >
> > test-description: UnixBench is the original BYTE UNIX benchmark suite aims to test performance of Unix-like system.
> > test-url: https://github.com/kdlucas/byte-unixbench
> >
> > In addition to that, the commit also has significant impact on the following tests:
> >
> > +------------------+-------------------------------------------------------------------------------------+
> > | testcase: change | will-it-scale: will-it-scale.per_thread_ops -1.3% regression |
> > | test machine | 192 threads 4 sockets Intel(R) Xeon(R) Platinum 9242 CPU @ 2.30GHz with 192G memory |
> > | test parameters | cpufreq_governor=performance |
> > | | mode=thread |
> > | | nr_task=100% |
> > | | test=eventfd1 |
> > | | ucode=0x5003006 |
> > +------------------+-------------------------------------------------------------------------------------+
> >
> >
> > If you fix the issue, kindly add following tag
> > Reported-by: kernel test robot <[email protected]>
> >
>
> Gabriel,
>
> It looks like my change throws away much of the performance gain for
> small IO on pipes without any watches that was achieved by commit
> 71d734103edf ("fsnotify: Rearrange fast path to minimise overhead
> when there is no watcher").
>
> I think the way to fix it is to lift the optimization in __fsnotify()
> to the fsnotify_parent() inline wrapper as Mel considered doing
> but was not sure it was worth the effort at the time.
>
> It's not completely trivial. I think it requires setting a flag
> MNT_FSNOTIFY_WATCHED when there are watches on the
> vfsmount. I will look into it.
>
Oliver,
Would it be possible to request a re-test with the branch:
https://github.com/amir73il/linux fsnotify-perf
The patch at the tip of that branch is the one this regression report
has blamed.
My expectation is that the patch at fsnotify-perf^ ("fsnotify: optimize the
case of no marks of any type") will improve performance of the test case
compared to baseline (v5.14-rc3) and that the patch at the tip of fsnotify-perf
would not regress performance.
Thanks,
Amir.
Amir Goldstein <[email protected]> writes:
> On Sat, Jul 31, 2021 at 9:20 AM kernel test robot <[email protected]> wrote:
>>
>>
>>
>> Greeting,
>>
>> FYI, we noticed a -3.3% regression of unixbench.score due to commit:
>>
>>
>> commit: 4c40d6efc8b22b88a45c335ffd6d25b55d769f5b ("[PATCH v4 08/16] fsnotify: pass arguments of fsnotify() in struct fsnotify_event_info")
>> url: https://github.com/0day-ci/linux/commits/Gabriel-Krisman-Bertazi/File-system-wide-monitoring/20210721-001444
>> base: https://git.kernel.org/cgit/linux/kernel/git/jack/linux-fs.git fsnotify
>>
>> in testcase: unixbench
>> on test machine: 96 threads 2 sockets Intel(R) Xeon(R) CPU @ 2.30GHz with 128G memory
>> with following parameters:
>>
>> runtime: 300s
>> nr_task: 1
>> test: pipe
>> cpufreq_governor: performance
>> ucode: 0x4003006
>>
>> test-description: UnixBench is the original BYTE UNIX benchmark suite aims to test performance of Unix-like system.
>> test-url: https://github.com/kdlucas/byte-unixbench
>>
>> In addition to that, the commit also has significant impact on the following tests:
>>
>> +------------------+-------------------------------------------------------------------------------------+
>> | testcase: change | will-it-scale: will-it-scale.per_thread_ops -1.3% regression |
>> | test machine | 192 threads 4 sockets Intel(R) Xeon(R) Platinum 9242 CPU @ 2.30GHz with 192G memory |
>> | test parameters | cpufreq_governor=performance |
>> | | mode=thread |
>> | | nr_task=100% |
>> | | test=eventfd1 |
>> | | ucode=0x5003006 |
>> +------------------+-------------------------------------------------------------------------------------+
>>
>>
>> If you fix the issue, kindly add following tag
>> Reported-by: kernel test robot <[email protected]>
>>
>
> Gabriel,
>
> It looks like my change throws away much of the performance gain for
> small IO on pipes without any watches that was achieved by commit
> 71d734103edf ("fsnotify: Rearrange fast path to minimise overhead
> when there is no watcher").
>
> I think the way to fix it is to lift the optimization in __fsnotify()
> to the fsnotify_parent() inline wrapper as Mel considered doing
> but was not sure it was worth the effort at the time.
>
> It's not completely trivial. I think it requires setting a flag
> MNT_FSNOTIFY_WATCHED when there are watches on the
> vfsmount. I will look into it.
Amir,
Since this patch is a clean up, would you mind if I drop it from my
series and base my work on top of mainline? Eventually, we can rebase
this patch, when the performance issue is addressed.
I ask because I'm about to send a v5 and I'm not sure if I should wait
to have this fixed.
--
Gabriel Krisman Bertazi
On Sat, Jul 31, 2021 at 10:51 PM Gabriel Krisman Bertazi
<[email protected]> wrote:
>
> Amir Goldstein <[email protected]> writes:
>
> > On Sat, Jul 31, 2021 at 9:20 AM kernel test robot <[email protected]> wrote:
> >>
> >>
> >>
> >> Greeting,
> >>
> >> FYI, we noticed a -3.3% regression of unixbench.score due to commit:
> >>
> >>
> >> commit: 4c40d6efc8b22b88a45c335ffd6d25b55d769f5b ("[PATCH v4 08/16] fsnotify: pass arguments of fsnotify() in struct fsnotify_event_info")
> >> url: https://github.com/0day-ci/linux/commits/Gabriel-Krisman-Bertazi/File-system-wide-monitoring/20210721-001444
> >> base: https://git.kernel.org/cgit/linux/kernel/git/jack/linux-fs.git fsnotify
> >>
> >> in testcase: unixbench
> >> on test machine: 96 threads 2 sockets Intel(R) Xeon(R) CPU @ 2.30GHz with 128G memory
> >> with following parameters:
> >>
> >> runtime: 300s
> >> nr_task: 1
> >> test: pipe
> >> cpufreq_governor: performance
> >> ucode: 0x4003006
> >>
> >> test-description: UnixBench is the original BYTE UNIX benchmark suite aims to test performance of Unix-like system.
> >> test-url: https://github.com/kdlucas/byte-unixbench
> >>
> >> In addition to that, the commit also has significant impact on the following tests:
> >>
> >> +------------------+-------------------------------------------------------------------------------------+
> >> | testcase: change | will-it-scale: will-it-scale.per_thread_ops -1.3% regression |
> >> | test machine | 192 threads 4 sockets Intel(R) Xeon(R) Platinum 9242 CPU @ 2.30GHz with 192G memory |
> >> | test parameters | cpufreq_governor=performance |
> >> | | mode=thread |
> >> | | nr_task=100% |
> >> | | test=eventfd1 |
> >> | | ucode=0x5003006 |
> >> +------------------+-------------------------------------------------------------------------------------+
> >>
> >>
> >> If you fix the issue, kindly add following tag
> >> Reported-by: kernel test robot <[email protected]>
> >>
> >
> > Gabriel,
> >
> > It looks like my change throws away much of the performance gain for
> > small IO on pipes without any watches that was achieved by commit
> > 71d734103edf ("fsnotify: Rearrange fast path to minimise overhead
> > when there is no watcher").
> >
> > I think the way to fix it is to lift the optimization in __fsnotify()
> > to the fsnotify_parent() inline wrapper as Mel considered doing
> > but was not sure it was worth the effort at the time.
> >
> > It's not completely trivial. I think it requires setting a flag
> > MNT_FSNOTIFY_WATCHED when there are watches on the
> > vfsmount. I will look into it.
>
> Amir,
>
> Since this patch is a clean up, would you mind if I drop it from my
> series and base my work on top of mainline? Eventually, we can rebase
> this patch, when the performance issue is addressed.
>
> I ask because I'm about to send a v5 and I'm not sure if I should wait
> to have this fixed.
I guess you mean that you want to add the sb to fsnotify() args list.
I don't mind, it's up to Jan.
Thanks,
Amir.
On Sun 01-08-21 09:32:40, Amir Goldstein wrote:
> On Sat, Jul 31, 2021 at 10:51 PM Gabriel Krisman Bertazi
> <[email protected]> wrote:
> >
> > Amir Goldstein <[email protected]> writes:
> >
> > > On Sat, Jul 31, 2021 at 9:20 AM kernel test robot <[email protected]> wrote:
> > >>
> > >>
> > >>
> > >> Greeting,
> > >>
> > >> FYI, we noticed a -3.3% regression of unixbench.score due to commit:
> > >>
> > >>
> > >> commit: 4c40d6efc8b22b88a45c335ffd6d25b55d769f5b ("[PATCH v4 08/16] fsnotify: pass arguments of fsnotify() in struct fsnotify_event_info")
> > >> url: https://github.com/0day-ci/linux/commits/Gabriel-Krisman-Bertazi/File-system-wide-monitoring/20210721-001444
> > >> base: https://git.kernel.org/cgit/linux/kernel/git/jack/linux-fs.git fsnotify
> > >>
> > >> in testcase: unixbench
> > >> on test machine: 96 threads 2 sockets Intel(R) Xeon(R) CPU @ 2.30GHz with 128G memory
> > >> with following parameters:
> > >>
> > >> runtime: 300s
> > >> nr_task: 1
> > >> test: pipe
> > >> cpufreq_governor: performance
> > >> ucode: 0x4003006
> > >>
> > >> test-description: UnixBench is the original BYTE UNIX benchmark suite aims to test performance of Unix-like system.
> > >> test-url: https://github.com/kdlucas/byte-unixbench
> > >>
> > >> In addition to that, the commit also has significant impact on the following tests:
> > >>
> > >> +------------------+-------------------------------------------------------------------------------------+
> > >> | testcase: change | will-it-scale: will-it-scale.per_thread_ops -1.3% regression |
> > >> | test machine | 192 threads 4 sockets Intel(R) Xeon(R) Platinum 9242 CPU @ 2.30GHz with 192G memory |
> > >> | test parameters | cpufreq_governor=performance |
> > >> | | mode=thread |
> > >> | | nr_task=100% |
> > >> | | test=eventfd1 |
> > >> | | ucode=0x5003006 |
> > >> +------------------+-------------------------------------------------------------------------------------+
> > >>
> > >>
> > >> If you fix the issue, kindly add following tag
> > >> Reported-by: kernel test robot <[email protected]>
> > >>
> > >
> > > Gabriel,
> > >
> > > It looks like my change throws away much of the performance gain for
> > > small IO on pipes without any watches that was achieved by commit
> > > 71d734103edf ("fsnotify: Rearrange fast path to minimise overhead
> > > when there is no watcher").
> > >
> > > I think the way to fix it is to lift the optimization in __fsnotify()
> > > to the fsnotify_parent() inline wrapper as Mel considered doing
> > > but was not sure it was worth the effort at the time.
> > >
> > > It's not completely trivial. I think it requires setting a flag
> > > MNT_FSNOTIFY_WATCHED when there are watches on the
> > > vfsmount. I will look into it.
> >
> > Amir,
> >
> > Since this patch is a clean up, would you mind if I drop it from my
> > series and base my work on top of mainline? Eventually, we can rebase
> > this patch, when the performance issue is addressed.
> >
> > I ask because I'm about to send a v5 and I'm not sure if I should wait
> > to have this fixed.
>
> I guess you mean that you want to add the sb to fsnotify() args list.
> I don't mind, it's up to Jan.
Yeah, no problem with that from my side either.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR