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

[Anton Altaparmakov]

Hi David,

On 2 Sep 2010, at 02:03, David Rientjes wrote:
> Reimplement ntfs_malloc_nofs_nofail() to loop forever calling
> ntfs_malloc_nofs() until the allocation succeeds.
> 
> If the first allocation attempt fails because the page allocator doesn't
> implicitly loop, a warning will be emitted, including a call trace.
> Subsequent failures will suppress this warning.
> 
> This was added as a helper function for documentation and auditability.
> No future callers should be added.
> 
> Signed-off-by: David Rientjes <rientjes@xxxxxxxxxx>
> ---
> fs/ntfs/malloc.h |   12 ++++++++++--
> 1 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ntfs/malloc.h b/fs/ntfs/malloc.h
> --- a/fs/ntfs/malloc.h
> +++ b/fs/ntfs/malloc.h
> @@ -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()?

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...

Best regards,

	Anton


> +	}
> }
> 
> static inline void ntfs_free(void *addr)

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/

--
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/