Re: How to handle an kmalloc failure in evict_inode()?

[Dave Chinner <david@fromorbit.com>]

On Mon, Aug 04, 2014 at 09:53:22PM -0400, Theodore Ts'o wrote:
> On Tue, Aug 05, 2014 at 11:13:56AM +1000, Dave Chinner wrote:
> > Sure, because failing to free EOF blocks on referenced inodes (i.e. link
> > count > 0) is not a serious problem. i.e. freeing speculative preallocation
> > beyond EOF is a best effort operation so lock inversions or memory
> > allocation failure simply means we don't do it. It'll get cleaned up
> > in the future (i.e. next time the inode gets pulled into cache).
> 
> I was worried about the case where the link count == 0 and
> evict_inode() needs to release the inode --- and you get a memory
> allocation failure.

Even with XFS, that's not a big deal. We need code to handle it, but
evict/destroy_inode doesn't actually free the inode n XFS - it simply
removes it from the visibility of the VFS and marks it as
reclaimable in internal structures.

Reclaiming the inode and freeing the memory is done by another
shrinker - the fs specific one attached to the superblock - and so
in XFS we have the /potential/ to "fail" to evict inodes for as
long as memory allocation fails because we can redo the operation
inside the filesystem....

IOWs, the longer term plan is to move all this stuff to async
workqueue processing and so be able to defer and batch unlink and
reclaim work more efficiently:

http://xfs.org/index.php/Improving_inode_Caching#Inode_Unlink
http://xfs.org/index.php/Improving_inode_Caching#Reclaim_Optimizations

> I didn't realize that xfs's kmem_*alloc() functions uses a retry loop.
> I think these days the preferred solution from the MM folks is to use
> GFP_NOFAIL (although for a long time folks like David Rientjes were
> trying to claim that both GFP_NOFAIL and looping was evil, and that
> all kernel code had to be able to handle memory allocation failures).

I've always just ignored that "looping is evil" stupidity because
it's a necessity. Nobody has tried to remove it from XFS, so until
then ignorance is bliss....

> As near as I can tell, in the case of evict_inode and link_count == 0,
> using either GFP_NOFAIL or GFP_NOWARN and a retry loop is the only way
> to deal because in the evict_inode and link_count == 0 case, failure
> is simply not an option.

*nod*

> > Yup, that's why XFS uses __GFP_NOWARN and a retry loop - because
> > there are places where failure to allocate memory can have only
> > one result: denial of service via a filesystem shutdown.
> 
> Great, the next time David Rijentes whines at me, I'll direct him in
> your direction, and we can flame him together.  :-)

Fun for everyone. Popcorn? ;)

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com