Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] tracefs: commit message blames eventfs_create_dir() instead of eventfs_create_events_dir() --
commit 518d04c37ab25c6aa9068d020ccd9d857768cb9a Author: Deepanshu Kartikey <[email protected]> ms/eventfs: Initialize ei->children and ei->list in init_ei() This patch moves the INIT_LIST_HEAD() calls for ei->children and ei->list into init_ei(). Both eventfs_create_dir() and eventfs_create_events_dir() then get initialized lists before any failure path can reach free_ei(). Link: https://patch.msgid.link/[email protected] > ms/eventfs: Initialize ei->children and ei->list in init_ei() > > 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] This isn't a code bug, but is the commit message naming the right function? Before this patch, eventfs_create_dir() already ran INIT_LIST_HEAD() on ei->children and ei->list right after alloc_ei() and the field assignments: ei = alloc_ei(name); if (!ei) return ERR_PTR(-ENOMEM); ei->entries = entries; ei->nr_entries = size; ei->data = data; INIT_LIST_HEAD(&ei->children); INIT_LIST_HEAD(&ei->list); The only later cleanup_ei() call in that function comes after list_add_tail() and list_empty(), so the lists were already initialized there. The path that could reach free_ei() with uninitialized (zeroed) list heads is the failure path in 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); Here the INIT_LIST_HEAD() calls ran only after those failure points. Could the commit message be reworded to name eventfs_create_events_dir() as the function that hit the false warning? The code change itself looks correct, since it covers both callers. > diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c > --- a/fs/tracefs/event_inode.c > +++ b/fs/tracefs/event_inode.c > @@ -444,6 +444,8 @@ static inline struct eventfs_inode *init_ei(struct > eventfs_inode *ei, const char > if (!ei->name) > return NULL; > kref_init(&ei->kref); > + INIT_LIST_HEAD(&ei->children); > + INIT_LIST_HEAD(&ei->list); > return ei; > } > [ ... ] -- Sashiko AI review ยท https://sashiko.vzint.dev/#/patchset/[email protected]?part=4 _______________________________________________ Devel mailing list [email protected] https://lists.openvz.org/mailman/listinfo/devel
