Re: [patch v2 5/5] ntfs: remove dependency on __GFP_NOFAIL

[David Rientjes]

On Thu, 2 Sep 2010, Anton Altaparmakov wrote:

> > @@ -76,11 +76,19 @@ static inline void *ntfs_malloc_nofs(unsigned long size)
> >  * This function guarantees that the allocation will succeed.  It will sleep
> >  * for as long as it takes to complete the allocation.
> >  *
> > - * If there was insufficient memory to complete the request, return NULL.
> > + * NOTE: no new callers of this function should be implemented!
> > + * All memory allocations should be failable whenever possible.
> >  */
> > static inline void *ntfs_malloc_nofs_nofail(unsigned long size)
> > {
> > -	return __ntfs_malloc(size, GFP_NOFS | __GFP_HIGHMEM | __GFP_NOFAIL);
> > +	void *ret;
> > +
> > +	for (;;) {
> > +		ret = ntfs_malloc_nofs(size);
> > +		if (ret)
> > +			return ret;
> > +		WARN_ON_ONCE(get_order(size) > PAGE_ALLOC_COSTLY_ORDER);
> 
> What is the relevance of the above get_order check?  ntfs_malloc_nofs() 
> does a vmalloc() if size >= PAGE_SIZE so I cannot see how that has 
> anything to do with get_order()?
> 

It probably shouldn't be doing vmalloc() for size == PAGE_SIZE, but I can 
understand how it would use it for higher order allocations.  I didn't 
look at that specifically because I assumed these were going through the 
page allocator (and no current user should have any order less than 
PAGE_ALLOC_COSTLY_ORDER, correct me if I'm wrong).

I'm pretty sure we want some sort of warning emitted once if this has the 
potential to loop forever, so it should probably be changed to WARN_ONCE() 
in this patch only.  Thanks for pointing that out.

> Other than that patch looks fine.
> 
> Note the _nofail function is only called from one place and there will 
> be no more call sites for it.  One day we ought to work something out so 
> it is not needed at all but given it only gets called in very obscure 
> circumstances (definitely not in the general code paths) it shouldn't 
> matter too much...
> 

That would be great!
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/