On Thu 15-02-24 15:33:30, Kent Overstreet wrote:

On Thu, Feb 15, 2024 at 09:22:07PM +0100, Vlastimil Babka wrote:

quoted

On 2/15/24 19:29, Kent Overstreet wrote:

quoted

On Thu, Feb 15, 2024 at 08:47:59AM -0800, Suren Baghdasaryan wrote:

quoted

On Thu, Feb 15, 2024 at 8:45 AM Michal Hocko [off-list ref] wrote:

quoted

On Thu 15-02-24 06:58:42, Suren Baghdasaryan wrote:

quoted

On Thu, Feb 15, 2024 at 1:22 AM Michal Hocko [off-list ref] wrote:

quoted

On Mon 12-02-24 13:39:17, Suren Baghdasaryan wrote:
[...]

quoted

@@ -423,4 +424,18 @@ void __show_mem(unsigned int filter, nodemask_t *nodemask, int max_zone_idx)
 #ifdef CONFIG_MEMORY_FAILURE
      printk("%lu pages hwpoisoned\n", atomic_long_read(&num_poisoned_pages));
 #endif
+#ifdef CONFIG_MEM_ALLOC_PROFILING
+     {
+             struct seq_buf s;
+             char *buf = kmalloc(4096, GFP_ATOMIC);
+
+             if (buf) {
+                     printk("Memory allocations:\n");
+                     seq_buf_init(&s, buf, 4096);
+                     alloc_tags_show_mem_report(&s);
+                     printk("%s", buf);
+                     kfree(buf);
+             }
+     }
+#endif

I am pretty sure I have already objected to this. Memory allocations in
the oom path are simply no go unless there is absolutely no other way
around that. In this case the buffer could be preallocated.

Good point. We will change this to a smaller buffer allocated on the
stack and will print records one-by-one. Thanks!

__show_mem could be called with a very deep call chains. A single
pre-allocated buffer should just do ok.

Ack. Will do.

No, we're not going to permanently burn 4k here.

It's completely fine if the allocation fails, there's nothing "unsafe"
about doing a GFP_ATOMIC allocation here.

Well, I think without __GFP_NOWARN it will cause a warning and thus
recursion into __show_mem(), potentially infinite? Which is of course
trivial to fix, but I'd myself rather sacrifice a bit of memory to get this
potentially very useful output, if I enabled the profiling. The necessary
memory overhead of page_ext and slabobj_ext makes the printing buffer
overhead negligible in comparison?

__GFP_NOWARN is a good point, we should have that.

But - and correct me if I'm wrong here - doesn't an OOM kick in well
before GFP_ATOMIC 4k allocations are failing?

Not really, GFP_ATOMIC users can compete with reclaimers and consume
those reserves.

I'd expect the system to
be well and truly hosed at that point.

It is OOMed...
 

If we want this report to be 100% reliable, then yes the preallocated
buffer makes sense - but I don't think 100% makes sense here; I think we
can accept ~99% and give back that 4k.

Think about that from the memory reserves consumers. The atomic reserve
is a scarse resource and now you want to use it for debugging purposes
for which you could have preallocated.
-- 
Michal Hocko
SUSE Labs