From: Jens Axboe <axboe@kernel.dk>
To: Christian Brauner <brauner@kernel.org>,
Amir Goldstein <amir73il@gmail.com>,
"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>
Subject: [PATCH] Revert "fsnotify: optionally pass access range in file permission hooks"
Date: Tue, 9 Jan 2024 09:08:40 -0700 [thread overview]
Message-ID: <53682ece-f0e7-48de-9a1c-879ee34b0449@kernel.dk> (raw)
This reverts commit d9e5d31084b024734e64307521414ef0ae1d5333.
This commit added an extra fsnotify call from rw_verify_area(), which
can be a very hot path. In my testing, this adds roughly 5-6% extra
overhead per IO, which is quite a lot. As a result, throughput of
said test also drops by 5-6%, as it is CPU bound. Looking at perf,
it's apparent why:
3.36% [kernel.vmlinux] [k] fsnotify
2.32% [kernel.vmlinux] [k] __fsnotify_paren
which directly correlates with performance lost.
As the rationale for this patch isn't very strong, revert this commit
for now and reclaim the performance.
Signed-off-by: Jens Axboe <axboe@kernel.dk>
---
fs/open.c | 4 ----
fs/read_write.c | 10 ++--------
fs/readdir.c | 4 ----
fs/remap_range.c | 8 +-------
include/linux/fsnotify.h | 13 ++-----------
security/security.c | 8 +++++++-
6 files changed, 12 insertions(+), 35 deletions(-)
diff --git a/fs/open.c b/fs/open.c
index a84d21e55c39..f4f157405d1e 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -304,10 +304,6 @@ int vfs_fallocate(struct file *file, int mode, loff_t offset, loff_t len)
if (ret)
return ret;
- ret = fsnotify_file_area_perm(file, MAY_WRITE, &offset, len);
- if (ret)
- return ret;
-
if (S_ISFIFO(inode->i_mode))
return -ESPIPE;
diff --git a/fs/read_write.c b/fs/read_write.c
index d4c036e82b6c..e3abf603eaaf 100644
--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -354,9 +354,6 @@ SYSCALL_DEFINE5(llseek, unsigned int, fd, unsigned long, offset_high,
int rw_verify_area(int read_write, struct file *file, const loff_t *ppos, size_t count)
{
- int mask = read_write == READ ? MAY_READ : MAY_WRITE;
- int ret;
-
if (unlikely((ssize_t) count < 0))
return -EINVAL;
@@ -374,11 +371,8 @@ int rw_verify_area(int read_write, struct file *file, const loff_t *ppos, size_t
}
}
- ret = security_file_permission(file, mask);
- if (ret)
- return ret;
-
- return fsnotify_file_area_perm(file, mask, ppos, count);
+ return security_file_permission(file,
+ read_write == READ ? MAY_READ : MAY_WRITE);
}
EXPORT_SYMBOL(rw_verify_area);
diff --git a/fs/readdir.c b/fs/readdir.c
index 278bc0254732..c8c46e294431 100644
--- a/fs/readdir.c
+++ b/fs/readdir.c
@@ -96,10 +96,6 @@ int iterate_dir(struct file *file, struct dir_context *ctx)
if (res)
goto out;
- res = fsnotify_file_perm(file, MAY_READ);
- if (res)
- goto out;
-
res = down_read_killable(&inode->i_rwsem);
if (res)
goto out;
diff --git a/fs/remap_range.c b/fs/remap_range.c
index f8c1120b8311..12131f2a6c9e 100644
--- a/fs/remap_range.c
+++ b/fs/remap_range.c
@@ -102,9 +102,7 @@ static int generic_remap_checks(struct file *file_in, loff_t pos_in,
static int remap_verify_area(struct file *file, loff_t pos, loff_t len,
bool write)
{
- int mask = write ? MAY_WRITE : MAY_READ;
loff_t tmp;
- int ret;
if (unlikely(pos < 0 || len < 0))
return -EINVAL;
@@ -112,11 +110,7 @@ static int remap_verify_area(struct file *file, loff_t pos, loff_t len,
if (unlikely(check_add_overflow(pos, len, &tmp)))
return -EINVAL;
- ret = security_file_permission(file, mask);
- if (ret)
- return ret;
-
- return fsnotify_file_area_perm(file, mask, &pos, len);
+ return security_file_permission(file, write ? MAY_WRITE : MAY_READ);
}
/*
diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
index 11e6434b8e71..0a9d6a8a747a 100644
--- a/include/linux/fsnotify.h
+++ b/include/linux/fsnotify.h
@@ -101,10 +101,9 @@ static inline int fsnotify_file(struct file *file, __u32 mask)
}
/*
- * fsnotify_file_area_perm - permission hook before access to file range
+ * fsnotify_file_perm - permission hook before file access
*/
-static inline int fsnotify_file_area_perm(struct file *file, int perm_mask,
- const loff_t *ppos, size_t count)
+static inline int fsnotify_file_perm(struct file *file, int perm_mask)
{
__u32 fsnotify_mask = FS_ACCESS_PERM;
@@ -121,14 +120,6 @@ static inline int fsnotify_file_area_perm(struct file *file, int perm_mask,
return fsnotify_file(file, fsnotify_mask);
}
-/*
- * fsnotify_file_perm - permission hook before file access
- */
-static inline int fsnotify_file_perm(struct file *file, int perm_mask)
-{
- return fsnotify_file_area_perm(file, perm_mask, NULL, 0);
-}
-
/*
* fsnotify_open_perm - permission hook before file open
*/
diff --git a/security/security.c b/security/security.c
index 2a7fc7881cbc..d7f3703c5905 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2580,7 +2580,13 @@ int security_kernfs_init_security(struct kernfs_node *kn_dir,
*/
int security_file_permission(struct file *file, int mask)
{
- return call_int_hook(file_permission, 0, file, mask);
+ int ret;
+
+ ret = call_int_hook(file_permission, 0, file, mask);
+ if (ret)
+ return ret;
+
+ return fsnotify_file_perm(file, mask);
}
/**
--
2.43.0
--
Jens Axboe
next reply other threads:[~2024-01-09 16:08 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-09 16:08 Jens Axboe [this message]
2024-01-09 17:30 ` [PATCH] Revert "fsnotify: optionally pass access range in file permission hooks" Christian Brauner
2024-01-09 17:33 ` Jens Axboe
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53682ece-f0e7-48de-9a1c-879ee34b0449@kernel.dk \
--to=axboe@kernel.dk \
--cc=amir73il@gmail.com \
--cc=brauner@kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).