Messages in this thread | | | Date | Tue, 24 Apr 2018 13:25:42 -0600 | From | Michal Hocko <> | Subject | Re: vmalloc with GFP_NOFS |
| |
On Tue 24-04-18 14:35:36, Theodore Ts'o wrote: > On Tue, Apr 24, 2018 at 10:27:12AM -0600, Michal Hocko wrote: > > fs/ext4/xattr.c > > > > What to do about this? Well, there are two things. Firstly, it would be > > really great to double check whether the GFP_NOFS is really needed. I > > cannot judge that because I am not familiar with the code. > > *Most* of the time it's not needed, but there are times when it is. > We could be more smart about sending down GFP_NOFS only when it is > needed.
Well, the primary idea is that you do not have to. All you care about is to use the scope api where it matters + a comment describing the reclaim recursion context (e.g. this lock will be held in the reclaim path here and there).
> If we are sending too many GFP_NOFS's allocations such that > it's causing heartburn, we could fix this. (xattr commands are rare > enough that I dind't think it was worth it to modulate the GFP flags > for this particular case, but we could make it be smarter if it would > help.)
Well, the vmalloc is actually a correctness issue rather than a heartburn...
> > If the use is really valid then we have a way to do the vmalloc > > allocation properly. We have memalloc_nofs_{save,restore} scope api. How > > does that work? You simply call memalloc_nofs_save when the reclaim > > recursion critical section starts (e.g. when you take a lock which is > > then used in the reclaim path - e.g. shrinker) and memalloc_nofs_restore > > when the critical section ends. _All_ allocations within that scope > > will get GFP_NOFS semantic automagically. If you are not sure about the > > scope itself then the easiest workaround is to wrap the vmalloc itself > > with a big fat comment that this should be revisited. > > This is something we could do in ext4. It hadn't been high priority, > because we've been rather overloaded.
Well, ext/jbd already has scopes defined for the transaction context so anything down that road can be converted to GFP_KERNEL (well, unless the same code path is shared outside of the transaction context and still requires a protection). It would be really great to identify other contexts and slowly move away from the explicit GFP_NOFS. Are you aware of other contexts?
> As a suggestion, could you take > documentation about how to convert to the memalloc_nofs_{save,restore} > scope api (which I think you've written about e-mails at length > before), and put that into a file in Documentation/core-api?
I can.
> The question I was trying to figure out which triggered the above > request is how/whether to gradually convert to that scope API. Is it > safe to add the memalloc_nofs_{save,restore} to code and keep the > GFP_NOFS flags until we're sure we got it all right, for all of the > code paths, and then drop the GFP_NOFS?
The first stage is to define and document those scopes. I have provided a debugging patch [1] in the past that would dump_stack when seeing an explicit GFP_NOFS from a scope which could help to eliminate existing users.
[1] http://lkml.kernel.org/r/20170106141845.24362-1-mhocko@kernel.org -- Michal Hocko SUSE Labs
| |