From: Kuan-Wei Chiu <visitorckw@gmail.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: colyli@suse.de, kent.overstreet@linux.dev, msakai@redhat.com,
mingo@redhat.com, acme@kernel.org, namhyung@kernel.org,
akpm@linux-foundation.org, bfoster@redhat.com,
mark.rutland@arm.com, alexander.shishkin@linux.intel.com,
jolsa@kernel.org, irogers@google.com, adrian.hunter@intel.com,
bagasdotme@gmail.com, jserv@ccns.ncku.edu.tw,
linux-bcache@vger.kernel.org, linux-kernel@vger.kernel.org,
dm-devel@lists.linux.dev, linux-bcachefs@vger.kernel.org,
linux-perf-users@vger.kernel.org
Subject: Re: [RESEND PATCH v5 11/16] lib min_heap: Update min_heap_push() and min_heap_pop() to return bool values
Date: Wed, 15 May 2024 17:25:54 +0800 [thread overview]
Message-ID: <ZkR/ovACvPFqvFCv@visitorckw-System-Product-Name> (raw)
In-Reply-To: <20240515083755.GB40213@noisy.programming.kicks-ass.net>
On Wed, May 15, 2024 at 10:37:55AM +0200, Peter Zijlstra wrote:
> On Tue, May 14, 2024 at 04:47:19PM +0800, Kuan-Wei Chiu wrote:
> > Modify the min_heap_push() and min_heap_pop() to return a boolean
> > value. They now return false when the operation fails and true when it
> > succeeds.
>
> But why ?!
When handling failures of push/pop operations, although we could
achieve the same effect by checking whether the heap is empty/full
before push/pop, we have already performed such checks within the
push/pop operations. Therefore, I believe directly using the result
of the check as the return value will make the code written by the user
more concise. This return value is used in subsequent patches for
replacing the heap macro in bcache and bcachefs to determine if an
error has occurred. The original heap macros in bcache and bcachefs
also do the same thing.
Regards,
Kuan-Wei
>
> > Signed-off-by: Kuan-Wei Chiu <visitorckw@gmail.com>
> > Reviewed-by: Ian Rogers <irogers@google.com>
> > ---
> > include/linux/min_heap.h | 12 ++++++++----
> > 1 file changed, 8 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/linux/min_heap.h b/include/linux/min_heap.h
> > index c94f9d303205..2d080f85ad0d 100644
> > --- a/include/linux/min_heap.h
> > +++ b/include/linux/min_heap.h
> > @@ -147,18 +147,20 @@ void __min_heapify_all(min_heap_char *heap, size_t elem_size,
> >
> > /* Remove minimum element from the heap, O(log2(nr)). */
> > static __always_inline
> > -void __min_heap_pop(min_heap_char *heap, size_t elem_size,
> > +bool __min_heap_pop(min_heap_char *heap, size_t elem_size,
> > const struct min_heap_callbacks *func, void *args)
> > {
> > void *data = heap->data;
> >
> > if (WARN_ONCE(heap->nr <= 0, "Popping an empty heap"))
> > - return;
> > + return false;
> >
> > /* Place last element at the root (position 0) and then sift down. */
> > heap->nr--;
> > memcpy(data, data + (heap->nr * elem_size), elem_size);
> > __min_heapify(heap, 0, elem_size, func, args);
> > +
> > + return true;
> > }
> >
> > #define min_heap_pop(_heap, _func, _args) \
> > @@ -184,7 +186,7 @@ void __min_heap_pop_push(min_heap_char *heap,
> >
> > /* Push an element on to the heap, O(log2(nr)). */
> > static __always_inline
> > -void __min_heap_push(min_heap_char *heap, const void *element, size_t elem_size,
> > +bool __min_heap_push(min_heap_char *heap, const void *element, size_t elem_size,
> > const struct min_heap_callbacks *func, void *args)
> > {
> > void *data = heap->data;
> > @@ -192,7 +194,7 @@ void __min_heap_push(min_heap_char *heap, const void *element, size_t elem_size,
> > int pos;
> >
> > if (WARN_ONCE(heap->nr >= heap->size, "Pushing on a full heap"))
> > - return;
> > + return false;
> >
> > /* Place at the end of data. */
> > pos = heap->nr;
> > @@ -207,6 +209,8 @@ void __min_heap_push(min_heap_char *heap, const void *element, size_t elem_size,
> > break;
> > func->swp(parent, child, args);
> > }
> > +
> > + return true;
> > }
> >
> > #define min_heap_push(_heap, _element, _func, _args) \
> > --
> > 2.34.1
> >
next prev parent reply other threads:[~2024-05-15 9:26 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-14 8:47 [RESEND PATCH v5 00/16] treewide: Refactor heap related implementation Kuan-Wei Chiu
2024-05-14 8:47 ` [RESEND PATCH v5 01/16] perf/core: Fix several typos Kuan-Wei Chiu
2024-05-14 8:47 ` [RESEND PATCH v5 02/16] bcache: Fix typo Kuan-Wei Chiu
2024-05-14 8:47 ` [RESEND PATCH v5 03/16] bcachefs: " Kuan-Wei Chiu
2024-05-14 8:47 ` [RESEND PATCH v5 04/16] lib min_heap: Add type safe interface Kuan-Wei Chiu
2024-05-22 23:10 ` Andrew Morton
2024-05-22 23:33 ` Kent Overstreet
2024-05-23 3:51 ` Ian Rogers
2024-05-23 15:35 ` Kuan-Wei Chiu
2024-05-14 8:47 ` [RESEND PATCH v5 05/16] lib min_heap: Add min_heap_init() Kuan-Wei Chiu
2024-05-14 8:47 ` [RESEND PATCH v5 06/16] lib min_heap: Add min_heap_peek() Kuan-Wei Chiu
2024-05-14 8:47 ` [RESEND PATCH v5 07/16] lib min_heap: Add min_heap_full() Kuan-Wei Chiu
2024-05-14 8:47 ` [RESEND PATCH v5 08/16] lib min_heap: Add args for min_heap_callbacks Kuan-Wei Chiu
2024-05-14 8:47 ` [RESEND PATCH v5 09/16] lib min_heap: Add min_heap_sift_up() Kuan-Wei Chiu
2024-05-14 8:47 ` [RESEND PATCH v5 10/16] lib min_heap: Add min_heap_del() Kuan-Wei Chiu
2024-05-14 8:47 ` [RESEND PATCH v5 11/16] lib min_heap: Update min_heap_push() and min_heap_pop() to return bool values Kuan-Wei Chiu
2024-05-15 8:37 ` Peter Zijlstra
2024-05-15 9:25 ` Kuan-Wei Chiu [this message]
2024-05-19 23:03 ` Kent Overstreet
2024-05-14 8:47 ` [RESEND PATCH v5 12/16] lib min_heap: Rename min_heapify() to min_heap_sift_down() Kuan-Wei Chiu
2024-05-14 8:47 ` [RESEND PATCH v5 13/16] lib min_heap: Update min_heap_push() to use min_heap_sift_up() Kuan-Wei Chiu
2024-05-14 8:47 ` [RESEND PATCH v5 14/16] lib/test_min_heap: Add test for heap_del() Kuan-Wei Chiu
2024-05-14 8:47 ` [RESEND PATCH v5 15/16] bcache: Remove heap-related macros and switch to generic min_heap Kuan-Wei Chiu
2024-05-14 8:47 ` [RESEND PATCH v5 16/16] bcachefs: " Kuan-Wei Chiu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZkR/ovACvPFqvFCv@visitorckw-System-Product-Name \
--to=visitorckw@gmail.com \
--cc=acme@kernel.org \
--cc=adrian.hunter@intel.com \
--cc=akpm@linux-foundation.org \
--cc=alexander.shishkin@linux.intel.com \
--cc=bagasdotme@gmail.com \
--cc=bfoster@redhat.com \
--cc=colyli@suse.de \
--cc=dm-devel@lists.linux.dev \
--cc=irogers@google.com \
--cc=jolsa@kernel.org \
--cc=jserv@ccns.ncku.edu.tw \
--cc=kent.overstreet@linux.dev \
--cc=linux-bcache@vger.kernel.org \
--cc=linux-bcachefs@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-perf-users@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mingo@redhat.com \
--cc=msakai@redhat.com \
--cc=namhyung@kernel.org \
--cc=peterz@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).