Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into highmem

[Al Viro <viro@ZenIV.linux.org.uk>]

On Thu, Jan 14, 2016 at 01:40:32PM -0800, Linus Torvalds wrote:
> On Thu, Jan 14, 2016 at 1:02 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > Arrrgh.  Try this:
> 
> Yeah, that would do it.
> 
> Al, did you check any other filesystems do this?

There's one more turd like that - shmem should've done inode_nohighmem()
a bit earlier in shmem_symlink().  The rest is OK.

> Also, I'm wondering if we should perhaps revert the "don't use
> highmem". Do we actually have examples of running out of kmaps? Do we
> care?

For one thing, we'll lose RCU ->get_link() for those.  For another... yes,
it was a nasty bug (I missed the possibility that filesystem might seed
the page cache on ->symlink() directly and use a highmem page - mea culpa),
but we can easily catch it at runtime.  We really shouldn't put highmem
pages into address_space without __GFP_HIGHMEM, and catching those in
__add_to_page_cache_locked() isn't costly.

Anyway, mm/shmem.c bit follows.  With that + NFS one we ought to be OK
wrt that class of bogosities.  I'll write the bits for
Documentation/filesystems/porting (basically, "if you preseed the pagecache
at ->symlink() time, don't put highmem pages there; page_symlink() will
take care of that, provided that inode_nohighmem() is called first") and
push the combined patch to #for-linus.

diff --git a/mm/shmem.c b/mm/shmem.c
index 5813b7f..642471b 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2469,6 +2469,7 @@ static int shmem_symlink(struct inode *dir, struct dentry *dentry, const char *s
 		inode->i_op = &shmem_short_symlink_operations;
 		inode->i_link = info->symlink;
 	} else {
+		inode_nohighmem(inode);
 		error = shmem_getpage(inode, 0, &page, SGP_WRITE, NULL);
 		if (error) {
 			iput(inode);
@@ -2476,7 +2477,6 @@ static int shmem_symlink(struct inode *dir, struct dentry *dentry, const char *s
 		}
 		inode->i_mapping->a_ops = &shmem_aops;
 		inode->i_op = &shmem_symlink_inode_operations;
-		inode_nohighmem(inode);
 		memcpy(page_address(page), symname, len);
 		SetPageUptodate(page);
 		set_page_dirty(page);