Messages in this thread | | | Subject | Re: [PATCH] debugobjects: Move printk out of db lock critical sections | From | Dmitry Safonov <> | Date | Fri, 14 Dec 2018 18:06:56 +0000 |
| |
On 12/13/18 10:10 PM, Waiman Long wrote: > The bucket lock is for protecting the insertion and deletion of > debug_obj to/from the bucket list as well as searching within the bucket > list. It has nothing to do with the life time of the debug_obj itself.
The bucket lock also protects lookups on a bucket.
1. Imagine, you have object in ODEBUG_STATE_DESTROYED. If you will try debug_object_activate() with this patch, it will debug print object outside of bucket lock, which means that debug_check_no_obj_freed() may concurrently fixup/free object and meta-data on another CPU.
I don't see this state is being used in many places around the kernel, except selftest and i915 driver.
2. If you try to deactivate already non-active object - you will have debug print outside of bucket lock. The other CPU can fixup/free page with this object concurrently.
(before your patch the lookup would fail and no printing)
So, I might be mistaken, I'm mostly worried about dereferencing the descriptor inside of the object during printing. And in my opinion, i.e., there was a reason to save `descr` before releasing the bucket lock here: https://elixir.bootlin.com/linux/latest/source/lib/debugobjects.c#L789
Anyway, I see that your patch is already in -mm tree and Thomas seems to be fine with the change so whatever %)
Thanks, Dima
| |