- 28 Jul, 2020 1 commit
-
-
Jan Kara authored
When merging name events, fsids of the two involved events have to match. Otherwise we could merge events from two different filesystems and thus effectively loose the second event. Backporting note: Although the commit cacfb956 introducing this bug was merged for 5.7, the relevant code didn't get used in the end until 7e8283af ("fanotify: report parent fid + name + child fid") which will be merged with this patch. So there's no need for backporting this. Fixes: cacfb956 ("fanotify: record name info for FAN_DIR_MODIFY event") Reported-by:
Amir Goldstein <amir73il@gmail.com> Signed-off-by:
Jan Kara <jack@suse.cz>
-
- 27 Jul, 2020 27 commits
-
-
Amir Goldstein authored
The method handle_event() grew a lot of complexity due to the design of fanotify and merging of ignore masks. Most backends do not care about this complex functionality, so we can hide this complexity from them. Introduce a method handle_inode_event() that serves those backends and passes a single inode mark and less arguments. This change converts all backends except fanotify and inotify to use the simplified handle_inode_event() method. In pricipal, inotify could have also used the new method, but that would require passing more arguments on the simple helper (data, data_type, cookie), so we leave it with the handle_event() method. Link: https://lore.kernel.org/r/20200722125849.17418-9-amir73il@gmail.comSuggested-by:
-
Amir Goldstein authored
Add support for FAN_REPORT_FID | FAN_REPORT_DIR_FID. Internally, it is implemented as a private case of reporting both parent and child fids and name, the parent and child fids are recorded in a variable length fanotify_name_event, but there is no name. It should be noted that directory modification events are recorded in fixed size fanotify_fid_event when not reporting name, just like with group flags FAN_REPORT_FID. Link: https://lore.kernel.org/r/20200716084230.30611-23-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
For a group with fanotify_init() flag FAN_REPORT_DFID_NAME, the parent fid and name are reported for events on non-directory objects with an info record of type FAN_EVENT_INFO_TYPE_DFID_NAME. If the group also has the init flag FAN_REPORT_FID, the child fid is also reported with another info record that follows the first info record. The second info record is the same info record that would have been reported to a group with only FAN_REPORT_FID flag. When the child fid needs to be recorded, the variable size struct fanotify_name_event is preallocated with enough space to store the child fh between the dir fh and the name. Link: https://lore.kernel.org/r/20200716084230.30611-22-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
Introduce a new fanotify_init() flag FAN_REPORT_NAME. It requires the flag FAN_REPORT_DIR_FID and there is a constant for setting both flags named FAN_REPORT_DFID_NAME. For a group with flag FAN_REPORT_NAME, the parent fid and name are reported for directory entry modification events (create/detete/move) and for events on non-directory objects. Events on directories themselves are reported with their own fid and "." as the name. The parent fid and name are reported with an info record of type FAN_EVENT_INFO_TYPE_DFID_NAME, similar to the way that parent fid is reported with into type FAN_EVENT_INFO_TYPE_DFID, but with an appended null terminated name string. Link: https://lore.kernel.org/r/20200716084230.30611-21-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
In a group with flag FAN_REPORT_DIR_FID, when adding an inode mark with FAN_EVENT_ON_CHILD, events on non-directory children are reported with the fid of the parent. When adding a filesystem or mount mark or mark on a non-dir inode, we want to report events that are "possible on child" (e.g. open/close) also with fid of the parent, as if the victim inode's parent is interested in events "on child". Some events, currently only FAN_MOVE_SELF, should be reported to a sb/mount/non-dir mark with parent fid even though they are not reported to a watching parent. To get the desired behavior we set the flag FAN_EVENT_ON_CHILD on all the sb/mount/non-dir mark masks in a group with FAN_REPORT_DIR_FID. Link: https://lore.kernel.org/r/20200716084230.30611-20-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
For now, the flag is mutually exclusive with FAN_REPORT_FID. Events include a single info record of type FAN_EVENT_INFO_TYPE_DFID with a directory file handle. For now, events are only reported for: - Directory modification events - Events on children of a watching directory - Events on directory objects Soon, we will add support for reporting the parent directory fid for events on non-directories with filesystem/mount mark and support for reporting both parent directory fid and child fid. Link: https://lore.kernel.org/r/20200716084230.30611-19-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
Remove the unneeded check for positive source dentry in fsnotify_move(). fsnotify_move() hook is mostly called from vfs_rename() under lock_rename() and vfs_rename() starts with may_delete() test that verifies positive source dentry. The only other caller of fsnotify_move() - debugfs_rename() also verifies positive source. Link: https://lore.kernel.org/r/20200716084230.30611-17-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
Similar to events "on child" to watching directory, send event with parent/name info if sb/mount/non-dir marks are interested in parent/name info. The FS_EVENT_ON_CHILD flag can be set on sb/mount/non-dir marks to specify interest in parent/name info for events on non-directory inodes. Events on "orphan" children (disconnected dentries) are sent without parent/name info. Events on directories are sent with parent/name info only if the parent directory is watching. After this change, even groups that do not subscribe to events on children could get an event with mark iterator type TYPE_CHILD and without mark iterator type TYPE_INODE if fanotify has marks on the same objects. dnotify and inotify event handlers can already cope with that situation. audit does not subscribe to events that are possible on child, so won't get to this situation. nfsd does not access the marks iterator from its event handler at the moment, so it is not affected. This is a bit too fragile, so we should prepare all groups to cope with mark type TYPE_CHILD preferably using a generic helper. Link: https://lore.kernel.org/r/20200716084230.30611-16-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
The audit group marks mask does not contain any events possible on a child so setting the flag FS_EVENT_ON_CHILD in the mask is counter productive. It may lead to the undesired outcome of setting the dentry flag DCACHE_FSNOTIFY_PARENT_WATCHED on a directory inode even though it is not watching children, because the audit mark contribute the flag FS_EVENT_ON_CHILD to the inode's fsnotify_mask and another mark could be contributing an event that is possible on child to the inode's mask. Furthermore in the following patches we want to use FS_EVENT_ON_CHILD for non-dir inodes for other purposes so stop using the flag. Link: https://lore.kernel.org/r/20200722125849.17418-4-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
FS_EVENT_ON_CHILD has currently no meaning for non-dir inode marks. In the following patches we want to use that bit to mean that mark's notification group cares about parent and name information. So stop setting FS_EVENT_ON_CHILD for non-dir marks. Link: https://lore.kernel.org/r/20200722125849.17418-3-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
The arguments of fsnotify() are overloaded and mean different things for different event types. Replace the to_tell argument with separate arguments @dir and @inode, because we may be sending to both dir and child. Using the @data argument to pass the child is not enough, because dirent events pass this argument (for audit), but we do not report to child. Document the new fsnotify() function argumenets. Link: https://lore.kernel.org/r/20200722125849.17418-7-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
Simple helper to consolidate biolerplate code. Link: https://lore.kernel.org/r/20200722125849.17418-5-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
Instead of calling fsnotify() twice, once with parent inode and once with child inode, if event should be sent to parent inode, send it with both parent and child inodes marks in object type iterator and call the backend handle_event() callback only once. The parent inode is assigned to the standard "inode" iterator type and the child inode is assigned to the special "child" iterator type. In that case, the bit FS_EVENT_ON_CHILD will be set in the event mask, the dir argument to handle_event will be the parent inode, the file_name argument to handle_event is non NULL and refers to the name of the child and the child inode can be accessed with fsnotify_data_inode(). This will allow fanotify to make decisions based on child or parent's ignored mask. For example, when a parent is interested in a specific event on its children, but a specific child wishes to ignore this event, the event will not be reported. This is not what happens with current code, but according to man page, it is the expected behavior. Link: https://lore.kernel.org/r/20200716084230.30611-15-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
fsnotify usually calls inotify_handle_event() once for watching parent to report event with child's name and once for watching child to report event without child's name. Do the same thing with a single callback instead of two callbacks when marks iterator contains both inode and child entries. Link: https://lore.kernel.org/r/20200716084230.30611-13-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
For some events (e.g. DN_ATTRIB on sub-directory) fsnotify may call dnotify_handle_event() once for watching parent and once again for the watching sub-directory. Do the same thing with a single callback instead of two callbacks when marks iterator contains both inode and child entries. Link: https://lore.kernel.org/r/20200716084230.30611-12-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
The fanotify_fh struct has an inline buffer of size 12 which is enough to store the most common local filesystem file handles (e.g. ext4, xfs). For file handles that do not fit in the inline buffer (e.g. btrfs), an external buffer is allocated to store the file handle. When allocating a variable size fanotify_name_event, there is no point in allocating also an external fh buffer when file handle does not fit in the inline buffer. Check required size for encoding fh, preallocate an event buffer sufficient to contain both file handle and name and store the name after the file handle. At this time, when not reporting name in event, we still allocate the fixed size fanotify_fid_event and an external buffer for large file handles, but fanotify_alloc_name_event() has already been prepared to accept a NULL file_name. Link: https://lore.kernel.org/r/20200716084230.30611-11-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
An fanotify event name is always recorded relative to a dir fh. Encapsulate the name_len member of fanotify_name_event in a new struct fanotify_info, which describes the parceling of the variable size buffer of an fanotify_name_event. The dir_fh member of fanotify_name_event is renamed to _dir_fh and is not accessed directly, but via the fanotify_info_dir_fh() accessor. Although the dir_fh len information is already available in struct fanotify_fh, we store it also in dif_fh_totlen member of fanotify_info, including the size of fanotify_fh header, so we know the offset of the name in the buffer without looking inside the dir_fh. We also add a file_fh_totlen member to allow packing another file handle in the variable size buffer after the dir_fh and before the name. We are going to use that space to store the child fid. Link: https://lore.kernel.org/r/20200716084230.30611-10-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
The object type iterator is used to collect all the marks of a specific group that have interest in an event. It is used by fanotify to get a single handle_event callback when an event has a match to either of inode/sb/mount marks of the group. The nature of fsnotify events is that they are associated with at most one sb at most one mount and at most one inode. When a parent and child are both watching, two events are sent to backend, one associated to parent inode and one associated to the child inode. This results in duplicate events in fanotify, which usually get merged before user reads them, but this is sub-optimal. It would be better if the same event is sent to backend with an object type iterator that has both the child inode and its parent, and let the backend decide if the event should be reported once (fanotify) or twice (inotify). Link: https://lore.kernel.org/r/20200716084230.30611-9-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
Up to now, fanotify allowed to set the FAN_EVENT_ON_CHILD flag on sb/mount marks and non-directory inode mask, but the flag was ignored. Mask out the flag if it is provided by user on sb/mount/non-dir marks and define it as an implicit flag that cannot be removed by user. This flag is going to be used internally to request for events with parent and name info. Link: https://lore.kernel.org/r/20200716084230.30611-8-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
So far, all flags that can be set in an fanotify mark mask can be set explicitly by a call to fanotify_mark(2). Prepare for defining implicit event flags that cannot be set by user with fanotify_mark(2), similar to how inotify/dnotify implicitly set the FS_EVENT_ON_CHILD flag. Implicit event flags cannot be removed by user and mark gets destroyed when only implicit event flags remain in the mask. Link: https://lore.kernel.org/r/20200716084230.30611-7-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
The special event flags (FAN_ONDIR, FAN_EVENT_ON_CHILD) never had any meaning in ignored mask. Mask them out explicitly. Link: https://lore.kernel.org/r/20200716084230.30611-6-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
As preparation for new flags that report fids, define a bit set of flags for a group reporting fids, currently containing the only bit FAN_REPORT_FID. Link: https://lore.kernel.org/r/20200716084230.30611-5-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
In fanotify_encode_fh(), both cases of NULL inode and failure to encode ended up with fh type FILEID_INVALID. Distiguish the case of NULL inode, by setting fh type to FILEID_ROOT. This is just a semantic difference at this point. Remove stale comment and unneeded check from fid event compare helpers. Link: https://lore.kernel.org/r/20200716084230.30611-4-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
An event on directory should never be merged with an event on non-directory regardless of the event struct type. This change has no visible effect, because currently, with struct fanotify_path_event, the relevant events will not be merged because event path of dir will be different than event path of non-dir. Link: https://lore.kernel.org/r/20200716084230.30611-3-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
In fanotify_group_event_mask() there is logic in place to make sure we are not going to handle an event with no type and just FAN_ONDIR flag. Generalize this logic to any FANOTIFY_EVENT_FLAGS. There is only one more flag in this group at the moment - FAN_EVENT_ON_CHILD. We never report it to user, but we do pass it in to fanotify_alloc_event() when group is reporting fid as indication that event happened on child. We will have use for this indication later on. Link: https://lore.kernel.org/r/20200716084230.30611-2-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
It was never enabled in uapi and its functionality is about to be superseded by events FAN_CREATE, FAN_DELETE, FAN_MOVE with group flag FAN_REPORT_NAME. Keep a place holder variable name_event instead of removing the name recording code since it will be used by the new events. Link: https://lore.kernel.org/r/20200708111156.24659-17-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
The 'inode' argument to handle_event(), sometimes referred to as 'to_tell' is somewhat obsolete. It is a remnant from the times when a group could only have an inode mark associated with an event. We now pass an iter_info array to the callback, with all marks associated with an event. Most backends ignore this argument, with two exceptions: 1. dnotify uses it for sanity check that event is on directory 2. fanotify uses it to report fid of directory on directory entry modification events Remove the 'inode' argument and add a 'dir' argument. The callback function signature is deliberately changed, because the meaning of the argument has changed and the arguments have been documented. The 'dir' argument is set to when 'file_name' is specified and it is referring to the directory that the 'file_name' entry belongs to. Signed-off-by:
-
- 15 Jul, 2020 9 commits
-
-
Amir Goldstein authored
Break up fanotify_alloc_event() into helpers by event struct type. Suggested-by:
-
Amir Goldstein authored
The special overflow event is allocated as struct fanotify_path_event, but with a null path. Use a special event type to identify the overflow event, so the helper fanotify_has_event_path() will always indicate a non null path. Allocating the overflow event doesn't need any of the fancy stuff in fanotify_alloc_event(), so create a simplified helper for allocating the overflow event. There is also no need to store and report the pid with an overflow event. Link: https://lore.kernel.org/r/20200708111156.24659-7-amir73il@gmail.comSuggested-by:
-
Amir Goldstein authored
inotify's event->wd is the object identifier. Compare that instead of the common fsnotidy event objectid, so we can get rid of the objectid field later. Link: https://lore.kernel.org/r/20200708111156.24659-6-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
When creating an FS_MODIFY event on inode itself (not on parent) the file_name argument should be NULL. The change to send a non NULL name to inode itself was done on purpuse as part of another commit, as Tejun writes: "...While at it, supply the target file name to fsnotify() from kernfs_node->name.". But this is wrong practice and inconsistent with inotify behavior when watching a single file. When a child is being watched (as opposed to the parent directory) the inotify event should contain the watch descriptor, but not the file name. Fixes: df6a58c5 ("kernfs: don't depend on d_find_any_alias()...") Link: https://lore.kernel.org/r/20200708111156.24659-5-amir73il@gmail.comAcked-by:
Tejun Heo <tj@kernel.org> Acked-by:
Greg Kroah-Hartman <gregkh@linuxfoundation.org> Signed-off-by:
-
Amir Goldstein authored
The inode argument to handle_event() is about to become obsolete. Link: https://lore.kernel.org/r/20200708111156.24659-4-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
Return non const inode pointer from fsnotify_data_inode(). None of the fsnotify hooks pass const inode pointer as data and callers often need to cast to a non const pointer. Link: https://lore.kernel.org/r/20200708111156.24659-3-amir73il@gmail.comSigned-off-by:
-
Amir Goldstein authored
All (two) callers of fsnotify_parent() also call fsnotify() to notify the child inode. Move the second fsnotify() call into fsnotify_parent(). This will allow more flexibility in making decisions about which of the two event falvors should be sent. Using 'goto notify_child' in the inline helper seems a bit strange, but it mimics the code in __fsnotify_parent() for clarity and the goto pattern will become less strage after following patches are applied. Link: https://lore.kernel.org/r/20200708111156.24659-2-amir73il@gmail.comSigned-off-by:
-
Mel Gorman authored
The fsnotify paths are trivial to hit even when there are no watchers and they are surprisingly expensive. For example, every successful vfs_write() hits fsnotify_modify which calls both fsnotify_parent and fsnotify unless FMODE_NONOTIFY is set which is an internal flag invisible to userspace. As it stands, fsnotify_parent is a guaranteed functional call even if there are no watchers and fsnotify() does a substantial amount of unnecessary work before it checks if there are any watchers. A perf profile showed that applying mnt->mnt_fsnotify_mask in fnotify() was almost half of the total samples taken in that function during a test. This patch rearranges the fast paths to reduce the amount of work done when there are no watchers. The test motivating this was "perf bench sched messaging --pipe". Despite the fact the pipes are anonymous, fsnotify is still called a lot and the overhead is noticeable even though it's completely pointless. It's likely the overhead is negligible for real IO so this is an extreme example. This is a comparison of hackbench using processes and pipes on a 1-socket machine with 8 CPU threads without fanotify watchers. 5.7.0 5.7.0 vanilla fastfsnotify-v1r1 Amean 1 0.4837 ( 0.00%) 0.4630 * 4.27%* Amean 3 1.5447 ( 0.00%) 1.4557 ( 5.76%) Amean 5 2.6037 ( 0.00%) 2.4363 ( 6.43%) Amean 7 3.5987 ( 0.00%) 3.4757 ( 3.42%) Amean 12 5.8267 ( 0.00%) 5.6983 ( 2.20%) Amean 18 8.4400 ( 0.00%) 8.1327 ( 3.64%) Amean 24 11.0187 ( 0.00%) 10.0290 * 8.98%* Amean 30 13.1013 ( 0.00%) 12.8510 ( 1.91%) Amean 32 13.9190 ( 0.00%) 13.2410 ( 4.87%) 5.7.0 5.7.0 vanilla fastfsnotify-v1r1 Duration User 157.05 152.79 Duration System 1279.98 1219.32 Duration Elapsed 182.81 174.52 This is showing that the latencies are improved by roughly 2-9%. The variability is not shown but some of these results are within the noise as this workload heavily overloads the machine. That said, the system CPU usage is reduced by quite a bit so it makes sense to avoid the overhead even if it is a bit tricky to detect at times. A perf profile of just 1 group of tasks showed that 5.14% of samples taken were in either fsnotify() or fsnotify_parent(). With the patch, 2.8% of samples were in fsnotify, mostly function entry and the initial check for watchers. The check for watchers is complicated enough that inlining it may be controversial. [Amir] Slightly simplify with mnt_or_sb_mask => marks_mask Link: https://lore.kernel.org/r/20200708111156.24659-1-amir73il@gmail.comSigned-off-by:Mel Gorman <mgorman@techsingularity.net> Signed-off-by:
-
Jan Kara authored
When user provides large buffer for events and there are lots of events available, we can try to copy them all to userspace without scheduling which can softlockup the kernel (furthermore exacerbated by the contention on notification_lock). Add a scheduling point after copying each event. Note that usually the real underlying problem is the cost of fanotify event merging and the resulting contention on notification_lock but this is a cheap way to somewhat reduce the problem until we can properly address that. Reported-by:
Francesco Ruggeri <fruggeri@arista.com> Link: https://lore.kernel.org/lkml/20200714025417.A25EB95C0339@us180.sjc.aristanetworks.comReviewed-by:
-
- 08 Jul, 2020 3 commits
-
-
Randy Dunlap authored
Update Documentation for the gcc v4.9 upgrade requirement. Fixes: 5429ef62 ("compiler/gcc: Raise minimum GCC version for kernel builds to 4.8") Fixes: 6ec4476a ("Raise gcc version requirement to 4.9") Signed-off-by:
Randy Dunlap <rdunlap@infradead.org> Acked-by:
Jonathan Corbet <corbet@lwn.net> Signed-off-by:
Linus Torvalds <torvalds@linux-foundation.org>
-
git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/soundLinus Torvalds authored
Pull sound fixes from Takashi Iwai: "A collection of small, mostly device-specific fixes. The significant one is the regression fix for USB-audio implicit feedback devices due to the incorrect frame size calculation, which landed in 5.8 and stable trees. In addition, a few usual HD-audio and USB-audio quirks, Intel HDMI fixes, ASoC fsl and rt5682 fixes, as well as the fix in compress-offload partial drain operation" * tag 'sound-5.8-rc5' of git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound: ALSA: compress: fix partial_drain completion state ALSA: usb-audio: Add implicit feedback quirk for RTX6001 ALSA: usb-audio: add quirk for MacroSilicon MS2109 ALSA: hda/realtek: Enable headset mic of Acer Veriton N4660G with ALC269VC ALSA: hda/realtek: Enable headset mic of Acer C20-820 with ALC269VC ALSA: hda/realtek - Enable audio jacks of Acer vCopperbox with ALC269VC ALSA: hda/realtek - Fix Lenovo Thinkpad X1 Carbon 7th quirk subdevice id ALSA: hda/hdmi: improve debug traces for stream lookups ALSA: hda/hdmi: fix failures at PCM open on Intel ICL and later ALSA: opl3: fix infoleak in opl3 ALSA: usb-audio: Replace s/frame/packet/ where appropriate ALSA: usb-audio: Fix packet size calculation AsoC: amd: add missing snd- module prefix to the acp3x-rn driver kernel module ALSA: hda - let hs_mic be picked ahead of hp_mic ASoC: rt5682: fix the pop noise while OMTP type headset plugin ASoC: fsl_mqs: Fix unchecked return value for clk_prepare_enable ASoC: fsl_mqs: Don't check clock is NULL before calling clk API
-
Linus Torvalds authored
I realize that we fairly recently raised it to 4.8, but the fact is, 4.9 is a much better minimum version to target. We have a number of workarounds for actual bugs in pre-4.9 gcc versions (including things like internal compiler errors on ARM), but we also have some syntactic workarounds for lacking features. In particular, raising the minimum to 4.9 means that we can now just assume _Generic() exists, which is likely the much better replacement for a lot of very convoluted built-time magic with conditionals on sizeof and/or __builtin_choose_expr() with same_type() etc. Using _Generic also means that you will need to have a very recent version of 'sparse', but thats easy to build yourself, and much less of a hassle than some old gcc version can be. The latest (in a long string) of reasons for minimum compiler version upgrades was commit 5435f73d ("efi/x86: Fix build with gcc 4"). Ard points out that RHEL 7 uses gcc-4.8, but the people who stay back on old RHEL versions persumably also don't build their own kernels anyway. And maybe they should cross-built or just have a little side affair with a newer compiler? Acked-by:
Ard Biesheuvel <ardb@kernel.org> Acked-by:
Peter Zijlstra <peterz@infradead.org> Signed-off-by:
-
