From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 14 Jan 2016 22:25:44 +0000 From: Al Viro To: Linus Torvalds Cc: Tomeu Vizoso , "linux-kernel@vger.kernel.org" , Neil Brown , linux-fsdevel Subject: Re: [PATCH v2 06/11] don't put symlink bodies in pagecache into highmem Message-ID: <20160114222544.GB17997@ZenIV.linux.org.uk> References: <1449639295-20512-6-git-send-email-viro@ZenIV.linux.org.uk> <20160114152553.GW17997@ZenIV.linux.org.uk> <20160114162333.GX17997@ZenIV.linux.org.uk> <20160114171341.GY17997@ZenIV.linux.org.uk> <20160114210223.GA17997@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Sender: linux-kernel-owner@vger.kernel.org List-ID: On Thu, Jan 14, 2016 at 01:40:32PM -0800, Linus Torvalds wrote: > On Thu, Jan 14, 2016 at 1:02 PM, Al Viro 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);