| Message ID | 20240317184154.1200192-1-amir73il@gmail.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Further reduce overhead of fsnotify permission hooks | expand |
On Sun, Mar 17, 2024 at 8:42 PM Amir Goldstein <amir73il@gmail.com> wrote: > > Jan, > > Commit 082fd1ea1f98 ("fsnotify: optimize the case of no parent watcher") > has reduced the CPU overhead of fsnotify hooks, but we can further > reduce the overhead of permission event hooks, by avoiding the call to > fsnotify() and fsnotify_parent() altogether when there are no permission > event watchers on the sb. > > The main motivation for this work was to avoid the overhead that was > reported by kernel test robot on the patch that adds the upcoming > per-content event hooks (i.e. FS_PRE_ACCESS/FS_PRE_MODIFY). > > Kernel test robot has confirmed that with this series, the addition of > pre-conent fsnotify hooks does not result in any regression [1]. > Kernet test robot has also reported performance improvements in some > workloads compared to upstream on an earlier version of this series, but > still waiting for the final results. FYI, the results are back [1] and they show clear improvement in two workloads by this patch set as expected when the permission hooks are practically being disabled: ---------------- --------------------------- --------------------------- --------------------------- %stddev %change %stddev %change %stddev %change %stddev \ | \ | \ | \ 1.338e+08 +0.4% 1.344e+08 +0.3% 1.342e+08 +5.8% 1.416e+08 unixbench.throughput 5.759e+10 +0.4% 5.784e+10 +0.2% 5.772e+10 +5.8% 6.094e+10 unixbench.workload Thanks, Amir. [1] https://lore.kernel.org/all/Zfj3wxDHolB1qCGO@xsang-OptiPlex-9020/
On Tue 19-03-24 11:59:11, Amir Goldstein wrote: > On Sun, Mar 17, 2024 at 8:42 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > Jan, > > > > Commit 082fd1ea1f98 ("fsnotify: optimize the case of no parent watcher") > > has reduced the CPU overhead of fsnotify hooks, but we can further > > reduce the overhead of permission event hooks, by avoiding the call to > > fsnotify() and fsnotify_parent() altogether when there are no permission > > event watchers on the sb. > > > > The main motivation for this work was to avoid the overhead that was > > reported by kernel test robot on the patch that adds the upcoming > > per-content event hooks (i.e. FS_PRE_ACCESS/FS_PRE_MODIFY). > > > > Kernel test robot has confirmed that with this series, the addition of > > pre-conent fsnotify hooks does not result in any regression [1]. > > Kernet test robot has also reported performance improvements in some > > workloads compared to upstream on an earlier version of this series, but > > still waiting for the final results. > > FYI, the results are back [1] and they show clear improvement in two > workloads by this patch set as expected when the permission hooks > are practically being disabled: Patches are now merged into my tree. Honza
On Thu, Apr 4, 2024 at 5:34 PM Jan Kara <jack@suse.cz> wrote: > > On Tue 19-03-24 11:59:11, Amir Goldstein wrote: > > On Sun, Mar 17, 2024 at 8:42 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > Jan, > > > > > > Commit 082fd1ea1f98 ("fsnotify: optimize the case of no parent watcher") > > > has reduced the CPU overhead of fsnotify hooks, but we can further > > > reduce the overhead of permission event hooks, by avoiding the call to > > > fsnotify() and fsnotify_parent() altogether when there are no permission > > > event watchers on the sb. > > > > > > The main motivation for this work was to avoid the overhead that was > > > reported by kernel test robot on the patch that adds the upcoming > > > per-content event hooks (i.e. FS_PRE_ACCESS/FS_PRE_MODIFY). > > > > > > Kernel test robot has confirmed that with this series, the addition of > > > pre-conent fsnotify hooks does not result in any regression [1]. > > > Kernet test robot has also reported performance improvements in some > > > workloads compared to upstream on an earlier version of this series, but > > > still waiting for the final results. > > > > FYI, the results are back [1] and they show clear improvement in two > > workloads by this patch set as expected when the permission hooks > > are practically being disabled: > > Patches are now merged into my tree. Yay! If possible, please also push fsnotify branch. Thanks, Amir.
On Thu 04-04-24 17:41:18, Amir Goldstein wrote: > On Thu, Apr 4, 2024 at 5:34 PM Jan Kara <jack@suse.cz> wrote: > > > > On Tue 19-03-24 11:59:11, Amir Goldstein wrote: > > > On Sun, Mar 17, 2024 at 8:42 PM Amir Goldstein <amir73il@gmail.com> wrote: > > > > > > > > Jan, > > > > > > > > Commit 082fd1ea1f98 ("fsnotify: optimize the case of no parent watcher") > > > > has reduced the CPU overhead of fsnotify hooks, but we can further > > > > reduce the overhead of permission event hooks, by avoiding the call to > > > > fsnotify() and fsnotify_parent() altogether when there are no permission > > > > event watchers on the sb. > > > > > > > > The main motivation for this work was to avoid the overhead that was > > > > reported by kernel test robot on the patch that adds the upcoming > > > > per-content event hooks (i.e. FS_PRE_ACCESS/FS_PRE_MODIFY). > > > > > > > > Kernel test robot has confirmed that with this series, the addition of > > > > pre-conent fsnotify hooks does not result in any regression [1]. > > > > Kernet test robot has also reported performance improvements in some > > > > workloads compared to upstream on an earlier version of this series, but > > > > still waiting for the final results. > > > > > > FYI, the results are back [1] and they show clear improvement in two > > > workloads by this patch set as expected when the permission hooks > > > are practically being disabled: > > > > Patches are now merged into my tree. > > Yay! > If possible, please also push fsnotify branch. Done. Honza
Jan, Commit 082fd1ea1f98 ("fsnotify: optimize the case of no parent watcher") has reduced the CPU overhead of fsnotify hooks, but we can further reduce the overhead of permission event hooks, by avoiding the call to fsnotify() and fsnotify_parent() altogether when there are no permission event watchers on the sb. The main motivation for this work was to avoid the overhead that was reported by kernel test robot on the patch that adds the upcoming per-content event hooks (i.e. FS_PRE_ACCESS/FS_PRE_MODIFY). Kernel test robot has confirmed that with this series, the addition of pre-conent fsnotify hooks does not result in any regression [1]. Kernet test robot has also reported performance improvements in some workloads compared to upstream on an earlier version of this series, but still waiting for the final results. For now, as you requested, I am posting this series for review. Thanks, Amir. [1] https://lore.kernel.org/all/Zc7KmlQ1cYVrPMQ+@xsang-OptiPlex-9020/ [2] https://lore.kernel.org/all/202403141505.807a722b-oliver.sang@intel.com/ Amir Goldstein (10): fsnotify: rename fsnotify_{get,put}_sb_connectors() fsnotify: create helpers to get sb and connp from object fsnotify: create a wrapper fsnotify_find_inode_mark() fanotify: merge two checks regarding add of ignore mark fsnotify: pass object pointer and type to fsnotify mark helpers fsnotify: create helper fsnotify_update_sb_watchers() fsnotify: lazy attach fsnotify_sb_info state to sb fsnotify: move s_fsnotify_connectors into fsnotify_sb_info fsnotify: use an enum for group priority constants fsnotify: optimize the case of no permission event watchers fs/nfsd/filecache.c | 4 +- fs/notify/dnotify/dnotify.c | 4 +- fs/notify/fanotify/fanotify_user.c | 141 +++++++---------------- fs/notify/fsnotify.c | 23 +++- fs/notify/fsnotify.h | 38 +++++-- fs/notify/inotify/inotify_user.c | 2 +- fs/notify/mark.c | 173 ++++++++++++++++++++++------- include/linux/fs.h | 14 +-- include/linux/fsnotify.h | 21 +++- include/linux/fsnotify_backend.h | 93 +++++++++++----- kernel/audit_tree.c | 2 +- kernel/audit_watch.c | 2 +- 12 files changed, 314 insertions(+), 203 deletions(-)