Thanks for the review and apologies for the delay.
Replies inlined below.

On 30/01/18 19:08, Jonathan Corbet wrote:

On Tue, 30 Jan 2018 17:14:45 +0200
Igor Stoppa [off-list ref] wrote:

[...]

Please don't put plain-text files into core-api - that's a directory full

ok

quoted

diff --git a/Documentation/core-api/pmalloc.txt b/Documentation/core-api/pmalloc.txt
new file mode 100644
index 0000000..934d356
--- /dev/null
+++ b/Documentation/core-api/pmalloc.txt

@@ -0,0 +1,104 @@

We might as well put the SPDX tag here, it's a new file.

ok, this is all new stuff to me ... I suppose I should do it also for
all the other new files I create

But what is the license for the documentation? It's not code, so GPL
seems wrong. Creative commons?

I just noticed a patch for checkpatch.pl about SPDX and asked the same
question there.

https://lkml.org/lkml/2018/2/2/365

quoted

+============================
+Protectable memory allocator
+============================
+
+Introduction
+------------

[...]

This is all good information, but I'd suggest it belongs more in the 0/n
patch posting than here.  The introduction of *this* document should say
what it actually covers.

ok

quoted

+
+Design
+------

[...]

quoted

+To keep it to a minimum, locking is left to the user of the API, in
+those cases where it's not strictly needed.

This seems like a relevant and important aspect of the API that shouldn't
be buried in the middle of a section talking about random things.

I'll move it to the Use section.

[...]

quoted

+Use
+---
+
+The typical sequence, when using pmalloc, is:
+
+1. create a pool
+2. [optional] pre-allocate some memory in the pool
+3. issue one or more allocation requests to the pool
+4. initialize the memory obtained
+   - iterate over points 3 & 4 as needed -
+5. write protect the pool
+6. use in read-only mode the handlers obtained through the allocations
+7. [optional] destroy the pool

So one gets this far, but has no actual idea of how to do these things.
Which leads me to wonder: what is this document for?  Who are you expecting
to read it?

I will add a reference to the selftest file.
In practice it can also work as example.

You could improve things a lot by (once again) going to RST and using
directives to bring in the kerneldoc comments from the source (which, I
note, do exist).  But I'd suggest rethinking this document and its
audience.  Most of the people reading it are likely wanting to learn how to
*use* this API; I think it would be best to not leave them frustrated.

ok, the example route should be more explicative.

--
thanks again for the review, igor
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html