On Mon, 10 Jul 2023 18:53:53 +0000 Ajay Kaher <akaher@xxxxxxxxxx> wrote: > > On 10-Jul-2023, at 7:24 AM, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: > > > > !! External Email > > > > On Mon, 3 Jul 2023 15:52:26 -0400 > > Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: > > > >> On Mon, 3 Jul 2023 18:51:22 +0000 > >> Ajay Kaher <akaher@xxxxxxxxxx> wrote: > >> > >>>> > >>>> We can also look to see if we can implement this with RCU. What exactly > >>>> is this rwsem protecting? > >>>> > >>> > >>> - struct eventfs_file holds the meta-data for file or dir. > >>> https://github.com/intel-lab-lkp/linux/blob/dfe0dc15a73261ed83cdc728e43f4b3d4e315aae/include/linux/tracefs.h#L28 > >>> - eventfs_rwsem is supposed to protect the 'link-list which is made of struct eventfs_file > >>> ' and elements of struct eventfs_file. > >> > >> RCU is usually the perfect solution for protecting link lists though. I'll > >> take a look at this when I get back to work. > >> > > > > So I did the below patch on top of this series. If you could fold this > > into the appropriate patches, it should get us closer to an acceptable > > solution. > > > > What I did was: > > > > 1. Moved the struct eventfs_file and eventfs_inode into event_inode.c as it > > really should not be exposed to all users. > > > > 2. Added a recursion check to eventfs_remove_rec() as it is really > > dangerous to have unchecked recursion in the kernel (we do have a fixed > > size stack). > > > > 3. Removed all the eventfs_rwsem code and replaced it with an srcu lock for > > the readers, and a mutex to synchronize the writers of the list. > > > > 4. Added a eventfs_mutex that is used for the modifications of the > > dentry itself (as well as modifying the list from 3 above). > > > > 5. Have the free use srcu callbacks. After the srcu grace periods are done, > > it adds the eventfs_file onto a llist (lockless link list) and wakes up a > > work queue. Then the work queue does the freeing (this needs to be done in > > task/workqueue context, as srcu callbacks are done in softirq context). > > > > This appears to pass through some of my instance stress tests as well as > > the in tree ftrace selftests. > > > > Awesome :) > > I have manually applied the patches and ftracetest results are same as v3. > No more complains from lockdep. > > I will merge this into appropriate patches of v3 and soon send v4. > > You have renamed eventfs_create_dir() to create_dir(), and kept eventfs_create_dir() > just a wrapper with lock, same for eventfs_create_file(). However these wrapper no where > used, I will drop these wrappers. Ah, I thought that because they started with "eventfs_" that they were used for some fops pointer. Note, I try to avoid using the "eventfs_" naming for static functions that are not exported elsewhere. > > I was trying to have independent lock for each instance of events. As common lock > for every instance of events is not must. We can find a way to make the lock for the root later. Let's get it working first before we optimize it. I do not want to expose any locking to the users of this interface. > > Something was broken in your mail (I guess cc list) and couldn’t reach to lkml or > ignored by lkml. I just wanted to track the auto test results from linux-kselftest. Yeah, claws-mail has an issue with some emails with quotes in it (sometimes drops the second quote). Sad part is, it happens after I hit send, and it is not part of the email. I'll send this reply now, but I bet it's going to happen again. Let's see :-/ I checked the To and Cc's and they all have the proper quotes. Let's see what ends up in my "Sent" folder. -- Steve
