From: William Lee Irwin III <wli@holomorphy.com>
To: Paul Jackson <pj@sgi.com>
Cc: colpatch@us.ibm.com, linux-kernel@vger.kernel.org,
mbligh@aracnet.com, akpm@osdl.org
Subject: Re: [PATCH] mask ADT: bitmap and bitop tweaks [1/22]
Date: Mon, 29 Mar 2004 18:55:35 -0800 [thread overview]
Message-ID: <20040330025535.GD791@holomorphy.com> (raw)
In-Reply-To: <20040329174637.3aa16260.pj@sgi.com>
At some point in the past, I wrote:
>> akpm, this is needed for mainline.
On Mon, Mar 29, 2004 at 05:46:37PM -0800, Paul Jackson wrote:
> How urgent to you consider this fix (masking unused bits in the
> arithmetic (single unsigned long word) cpumask implementation?
> So far as I know, the only way to get high bits set with correct
> invocations is by using cpus_complement(), which I don't see anyone
> doing.
> So I believe that this patch fixes latent bugs, not current bugs.
False. The semantics are currently "don't care" and the ADT fails to
ignore the upper bits in cpumask_arith.h. It's a bug in the ADT code.
Whether callers experience ill effects is irrelevant.
On Mon, Mar 29, 2004 at 05:46:37PM -0800, Paul Jackson wrote:
> And it would be my preference (not surprisingly) to fix this in a way
> that is consistent with my mask ADT proposal (avoid setting unused bits
> on proper calls; don't filter on Boolean/scalar predicate evaluations):
> +#if NR_CPUS % BITS_PER_LONG
> +#define __CPU_VALID_MASK__ (~((1UL<< (NR_CPUS%BITS_PER_LONG) - 1))
> +#else
> +#define __CPU_VALID_MASK__ (~0UL)
> +#endif
> -#define cpus_complement(map) do { map = ~(map); } while (0)
> +#define cpus_complement(map) \
> + do { map = ~(map) & __CPU_VALID_MASK__; } while (0)
> _instead_ of changing the several other macros to follow the
> bitmap convention (let the unused bits remain dont-care, until
> resolving a Boolean or scalar predicate).
You're missing the changes needed for cpus_shift_left() and
cpus_promote() to satisfy zeroed tail postconditions. IIRC the needed
changes to cpus_shift_left() are also missing from your other patches
in the bitmap code. You are also changing the invariants, which should
be the substance of a patch different from any bugfix.
-- wli
next prev parent reply other threads:[~2004-03-30 2:56 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2004-03-29 12:12 [PATCH] mask ADT: bitmap and bitop tweaks [1/22] Paul Jackson
2004-03-29 23:06 ` Matthew Dobson
2004-03-29 23:52 ` William Lee Irwin III
2004-03-29 23:43 ` Paul Jackson
2004-03-30 1:27 ` Matthew Dobson
2004-03-30 2:06 ` William Lee Irwin III
2004-03-30 1:46 ` Paul Jackson
2004-03-30 2:55 ` William Lee Irwin III [this message]
2004-03-30 5:09 ` Paul Jackson
2004-03-30 6:36 ` William Lee Irwin III
2004-03-30 8:00 ` Paul Jackson
2004-03-30 9:22 ` William Lee Irwin III
2004-03-29 23:50 ` Paul Jackson
2004-03-30 15:53 ` Chris Friesen
2004-03-30 18:30 ` Paul Jackson
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=20040330025535.GD791@holomorphy.com \
--to=wli@holomorphy.com \
--cc=akpm@osdl.org \
--cc=colpatch@us.ibm.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mbligh@aracnet.com \
--cc=pj@sgi.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.