On Thu, Jan 18, 2024 at 9:28 PM Chris Li <chrisl@xxxxxxxxxx> wrote: > > On Thu, Jan 18, 2024 at 10:25 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > > > On Thu, Jan 18, 2024 at 08:59:55AM -0800, Yosry Ahmed wrote: > > > On Thu, Jan 18, 2024 at 5:52 AM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote: > > > > > > > > On Wed, Jan 17, 2024 at 10:20:29PM -0800, Yosry Ahmed wrote: > > > > > > /* walk the tree and free everything */ > > > > > > spin_lock(&tree->lock); > > > > > > + > > > > > > + xas_for_each(&xas, e, ULONG_MAX) > > > > > > > > > > Why not use xa_for_each? > > > > > > > > xas_for_each() is O(n) while xa_for_each() is O(n log n), as mentioned > > > > in the fine documentation. If you don't need to drop the lock while > > > > in the body of the loop, always prefer xas_for_each(). > > > > > > Thanks for pointing this out. Out of ignorance, I skipped reading the > > > doc for this one and operated under the general assumption to use xa_* > > > functions were possible. > > > > > > The doc also says we should hold either the RCU read lock or the > > > xa_lock while iterating, we are not doing either here, no? > > > > I have no idea; I haven't studied the patches in detail yet. I have > > debugging assertions for that, so I was assuming that Chris had been > > developing with debug options turned on. If not, I guess the bots will > > do it for him. > > It is fine now because we have the extra zswap tree spin lock. When we > remove the zswap tree spin lock it does require RCU read lock. You are > right I would get assertion failure. I would imagine the assertions are that we are holding either the RCU read lock or the xa_lock, how would holding the zswap tree lock help?
