On March 7, 2018 4:48:25 PM GMT+02:00, Igor Stoppa [off-list ref] wrote:

On 06/03/18 15:19, Mike Rapoport wrote:

quoted

On Wed, Feb 28, 2018 at 10:06:14PM +0200, Igor Stoppa wrote:

[...]

quoted

If I'm not mistaken, several kernel-doc descriptions are duplicated

now.

quoted

Can you please keep a single copy? ;-)

What's the preferred approach?
Document the functions that are API in the .h file and leave in the .c
those which are not API?

I aggree with Matthew: "we usually recommend putting it with the definition so it's more likely to be updated."

I couldn't find the doc with this recommendation, though :)

[...]

quoted

quoted

+ * The alignment at which to perform the research for sequence of

empty

quoted

                                           ^ search?

yes

quoted

quoted

+ * get_boundary() - verifies address, then measure length.

There's some lack of consistency between the name and implementation

and

quoted

the description.
It seems that it would be simpler to actually make it get_length()

and

quoted

return the length of the allocation or nentries if the latter is

smaller.

quoted

Then in gen_pool_free() there will be no need to recalculate nentries
again.

There is an error in the documentation. I'll explain below.

quoted

quoted

  * @map: pointer to a bitmap
- * @start: a bit position in @map
- * @nr: number of bits to set
+ * @start_entry: the index of the first entry in the bitmap
+ * @nentries: number of entries to alter

Maybe: "maximal number of entries to check"?

No, it's actually the total number of entries in the chunk.

[...]

quoted

quoted

+	return nentries - start_entry;

Shouldn't it be "nentries + start_entry"?

And in the light of the correct comment, also what I am doing should be
now more clear:

* start_entry is the index of the initial entry
* nentries is the number of entries in the chunk

If I iterate over the rest of the chunk:

(i = start_entry + 1; i < nentries; i++)

without finding either another HEAD or an empty slot, then it means I
was measuring the length of the last allocation in the chunk, which was
taking up all the space, to the end.

Simple example:

- chunk with 7 entries -> nentries is 7
- start_entry is 2, meaning that the last allocation starts from the
3rd
element, iow it occupies indexes from 2 to 6, for a total of 5 entries
- so the length is (nentries - start_entry) = (7 - 2) = 5


But yeah, the kerneldoc was wrong.

[...]

quoted

quoted

- * gen_pool_alloc_algo - allocate special memory from the pool
+ * gen_pool_alloc_algo() - allocate special memory from the pool

+ using specified algorithm

ok

quoted

quoted

  * @pool: pool to allocate from
  * @size: number of bytes to allocate from the pool
  * @algo: algorithm passed from caller

@@ -285,14 +502,18 @@ EXPORT_SYMBOL(gen_pool_alloc);
  * Uses the pool allocation function (with first-fit algorithm by

default).

quoted

"uses the provided @algo function to find room for the allocation"

ok

--
igor

-- 
Sincerely yours,
Mike.