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@xxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html