Re: [systemd-devel] [PATCH 0/6] RFC: Initial implementation of mount table handling using libmount kernelwatch
On Mon, 2020-08-03 at 09:30 +0100, Colin Guthrie wrote: > Ian Kent wrote on 29/07/2020 01:57: > > On Tue, 2020-07-28 at 16:13 +0200, Lennart Poettering wrote: > > > On Mo, 27.07.20 12:57, Ian Kent (ik...@redhat.com) wrote: > > > > > > > Further to my post about using the new mount table > > > > notifications in > > > > systemd I'd like to start by posting a few patches for > > > > discussion > > > > to > > > > hopefully get a better understanding of some aspects of how > > > > systemd > > > > mount unit state handling works, and also to get guidance on > > > > systemd > > > > coding style, and generally how I should be doing things in > > > > systemd > > > > as well as how best to debug problems. > > > > > > Thank you for working on this! > > > > > > We do reviews exclusively on github these days, could you please > > > submit this as PR on github? > > > > Thanks Lennart, will do. > > > > Although I was hoping for some generic discussion on this to set > > me going in the right direction. > > > > If that's not ok then I'll submit a PR when I think it's ready. > > Although I'm not as familiar with the development process as I used > to > be, you can file it now, even in it's current state for that > discussion > process. It can be an RFC Pull request. Just means the discussion can > all take place in Github and can go through various iterations there. > > So, in short, don't worry about it being fully "ready" before pushing > and doing a pull requests. I know I said I wasn't quite ready but your right, there's bound to be good advice about what I've done and what might need to change, at least now I have something that seems to behave as it's supposed to, ;) Ian ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 0/6] RFC: Initial implementation of mount table handling using libmount kernelwatch
On Mon, 2020-08-03 at 11:34 +0200, Lennart Poettering wrote: > On Mi, 29.07.20 08:57, Ian Kent (ik...@redhat.com) wrote: > > > On Tue, 2020-07-28 at 16:13 +0200, Lennart Poettering wrote: > > > On Mo, 27.07.20 12:57, Ian Kent (ik...@redhat.com) wrote: > > > > > > > Further to my post about using the new mount table > > > > notifications in > > > > systemd I'd like to start by posting a few patches for > > > > discussion > > > > to > > > > hopefully get a better understanding of some aspects of how > > > > systemd > > > > mount unit state handling works, and also to get guidance on > > > > systemd > > > > coding style, and generally how I should be doing things in > > > > systemd > > > > as well as how best to debug problems. > > > > > > Thank you for working on this! > > > > > > We do reviews exclusively on github these days, could you please > > > submit this as PR on github? > > > > Thanks Lennart, will do. > > > > Although I was hoping for some generic discussion on this to set > > me going in the right direction. > > > > If that's not ok then I'll submit a PR when I think it's ready. > > I am sorry, but we do code review in gitub. It's what it is good > for. If you want reviews on patches, even if thery are RFCs, post > them > as PR, like everyone else. > > PRs are general code commenting tools, they are not exclusively for > "final" versions. Yes, I've been looking at the systemd PR's and I see that. I'm fairly confident that the core functionality is working now so I'm not so concerned about feedback about what's happening in systemd any more. And it appears the reduction in overhead is quite significant as was expected. I've also made adjustments based on the systemd coding style notes which I found because you pointed me at github above, that's been useful too. But there's a bit more to do before I can raise a PR. Handling notification buffer overflow (and fall back to the old method) testing needs to be done. And also more testing of various cases and adjustments that may come out of it, as well as a meson build change so systemd builds against a libmount without the new changes. And finally I'll need to re-test all this when David's changes are merged upstream. Thanks for the feedback, Ian ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 0/6] RFC: Initial implementation of mount table handling using libmount kernelwatch
On Mi, 29.07.20 08:57, Ian Kent (ik...@redhat.com) wrote: > On Tue, 2020-07-28 at 16:13 +0200, Lennart Poettering wrote: > > On Mo, 27.07.20 12:57, Ian Kent (ik...@redhat.com) wrote: > > > > > Further to my post about using the new mount table notifications in > > > systemd I'd like to start by posting a few patches for discussion > > > to > > > hopefully get a better understanding of some aspects of how systemd > > > mount unit state handling works, and also to get guidance on > > > systemd > > > coding style, and generally how I should be doing things in systemd > > > as well as how best to debug problems. > > > > Thank you for working on this! > > > > We do reviews exclusively on github these days, could you please > > submit this as PR on github? > > Thanks Lennart, will do. > > Although I was hoping for some generic discussion on this to set > me going in the right direction. > > If that's not ok then I'll submit a PR when I think it's ready. I am sorry, but we do code review in gitub. It's what it is good for. If you want reviews on patches, even if thery are RFCs, post them as PR, like everyone else. PRs are general code commenting tools, they are not exclusively for "final" versions. Lennart -- Lennart Poettering, Berlin ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 0/6] RFC: Initial implementation of mount table handling using libmount kernelwatch
Ian Kent wrote on 29/07/2020 01:57: > On Tue, 2020-07-28 at 16:13 +0200, Lennart Poettering wrote: >> On Mo, 27.07.20 12:57, Ian Kent (ik...@redhat.com) wrote: >> >>> Further to my post about using the new mount table notifications in >>> systemd I'd like to start by posting a few patches for discussion >>> to >>> hopefully get a better understanding of some aspects of how systemd >>> mount unit state handling works, and also to get guidance on >>> systemd >>> coding style, and generally how I should be doing things in systemd >>> as well as how best to debug problems. >> >> Thank you for working on this! >> >> We do reviews exclusively on github these days, could you please >> submit this as PR on github? > > Thanks Lennart, will do. > > Although I was hoping for some generic discussion on this to set > me going in the right direction. > > If that's not ok then I'll submit a PR when I think it's ready. Although I'm not as familiar with the development process as I used to be, you can file it now, even in it's current state for that discussion process. It can be an RFC Pull request. Just means the discussion can all take place in Github and can go through various iterations there. So, in short, don't worry about it being fully "ready" before pushing and doing a pull requests. Col -- Colin Guthrie gmane(at)colin.guthr.ie http://colin.guthr.ie/ Day Job: Tribalogic Limited http://www.tribalogic.net/ Open Source: Mageia Contributor http://www.mageia.org/ PulseAudio Hacker http://www.pulseaudio.org/ Trac Hacker http://trac.edgewall.org/ ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 0/6] RFC: Initial implementation of mount table handling using libmount kernelwatch
On Tue, 2020-07-28 at 16:13 +0200, Lennart Poettering wrote: > On Mo, 27.07.20 12:57, Ian Kent (ik...@redhat.com) wrote: > > > Further to my post about using the new mount table notifications in > > systemd I'd like to start by posting a few patches for discussion > > to > > hopefully get a better understanding of some aspects of how systemd > > mount unit state handling works, and also to get guidance on > > systemd > > coding style, and generally how I should be doing things in systemd > > as well as how best to debug problems. > > Thank you for working on this! > > We do reviews exclusively on github these days, could you please > submit this as PR on github? Thanks Lennart, will do. Although I was hoping for some generic discussion on this to set me going in the right direction. If that's not ok then I'll submit a PR when I think it's ready. Ian ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/systemd-devel
Re: [systemd-devel] [PATCH 0/6] RFC: Initial implementation of mount table handling using libmount kernelwatch
On Mo, 27.07.20 12:57, Ian Kent (ik...@redhat.com) wrote: > Further to my post about using the new mount table notifications in > systemd I'd like to start by posting a few patches for discussion to > hopefully get a better understanding of some aspects of how systemd > mount unit state handling works, and also to get guidance on systemd > coding style, and generally how I should be doing things in systemd > as well as how best to debug problems. Thank you for working on this! We do reviews exclusively on github these days, could you please submit this as PR on github? See: https://systemd.io/CONTRIBUTING https://github.com/systemd/systemd/pulls Lennart -- Lennart Poettering, Berlin ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/systemd-devel
[systemd-devel] [PATCH 0/6] RFC: Initial implementation of mount table handling using libmount kernelwatch
Further to my post about using the new mount table notifications in systemd I'd like to start by posting a few patches for discussion to hopefully get a better understanding of some aspects of how systemd mount unit state handling works, and also to get guidance on systemd coding style, and generally how I should be doing things in systemd as well as how best to debug problems. To test what's been done I use a simple autofs direct mount map with 2 mounts (ie. 2 autofs mounts are done when autofs is started) to trigger notifications. While, in this case, systemd bails out early in the individual mount unit update processing, it still does the mount unit traversal of all the units in the unpatched case so it should be a reasonable test for the notification handling. The patch series depends on changes to use the new fsinfo() system call and also changes to provide the new kernelwatch notifications that will be included in libmount. Karel Zak has done a lot of work on this and his repo. containing this work can be found at: https://github.com/karelzak/util-linux.git, branch topic/fsinfo The only additional libmount change I'm using here is some #define entries of constants MNT_NOTIFY_MOUNT_* that are the same as the kernelwatch NOTIFY_MOUNT_* constants. The reasons for that are, first including linux/watch_queue.h conflicts with existing systemd includes and also because the kernelwatch implementation should be usable from libmount without the need to include the kernel definitions. The patch series is against a fairly recent clone of the upstream systemd git repo but for the actual testing the patches were back ported to a source rpm of systemd-245.4 included in Fedora. There's also a need for work to detect if libmount has the new features and ensure systemd falls back to the old style mount table handing but I'll get to that. There are six patches so far, one to use fsinfo() where possible for mount table loading, another to use fsinfo() for systemd SIGCHLD handling of mount unit mounting, a refactor patch, two patches to enable use of the kernel mount id with the Mount units and finally an initial implementation that uses kernelwatch notifications. With only the patches to use fsinfo() I see a bit more than a 10% reduction in overhead which is to be expected because the entire mount table is still being read and the whole list of mount units traversed on every mountinfo notification. With the last patch added I haven't been able to work out why this isn't working properly. The boot looks OK but I see cores from crashes when I try and log in via gdm that look like a problem with unit garbage collection related to unit state. Also I'm pretty sure I'm falling back to the old mount table reading more often that I should so it's still a WIP. However, even though the kernelwatch implementation isn't completely working, running my test shows a reduction of about half in CPU usage. Comments on what I have so far are welcome. And also any discussion about how the mount unit state update works would be much appreciated, as well as approaches to debugging sysyemd problems in general. --- Ian Kent (6): systemd: try to use libmount fsinfo if available systemd: refactor mount_process_proc_self_mountinfo() systemd: try to use fsinfo for sigchld handling systemd: add mount id to MountParameters systemd: add Hashmap mount_unit_by_mnt_id systemd: use libmount kernelwatch if available src/core/mount.c | 463 src/core/mount.h |1 src/mount/mount-tool.c |4 src/shared/libmount-util.h | 28 +++ src/shared/mount-util.c|4 src/shutdown/umount.c | 18 +- 6 files changed, 425 insertions(+), 93 deletions(-) -- Ian ___ systemd-devel mailing list systemd-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/systemd-devel
