On Wed, Feb 21, 2024 at 5:22 AM Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx> wrote: > > On 2024/02/21 3:27, Vlastimil Babka wrote: > > I'm sure more such scenarios exist, Cc: Tetsuo who I recall was an expert on > > this topic. > > "[PATCH v3 10/35] lib: code tagging framework" says that codetag_lock_module_list() > calls down_read() (i.e. sleeping operation), and > "[PATCH v3 31/35] lib: add memory allocations report in show_mem()" says that > __show_mem() calls alloc_tags_show_mem_report() after kmalloc(GFP_ATOMIC) (i.e. > non-sleeping operation) but alloc_tags_show_mem_report() calls down_read() via > codetag_lock_module_list() !? > > If __show_mem() might be called from atomic context (e.g. kmalloc(GFP_ATOMIC)), > this will be a sleep in atomic bug. > If __show_mem() might be called while semaphore is held for write, > this will be a read-lock after write-lock deadlock bug. > > Not the matter of whether to allocate buffer statically or dynamically. > Please don't hold a lock when trying to report memory usage. Thanks for catching this, Tetsuo! Yes, we take the read-lock here to ensure that the list of modules is stable. I'm thinking I can replace the down_read() with down_read_trylock() and if we fail (there is a race with module load/unload) we will skip generating this report. The probability of racing with module load/unload while in OOM state I think is quite low, so skipping this report should not cause much information loss. > >
