linux-kernel - Re: [patch v2 5/5] ntfs: remove dependency on __GFP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <A326B3E1-DB22-4C25-9ED5-C6A5F25BF15E@cam.ac.uk>
Date:	Thu, 2 Sep 2010 10:08:28 +0100
From:	Anton Altaparmakov <aia21@....ac.uk>
To:	David Rientjes <rientjes@...gle.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Anton Altaparmakov <aia21@...tab.net>,
	linux-ntfs-dev@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: [patch v2 5/5] ntfs: remove dependency on __GFP_NOFAIL

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@...gle.com>
> ---
> 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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/