On Tue, 2008-08-12 at 17:46 +1000, Neil Brown wrote:

On Thursday July 24, a.p.zijlstra@chello.nl wrote:

quoted

Generic reserve management code. 

It provides methods to reserve and charge. Upon this, generic alloc/free style
reserve pools could be build, which could fully replace mempool_t
functionality.

More comments on this patch .....

quoted

+void *___kmalloc_reserve(size_t size, gfp_t flags, int node, void *ip,
+			 struct mem_reserve *res, int *emerg);
+
+static inline
+void *__kmalloc_reserve(size_t size, gfp_t flags, int node, void *ip,
+			struct mem_reserve *res, int *emerg)
+{
+	void *obj;
+
+	obj = __kmalloc_node_track_caller(size,
+			flags | __GFP_NOMEMALLOC | __GFP_NOWARN, node, ip);
+	if (!obj)
+		obj = ___kmalloc_reserve(size, flags, node, ip, res, emerg);
+
+	return obj;
+}
+
+#define kmalloc_reserve(size, gfp, node, res, emerg) 			\
+	__kmalloc_reserve(size, gfp, node, 				\
+			  __builtin_return_address(0), res, emerg)
+

......

quoted

+/*
+ * alloc wrappers
+ */
+
+void *___kmalloc_reserve(size_t size, gfp_t flags, int node, void *ip,
+			 struct mem_reserve *res, int *emerg)
+{
+	void *obj;
+	gfp_t gfp;
+
+	gfp = flags | __GFP_NOMEMALLOC | __GFP_NOWARN;
+	obj = __kmalloc_node_track_caller(size, gfp, node, ip);
+
+	if (obj || !(gfp_to_alloc_flags(flags) & ALLOC_NO_WATERMARKS))
+		goto out;
+
+	if (res && !mem_reserve_kmalloc_charge(res, size)) {
+		if (!(flags & __GFP_WAIT))
+			goto out;
+
+		wait_event(res->waitqueue,
+				mem_reserve_kmalloc_charge(res, size));
+
+		obj = __kmalloc_node_track_caller(size, gfp, node, ip);
+		if (obj) {
+			mem_reserve_kmalloc_charge(res, -size);
+			goto out;
+		}
+	}
+
+	obj = __kmalloc_node_track_caller(size, flags, node, ip);
+	WARN_ON(!obj);
+	if (emerg)
+		*emerg |= 1;
+
+out:
+	return obj;
+}

Two comments to be precise.

1/ __kmalloc_reserve attempts a __GFP_NOMEMALLOC allocation, and then
   if that fails, ___kmalloc_reserve immediately tries again.
   Is that pointless?  Should the second one be removed?

Pretty pointless yes, except that it made ___kmalloc_reserve a nicer
function to read, and as its an utter slow path I couldn't be arsed to
optimize :-)

2/ mem_reserve_kmalloc_charge appears to assume that the 'mem_reserve'
   has been 'connected' and so is active.

Hmm, that would be __mem_reserve_charge() then, because the callers
don't do much.

   While callers probably only set GFP_MEMALLOC in cases where the
   mem_reserve is connected, ALLOC_NO_WATERMARKS could get via
   PF_MEMALLOC so we could end up calling mem_reserve_kmalloc_charge
   when the mem_reserve is not connected.

Right..

   That seems to be 'odd' at least.
   It might even be 'wrong' as mem_reserve_connect doesn't add the
   usage of the child to the parent - only the ->pages and ->limit.

   What is your position on this?  Mine is "still slightly confused".

Uhmm,. good point. Let me ponder this while I go for breakfast ;-)