Messages in this thread | | | Date | Fri, 16 Jan 2009 03:53:10 +0100 | From | Nick Piggin <> | Subject | Re: [patch 68/85] fs: symlink write_begin allocation context fix |
| |
On Thu, Jan 15, 2009 at 02:31:17PM -0800, Greg KH wrote: > On Thu, Jan 15, 2009 at 02:01:36PM -0800, Greg KH wrote: > > 2.6.27-stable review patch. If anyone has any objections, please let us know. > > Hm, I think I got the backport of this patch wrong in two place: > > > --- a/mm/filemap.c > > +++ b/mm/filemap.c > > @@ -2033,7 +2033,7 @@ int pagecache_write_begin(struct file *f > > struct inode *inode = mapping->host; > > struct page *page; > > again: > > - page = __grab_cache_page(mapping, index); > > + page = grab_cache_page_write_begin(mapping, index, flags); > > I think this shouldn't be "flags", but 0, right?
I think flags is right. That way, a caller of pagecache_write_begin (eg. page_symlink) can instruct it with AOP_FLAG_NOFS to prevent grab_cache_page from doing a GFP_KERNEL allocation.
> > @@ -2263,7 +2268,7 @@ static ssize_t generic_perform_write_2co > > break; > > } > > > > - page = __grab_cache_page(mapping, index); > > + page = grab_cache_page_write_begin(mapping, index, GFP_KERNEL); > > And this one? Is GFP_KERNEL ok, or 0? > > Nick, your thoughts?
This should be 0. grab_cache_page_write_begin takes write_begin aop flags rather than regular gfp flags (it's a bit confusing, sorry :P). 0 ~= GFP_KERNEL (wheras AOP_FLAG_NOFS ~= GFP_NOFS). So you have the right idea ;)
Thanks, Nick
| |