[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZwzskczB85bGKWO/@visitorckw-System-Product-Name>
Date: Mon, 14 Oct 2024 18:04:01 +0800
From: Kuan-Wei Chiu <visitorckw@...il.com>
To: Matthew Wilcox <willy@...radead.org>
Cc: colyli@...e.de, kent.overstreet@...ux.dev, msakai@...hat.com,
corbet@....net, peterz@...radead.org, mingo@...hat.com,
acme@...nel.org, namhyung@...nel.org, akpm@...ux-foundation.org,
mark.rutland@....com, alexander.shishkin@...ux.intel.com,
jolsa@...nel.org, irogers@...gle.com, adrian.hunter@...el.com,
kan.liang@...ux.intel.com, jserv@...s.ncku.edu.tw,
linux-kernel@...r.kernel.org, linux-bcache@...r.kernel.org,
dm-devel@...ts.linux.dev, linux-bcachefs@...r.kernel.org,
linux-perf-users@...r.kernel.org, linux-doc@...r.kernel.org
Subject: Re: [PATCH 3/3] Documentation/core-api: Add min heap API introduction
On Mon, Oct 14, 2024 at 09:55:56AM +0100, Matthew Wilcox wrote:
> On Mon, Oct 14, 2024 at 02:47:03AM +0800, Kuan-Wei Chiu wrote:
> > Introduce an overview of the min heap API, detailing its usage and
> > functionality. The documentation aims to provide developers with a
> > clear understanding of how to implement and utilize min heaps within
> > the Linux kernel, enhancing the overall accessibility of this data
> > structure.
>
> Please format this text to 80 columns. Just pass it through 'fmt'.
>
> > +This API supports efficient insertion, deletion, and access to the minimum element. It is optimized
> > +for use in systems with performance constraints and is suitable for scenarios where the minimum
> > +element needs to be accessed or updated frequently.
>
> All systems have "performance constraints". I'm not sure what that
> means in this context.
>
> > +This document provides a guide to the Min Heap API, detailing how to define and use min-heaps.
> > +Please note that users should not directly call functions with **__min_heap_*()** names, but should
> > +instead use the provided macro wrappers.
>
> You can always remove "Please note that". It has no meaning. Just say
> "You should not call functions with **__min_heap_** prefixes; use the
> functions documented here instead.
>
> > +Min-Heap Definition
> > +-------------------
> > +
> > +The core data structure for representing a min-heap is defined using the **MIN_HEAP_PREALLOCATED**
> > +and **DEFINE_MIN_HEAP** macros. These macros allow you to define a min-heap with a preallocated
> > +buffer or dynamically allocated memory.
> > +
> > +Example:
> > +
> > +.. code-block:: c
> > +
> > + #define MIN_HEAP_PREALLOCATED(_type, _name, _nr)
> > + struct _name {
> > + int nr; /* Number of elements in the heap */
> > + int size; /* Maximum number of elements that can be held */
> > + _type *data; /* Pointer to the heap data */
> > + _type preallocated[_nr]; /* Static preallocated array */
> > + }
>
> This isn't an example of code the reader of this document would write
> though, is it? This looks like code already provided. An example
> should be something like:
>
> MIN_HEAP_PREALLOCATED(struct page, my_pages, 23);
>
> ... or whatever would actually make sense.
Thank you for the review. I'll wait for the decision on whether to keep
the non-inline API before sending a v2 patch to address these comments.
Regards,
Kuan-Wei
Powered by blists - more mailing lists