Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1575029

Re: [PATCH 4/6] xfs: use memalloc_nofs_{save,restore} instead of memalloc_noio*

Show all headers | View raw

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.

> 
> > -		noio_flag = memalloc_noio_save();
> > +		nofs_flag = memalloc_nofs_save();
> >  		do {
> >  			bp->b_addr = vm_map_ram(bp->b_pages, bp->b_page_count,
> >  						-1, PAGE_KERNEL);
> 
> Also, I think it shows that this is the wrong place in XFS to be calling
> memalloc_nofs_save().  I'm not arguing against including this patch;
> it's a step towards where we want to be.  I also don't know XFS well
> enough to know where to set that flag ;-)  Presumably when we start a
> transaction ... ?

Yes that is what I would like to achieve longterm. And the reason why I
didn't want to mimic this pattern in kvmalloc as some have suggested.
It just takes much more time to get there from the past experience and
we should really start somewhere.
-- 
Michal Hocko
SUSE Labs

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 4/6] xfs: use memalloc_nofs_{save,restore} instead of memalloc_noio* Michal Hocko <mhocko@kernel.org> - 2017-02-06 15:10 +0100
  Re: [PATCH 4/6] xfs: use memalloc_nofs_{save,restore} instead of  memalloc_noio* Matthew Wilcox <willy@infradead.org> - 2017-02-06 16:40 +0100
    Re: [PATCH 4/6] xfs: use memalloc_nofs_{save,restore} instead of  memalloc_noio* Michal Hocko <mhocko@kernel.org> - 2017-02-06 18:50 +0100
      Re: [PATCH 4/6] xfs: use memalloc_nofs_{save,restore} instead of  memalloc_noio* "Darrick J. Wong" <darrick.wong@oracle.com> - 2017-02-06 19:40 +0100
        Re: [PATCH 4/6] xfs: use memalloc_nofs_{save,restore} instead of  memalloc_noio* Michal Hocko <mhocko@kernel.org> - 2017-02-06 19:50 +0100
          Re: [PATCH 4/6] xfs: use memalloc_nofs_{save,restore} instead of  memalloc_noio* "Darrick J. Wong" <darrick.wong@oracle.com> - 2017-02-06 21:00 +0100
            Re: [PATCH 4/6] xfs: use memalloc_nofs_{save,restore} instead of  memalloc_noio* Michal Hocko <mhocko@kernel.org> - 2017-02-06 22:20 +0100
          Re: [PATCH 4/6] xfs: use memalloc_nofs_{save,restore} instead of  memalloc_noio* Dave Chinner <david@fromorbit.com> - 2017-02-07 00:00 +0100
            Re: [PATCH 4/6] xfs: use memalloc_nofs_{save,restore} instead of  memalloc_noio* Michal Hocko <mhocko@kernel.org> - 2017-02-07 08:20 +0100

csiph-web