On Wed, Oct 13, 2021 at 10:26:58AM +0200, Michal Hocko wrote: > > crap like this (found in btrfs): > > > > /* > > * We're holding a transaction handle, so use a NOFS memory > > * allocation context to avoid deadlock if reclaim happens. > > */ > > nofs_flag = memalloc_nofs_save(); > > value = kmalloc(size, GFP_KERNEL); > > memalloc_nofs_restore(nofs_flag); > > Yes this looks wrong indeed! If I were to review such a code I would ask > why the scope cannot match the transaction handle context. IIRC jbd does > that. Adding the transaction start/end as the NOFS scope is a long term plan and going on for years, because it's not a change we would need in btrfs, but rather a favor to MM to switch away from "GFP_NOFS everywhere because it's easy". The first step was to convert the easy cases. Almost all safe cases switching GFP_NOFS to GFP_KERNEL have happened. Another step is to convert GFP_NOFS to memalloc_nofs_save/GFP_KERNEL/memalloc_nofs_restore in contexts where we know we'd rely on the transaction NOFS scope in the future. Once this is implemented, the memalloc_nofs_* calls are deleted and it works as expected. Now you may argue that the switch could be changing GFP_NOFS to GFP_KERNEL at that time but that is not that easy to review or reason about in the whole transaction context in all allocations. This leads to code that was found in __btrfs_set_acl and called crap or wrong, because perhaps the background and the bigger plan is not immediately obvious. I hope the explanation above it puts it to the right perspective. The other class of scoped NOFS protection is around vmalloc-based allocations but that's for a different reason, would be solved by the same transaction start/end conversion as well. I'm working on that from time to time but this usually gets pushed down in the todo list. It's changing a lot of code, from what I've researched so far cannot be done at once and would probably introduce bugs hard to hit because of the external conditions (allocator, system load, ...). I have a plan to do that incrementally, adding assertions and converting functions in small batches to be able to catch bugs early, but I'm not exactly thrilled to start such endeavour in addition to normal development bug hunting. To get things moving again, I've refreshed the patch adding stubs and will try to find the best timing for merg to avoid patch conflicts, but no promises.
