lkml.org 
[lkml]   [2017]   [Feb]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 4/6] xfs: use memalloc_nofs_{save,restore} instead of memalloc_noio*
On Mon 06-02-17 10:32:37, Darrick J. Wong wrote:
> On Mon, Feb 06, 2017 at 06:44:15PM +0100, Michal Hocko wrote:
> > On Mon 06-02-17 07:39:23, Matthew Wilcox wrote:
> > > On Mon, Feb 06, 2017 at 03:07:16PM +0100, Michal Hocko wrote:
> > > > +++ b/fs/xfs/xfs_buf.c
> > > > @@ -442,17 +442,17 @@ _xfs_buf_map_pages(
> > > > bp->b_addr = NULL;
> > > > } else {
> > > > int retried = 0;
> > > > - unsigned noio_flag;
> > > > + unsigned nofs_flag;
> > > >
> > > > /*
> > > > * vm_map_ram() will allocate auxillary structures (e.g.
> > > > * pagetables) with GFP_KERNEL, yet we are likely to be under
> > > > * GFP_NOFS context here. Hence we need to tell memory reclaim
> > > > - * that we are in such a context via PF_MEMALLOC_NOIO to prevent
> > > > + * that we are in such a context via PF_MEMALLOC_NOFS to prevent
> > > > * memory reclaim re-entering the filesystem here and
> > > > * potentially deadlocking.
> > > > */
> > >
> > > This comment feels out of date ... how about:
> >
> > which part is out of date?
> >
> > >
> > > /*
> > > * vm_map_ram will allocate auxiliary structures (eg page
> > > * tables) with GFP_KERNEL. If that tries to reclaim memory
> > > * by calling back into this filesystem, we may deadlock.
> > > * Prevent that by setting the NOFS flag.
> > > */
> >
> > dunno, the previous wording seems clear enough to me. Maybe little bit
> > more chatty than yours but I am not sure this is worth changing.
>
> I prefer to keep the "...yet we are likely to be under GFP_NOFS..."
> wording of the old comment because it captures the uncertainty of
> whether or not we actually are already under NOFS. If someone actually
> has audited this code well enough to know for sure then yes let's change
> the comment, but I haven't gone that far.

I believe we can drop the memalloc_nofs_save then as well because either
we are called from a potentially dangerous context and thus we are in
the nofs scope we we do not need the protection at all.
--
Michal Hocko
SUSE Labs

\
 
 \ /
  Last update: 2017-02-06 19:48    [W:1.015 / U:0.536 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site