Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] tracefs: commit message names eventfs_create_dir() but the fix targets eventfs_create_events_dir() --
commit 9d0d1df3c54a5c4e101ce47e1ff3727b67a3af30 Author: Deepanshu Kartikey <[email protected]> ms/eventfs: Initialize ei->children and ei->list in init_ei() This commit moves the INIT_LIST_HEAD() calls for ei->children and ei->list into init_ei(), so every eventfs_inode gets initialized lists at allocation time. The calls are removed from eventfs_create_dir() and eventfs_create_events_dir(). Link: https://patch.msgid.link/[email protected] The commit message has an inaccuracy about which function is affected. > eventfs_create_dir() allocates the eventfs_inode and initializes it with > init_ei(). But this does not initialize the eventfs_inode list_heads. If > the eventfs_create_dir() fails due to memory pressure, it will call > free_ei() before it initialized the lists, and that checks to make sure > the eventfs_inode has no children. But because the list wasn't > initialized, it will give a false warning. [Severity: Low] Is this description accurate for eventfs_create_dir()? In eventfs_create_dir(), alloc_ei() is followed directly by plain field assignments, and the list initialization came right after those: fs/tracefs/event_inode.c:eventfs_create_dir() { ... ei = alloc_ei(name); if (!ei) return ERR_PTR(-ENOMEM); ei->entries = entries; ei->nr_entries = size; ei->data = data; ... } There is no failure window between init_ei() and the old INIT_LIST_HEAD() calls. If alloc_ei() fails, free_ei() is not reached. The only cleanup_ei() call runs after the lists were already initialized, so the WARN_ON_ONCE(!list_empty(&ei->children)) in free_ei() cannot fire falsely there. The false warning looks reachable in eventfs_create_events_dir(), where alloc_root_ei() succeeds and tracefs_get_inode() then fails: fs/tracefs/event_inode.c:eventfs_create_events_dir() { ... ei = alloc_root_ei(name); if (!ei) goto fail; inode = tracefs_get_inode(dentry->d_sb); if (unlikely(!inode)) goto fail; ... fail: cleanup_ei(ei); tracefs_failed_creating(dentry); ... } On that path the lists were still zeroed from kzalloc, so list_empty() on ei->children returns false and the warning fires. The code change covers this path too, since init_ei() is shared by alloc_ei() and alloc_root_ei(). Could the commit message name eventfs_create_events_dir() as the function whose failure path triggered the warning? The text is the same as in the upstream commit, so this does not affect the code change itself. -- Sashiko AI review ยท https://sashiko.vzint.dev/#/patchset/[email protected]?part=5 _______________________________________________ Devel mailing list [email protected] https://lists.openvz.org/mailman/listinfo/devel
