On Mon, Jan 10, 2022 at 9:30 AM Yonghong Song <yhs@xxxxxx> wrote: > > > > On 1/7/22 12:43 PM, Hao Luo wrote: > > On Thu, Jan 6, 2022 at 4:30 PM Yonghong Song <yhs@xxxxxx> wrote: > >> > >> > >> > >> On 1/6/22 1:50 PM, Hao Luo wrote: > >>> Bpffs is a pseudo file system that persists bpf objects. Previously > >>> bpf objects can only be pinned in bpffs, this patchset extends pinning > >>> to allow bpf objects to be pinned (or exposed) to other file systems. > >>> > >>> In particular, this patchset allows pinning bpf objects in kernfs. This > >>> creates a new file entry in the kernfs file system and the created file > >>> is able to reference the bpf object. By doing so, bpf can be used to > >>> customize the file's operations, such as seq_show. > >>> > >>> As a concrete usecase of this feature, this patchset introduces a > >>> simple new program type called 'bpf_view', which can be used to format > >>> a seq file by a kernel object's state. By pinning a bpf_view program > >>> into a cgroup directory, userspace is able to read the cgroup's state > >>> from file in a format defined by the bpf program. > >>> > >>> Different from bpffs, kernfs doesn't have a callback when a kernfs node > >>> is freed, which is problem if we allow the kernfs node to hold an extra > >>> reference of the bpf object, because there is no chance to dec the > >>> object's refcnt. Therefore the kernfs node created by pinning doesn't > >>> hold reference of the bpf object. The lifetime of the kernfs node > >>> depends on the lifetime of the bpf object. Rather than "pinning in > >>> kernfs", it is "exposing to kernfs". We require the bpf object to be > >>> pinned in bpffs first before it can be pinned in kernfs. When the > >>> object is unpinned from bpffs, their kernfs nodes will be removed > >>> automatically. This somehow treats a pinned bpf object as a persistent > >>> "device". > > > > Thanks Yonghong for the feedback. > > > >> > >> During our initial discussion for bpf_iter, we even proposed to > >> put cat-able files under /proc/ system. But there are some concerns > >> that /proc/ system holds stable APIs so people can rely on its format > >> etc. Not sure kernfs here has such requirement or not. > >> > >> I understand directly put files in respective target directories > >> (e.g., cgroup) helps. But you can also create directory hierarchy > >> in bpffs. This can make a bunch of cgroup-stat-dumping bpf programs > >> better organized. > >> > > > > I thought about this. I think the problem is that you need to > > simultaneously manage two hierarchies now. You may have > > synchronization problems in bpffs to deal with. For example, what if > > the target cgroup is being removed while there is an on-going 'cat' on > > the bpf program. I haven't given much thought in this direction. This > > patchset doesn't deal with this problem, because kernfs has already > > handled these synchronizations. > > If the file is going to pinned inside /sys/fs/cgroup, which arguably is > indeed a better place, maybe ask cgroup maintainer's opinion? > Yes, will do. I don't know if pinning in other file systems other than cgroup has any use. If not, I can tailor this patchset for cgroup only. Would be much simplified. > > > >> Also regarding new program type bpf_view, I think we can reuse > >> bpf_iter infrastructure. E.g., we already can customize bpf_iter > >> for a particular map. We can certainly customize bpf_iter targeting > >> a particular cgroup. > >> > > > > Right, that's what I was thinking. During the bpf office hour when I > > initially proposed the idea, Alexei suggested creating a new program > > type instead of reusing bpf_iter. The reason I remember was that iter > > is for iterating a set of objects. Even for dumping a particular map, > > it is iterating the entries in a map. While what I wanted to achieve > > here is printing for a particular cgroup, not iterating something. > > Maybe, we should reuse the infrastructure of bpf_iter (attach, target > > registration, etc) but having a different prog type? I do copy-pasted > > many code from bpf_iter for bpf_view. I haven't put too much thought > > there as I would like to get feedbacks on the idea in general in this > > first version of RFC. > > Sorry I am not aware of this bpf_view discussion. It is okay for me. > But it would be great if we can avoid lots of code duplication. > No problem. I totally agree. > > > >>> > >>> We rely on fsnotify to monitor the inode events in bpffs. A new function > >>> bpf_watch_inode() is introduced. It allows registering a callback > >>> function at inode destruction. For the kernfs case, a callback that > >>> removes kernfs node is registered at the destruction of bpffs inodes. > >>> For other file systems such as sockfs, bpf_watch_inode() can monitor the > >>> destruction of sockfs inodes and the created file entry can hold the bpf > >>> object's reference. In this case, it is truly "pinning". > >>> > >>> File operations other than seq_show can also be implemented using bpf. > >>> For example, bpf may be of help for .poll and .mmap in kernfs. > >>> > [...]
