Messages in this thread | | | Date | Thu, 9 Dec 2021 12:47:35 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH v6 02/13] user_events: Add minimal support for trace_event into ftrace |
| |
On Thu, 9 Dec 2021 09:40:50 -0800 Beau Belgrave <beaub@linux.microsoft.com> wrote:
> No, this is not a fast path, and I don't have a problem moving to a > mutex if you feel that is better. I've likely become too close to this > code to know when things are confusing for others.
Yeah. I really dislike the "protection by algorithms" then protection by locking unless it is a fast path.
If this was a fast path then I'd be more concerned. I dislike global locks as well, but unless contention becomes a concern, I don't think we should worry about it.
Also, for this comment:
+static int user_events_release(struct inode *node, struct file *file) +{ + struct user_event_refs *refs; + struct user_event *user; + int i; + + /* + * refs is protected by RCU and could in theory change immediately + * before this call on another core. To ensure we read the latest + * version of refs we acquire the RCU read lock again. + */ + rcu_read_lock_sched(); + refs = rcu_dereference_sched(file->private_data); + rcu_read_unlock_sched();
How do you see refs changing on another core if this can only be called when nothing has a reference to it?
I think this comment and grabbing the rcu locks is what is causing me concern.
-- Steve
| |