Re: [PATCH 3/4] ndctl: switch to tools/include/linux/{kernel, list, bitmap}.h
* Dan Williams wrote: > On Wed, Jul 26, 2017 at 10:19 AM, Ingo Molnar wrote: > > > > * Dan Williams wrote: > > > >> On Wed, Jul 26, 2017 at 4:29 AM, Ingo Molnar wrote: > >> > > >> > * Dan Williams wrote: > >> > > >> >> On Tue, Jul 25, 2017 at 4:55 PM, Arnaldo Carvalho de Melo > >> >> wrote: > >> >> > Em Tue, Jul 25, 2017 at 03:36:26PM -0700, Dan Williams escreveu: > >> >> >> Replace the ccan implementation of list primitives, bitmap helpers > >> >> >> and > >> >> >> small utility macros with the common definitions available in > >> >> >> tool/include/linux. > >> >> > > >> >> > You should first add what you need in separate patches, paving the way > >> >> > to then use it, and some stuff are already there, see below: > >> >> > >> >> Ok, I'll break out those changes separately. > >> > > >> > BTW., another general observation I have is that ndctl uses autotools - > >> > while perf > >> > uses its own build system, some of which is abstracted out into > >> > tools/build/ and > >> > reused by other tooling projects as well. > >> > > >> > I despise autotools with a passion, it's slow, bloated, and encourages > >> > all sorts > >> > of bad API/ABI practices that plagues many OSS projects. I know that > >> > Linus > >> > explicitly did a Makefile based build system for Git for (I think) > >> > similar > >> > reasons. > >> > > >> > It might be a good idea to not let autotools into the kernel tooling > >> > tree, not > >> > because ndctl's use of autotools is bad in any fashion (it appears to be > >> > a fairly > >> > straightforward use), but to generally encourage good API/ABI practices > >> > in our > >> > tooling space, and to encourage enhancements to the tools/build/ > >> > infrastructure. > >> > >> That's a fair point. Regardless, autotools will be in the git history, > >> but if you'd like to see the final merge product eliminate its use, I > >> can't really argue otherwise. I was originally not concerned because > >> tools/usb/usbip/ was an existing in tree autotools user. In any event > >> if you want the autotools removal to be done out-of-tree I'll need to > >> put this effort on the back burner until 4.15. > > > > So that was another thing I wanted to suggest: why not import the current > > ndctl > > version as a single commit? > > > > I had a quick look, and there's quite a few of commits in the ndctl history > > that > > don't conform to kernel standards, such as: > > > > ce881c1e78f6: ndctl: seed tracking > > > > which doesn't have any Signed-off-by tags. > > > > There's also commits with ambiguous titles that would be confusing in the > > kernel > > context - for example: > > > > 796b6f373dec: clarify copyright and license information > > > > ... which on the surface could be misunderstood as something talking about > > the > > kernel copyright ... > > > > Or: > > > > e38bd36e5d0a: completion: updates for file name completion > > > > which I could initially mistake for a commit about scheduler completions ;-) > > > > Or: > > > > 2ad6a39c9ae9: Fix attribute sizes to match NFIT 0.8s2 > > cc7cb44385d3: Import initial infrastructure > > > > etc. > > > > I suppose all that could be corrected, SOBs added, titles clarified and > > prefixed > > with tools/ndctl, but then it wouldn't really be unmodified history anymore, > > right? > > > > At that point we might as well do a clean start - and not import ~500 extra > > commits into the kernel tree? > > I think keeping the history is worth it, similar reasoning to why we > kept the btrfs history. Well, there's two key differences: - btrfs _is_ kernel code and as such was developed as a kernel module, albeit out of tree. ndctl is a standalone tooling project. - all the old btrfs commit titles are either prefixed with 'btrfs:', or have it as a string in the title, which made the merge unproblematic and unambiguous. > [...] Also, since the history is linear and already rewritten by 'git > filter-branch' to move everything to tools/ndctl/ it wouldn't be that much > more > work to go clean up these few commits that are problematic. So I think that's where to draw the line - grafting two kernel tree commit histories like for btrfs might be OK (just the repositories were incompatible), but if commit logs need to be rewritten manually then that's essentially whitewashing of history - which is a big no-no in Git flows. Anyway, it's up to Linus I guess. Thanks, Ingo ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 3/4] ndctl: switch to tools/include/linux/{kernel, list, bitmap}.h
On Wed, Jul 26, 2017 at 10:57 AM, Arnaldo Carvalho de Melo wrote: > Em Wed, Jul 26, 2017 at 10:31:48AM -0700, Dan Williams escreveu: >> On Wed, Jul 26, 2017 at 10:19 AM, Ingo Molnar wrote: >> > * Dan Williams wrote: >> >> On Wed, Jul 26, 2017 at 4:29 AM, Ingo Molnar wrote: >> > which I could initially mistake for a commit about scheduler completions >> > ;-) >> > >> > Or: > >> > 2ad6a39c9ae9: Fix attribute sizes to match NFIT 0.8s2 >> > cc7cb44385d3: Import initial infrastructure > >> > etc. > >> > I suppose all that could be corrected, SOBs added, titles clarified and >> > prefixed >> > with tools/ndctl, but then it wouldn't really be unmodified history >> > anymore, >> > right? > >> > At that point we might as well do a clean start - and not import ~500 extra >> > commits into the kernel tree? > >> I think keeping the history is worth it, similar reasoning to why we >> kept the btrfs history. Also, since the history is linear and already >> rewritten by 'git filter-branch' to move everything to tools/ndctl/ it >> wouldn't be that much more work to go clean up these few commits that >> are problematic. > > Hey, but we can have something like: > > https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/ > > It goes all the way to the 0.12 kernel. > > If someone wants to go that far, just clone that tree, merge it, etc. > > Could that be the solution for this case as well? > > I.e. the kernel history prior to what we have in linux.git can be > accessed via that other tree, history.git, can we have ndctl-history.git > and linux.git + ndctl from now on? That will have to be the option if Linus finds the history unpalatable, but reading the comedi inclusion thread [1] I came away with this, quoting Linus: --- I'd _like_ for old history to be merged, but quite frankly, bisectability is a fairly big deal, and while we often have cases where a _few_ commits don't build and make bisecting hard, if you import the past development history badly, you can easily end up with _hundreds_ of commits that simply don't build as a kernel at all. And at some point the "nice to have" history is suddenly "more pain than it is worth". --- So, the bigger reason to not include the full history is the potential for 'git bisect' confusion where you end up in the ndctl history and the rest of the kernel tree disappears. > If you prefer to go full merge, then I think prefixing + SOB'ing should > be done, right? Yes, definitely, and it's otherwise simple enough to switch to the forklift import at the last moment if it comes to that. [1]: https://lkml.org/lkml/2008/10/30/356 ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 3/4] ndctl: switch to tools/include/linux/{kernel, list, bitmap}.h
Em Wed, Jul 26, 2017 at 10:31:48AM -0700, Dan Williams escreveu: > On Wed, Jul 26, 2017 at 10:19 AM, Ingo Molnar wrote: > > * Dan Williams wrote: > >> On Wed, Jul 26, 2017 at 4:29 AM, Ingo Molnar wrote: > > which I could initially mistake for a commit about scheduler completions ;-) > > > > Or: > > 2ad6a39c9ae9: Fix attribute sizes to match NFIT 0.8s2 > > cc7cb44385d3: Import initial infrastructure > > etc. > > I suppose all that could be corrected, SOBs added, titles clarified and > > prefixed > > with tools/ndctl, but then it wouldn't really be unmodified history anymore, > > right? > > At that point we might as well do a clean start - and not import ~500 extra > > commits into the kernel tree? > I think keeping the history is worth it, similar reasoning to why we > kept the btrfs history. Also, since the history is linear and already > rewritten by 'git filter-branch' to move everything to tools/ndctl/ it > wouldn't be that much more work to go clean up these few commits that > are problematic. Hey, but we can have something like: https://git.kernel.org/pub/scm/linux/kernel/git/history/history.git/ It goes all the way to the 0.12 kernel. If someone wants to go that far, just clone that tree, merge it, etc. Could that be the solution for this case as well? I.e. the kernel history prior to what we have in linux.git can be accessed via that other tree, history.git, can we have ndctl-history.git and linux.git + ndctl from now on? If you prefer to go full merge, then I think prefixing + SOB'ing should be done, right? - Arnaldo ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 3/4] ndctl: switch to tools/include/linux/{kernel, list, bitmap}.h
On Wed, Jul 26, 2017 at 10:19 AM, Ingo Molnar wrote: > > * Dan Williams wrote: > >> On Wed, Jul 26, 2017 at 4:29 AM, Ingo Molnar wrote: >> > >> > * Dan Williams wrote: >> > >> >> On Tue, Jul 25, 2017 at 4:55 PM, Arnaldo Carvalho de Melo >> >> wrote: >> >> > Em Tue, Jul 25, 2017 at 03:36:26PM -0700, Dan Williams escreveu: >> >> >> Replace the ccan implementation of list primitives, bitmap helpers and >> >> >> small utility macros with the common definitions available in >> >> >> tool/include/linux. >> >> > >> >> > You should first add what you need in separate patches, paving the way >> >> > to then use it, and some stuff are already there, see below: >> >> >> >> Ok, I'll break out those changes separately. >> > >> > BTW., another general observation I have is that ndctl uses autotools - >> > while perf >> > uses its own build system, some of which is abstracted out into >> > tools/build/ and >> > reused by other tooling projects as well. >> > >> > I despise autotools with a passion, it's slow, bloated, and encourages all >> > sorts >> > of bad API/ABI practices that plagues many OSS projects. I know that Linus >> > explicitly did a Makefile based build system for Git for (I think) similar >> > reasons. >> > >> > It might be a good idea to not let autotools into the kernel tooling tree, >> > not >> > because ndctl's use of autotools is bad in any fashion (it appears to be a >> > fairly >> > straightforward use), but to generally encourage good API/ABI practices in >> > our >> > tooling space, and to encourage enhancements to the tools/build/ >> > infrastructure. >> >> That's a fair point. Regardless, autotools will be in the git history, >> but if you'd like to see the final merge product eliminate its use, I >> can't really argue otherwise. I was originally not concerned because >> tools/usb/usbip/ was an existing in tree autotools user. In any event >> if you want the autotools removal to be done out-of-tree I'll need to >> put this effort on the back burner until 4.15. > > So that was another thing I wanted to suggest: why not import the current > ndctl > version as a single commit? > > I had a quick look, and there's quite a few of commits in the ndctl history > that > don't conform to kernel standards, such as: > > ce881c1e78f6: ndctl: seed tracking > > which doesn't have any Signed-off-by tags. > > There's also commits with ambiguous titles that would be confusing in the > kernel > context - for example: > > 796b6f373dec: clarify copyright and license information > > ... which on the surface could be misunderstood as something talking about the > kernel copyright ... > > Or: > > e38bd36e5d0a: completion: updates for file name completion > > which I could initially mistake for a commit about scheduler completions ;-) > > Or: > > 2ad6a39c9ae9: Fix attribute sizes to match NFIT 0.8s2 > cc7cb44385d3: Import initial infrastructure > > etc. > > I suppose all that could be corrected, SOBs added, titles clarified and > prefixed > with tools/ndctl, but then it wouldn't really be unmodified history anymore, > right? > > At that point we might as well do a clean start - and not import ~500 extra > commits into the kernel tree? I think keeping the history is worth it, similar reasoning to why we kept the btrfs history. Also, since the history is linear and already rewritten by 'git filter-branch' to move everything to tools/ndctl/ it wouldn't be that much more work to go clean up these few commits that are problematic. ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 3/4] ndctl: switch to tools/include/linux/{kernel, list, bitmap}.h
* Dan Williams wrote: > On Wed, Jul 26, 2017 at 4:29 AM, Ingo Molnar wrote: > > > > * Dan Williams wrote: > > > >> On Tue, Jul 25, 2017 at 4:55 PM, Arnaldo Carvalho de Melo > >> wrote: > >> > Em Tue, Jul 25, 2017 at 03:36:26PM -0700, Dan Williams escreveu: > >> >> Replace the ccan implementation of list primitives, bitmap helpers and > >> >> small utility macros with the common definitions available in > >> >> tool/include/linux. > >> > > >> > You should first add what you need in separate patches, paving the way > >> > to then use it, and some stuff are already there, see below: > >> > >> Ok, I'll break out those changes separately. > > > > BTW., another general observation I have is that ndctl uses autotools - > > while perf > > uses its own build system, some of which is abstracted out into > > tools/build/ and > > reused by other tooling projects as well. > > > > I despise autotools with a passion, it's slow, bloated, and encourages all > > sorts > > of bad API/ABI practices that plagues many OSS projects. I know that Linus > > explicitly did a Makefile based build system for Git for (I think) similar > > reasons. > > > > It might be a good idea to not let autotools into the kernel tooling tree, > > not > > because ndctl's use of autotools is bad in any fashion (it appears to be a > > fairly > > straightforward use), but to generally encourage good API/ABI practices in > > our > > tooling space, and to encourage enhancements to the tools/build/ > > infrastructure. > > That's a fair point. Regardless, autotools will be in the git history, > but if you'd like to see the final merge product eliminate its use, I > can't really argue otherwise. I was originally not concerned because > tools/usb/usbip/ was an existing in tree autotools user. In any event > if you want the autotools removal to be done out-of-tree I'll need to > put this effort on the back burner until 4.15. So that was another thing I wanted to suggest: why not import the current ndctl version as a single commit? I had a quick look, and there's quite a few of commits in the ndctl history that don't conform to kernel standards, such as: ce881c1e78f6: ndctl: seed tracking which doesn't have any Signed-off-by tags. There's also commits with ambiguous titles that would be confusing in the kernel context - for example: 796b6f373dec: clarify copyright and license information ... which on the surface could be misunderstood as something talking about the kernel copyright ... Or: e38bd36e5d0a: completion: updates for file name completion which I could initially mistake for a commit about scheduler completions ;-) Or: 2ad6a39c9ae9: Fix attribute sizes to match NFIT 0.8s2 cc7cb44385d3: Import initial infrastructure etc. I suppose all that could be corrected, SOBs added, titles clarified and prefixed with tools/ndctl, but then it wouldn't really be unmodified history anymore, right? At that point we might as well do a clean start - and not import ~500 extra commits into the kernel tree? Thanks, Ingo ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 3/4] ndctl: switch to tools/include/linux/{kernel, list, bitmap}.h
On Wed, Jul 26, 2017 at 4:29 AM, Ingo Molnar wrote: > > * Dan Williams wrote: > >> On Tue, Jul 25, 2017 at 4:55 PM, Arnaldo Carvalho de Melo >> wrote: >> > Em Tue, Jul 25, 2017 at 03:36:26PM -0700, Dan Williams escreveu: >> >> Replace the ccan implementation of list primitives, bitmap helpers and >> >> small utility macros with the common definitions available in >> >> tool/include/linux. >> > >> > You should first add what you need in separate patches, paving the way >> > to then use it, and some stuff are already there, see below: >> >> Ok, I'll break out those changes separately. > > BTW., another general observation I have is that ndctl uses autotools - while > perf > uses its own build system, some of which is abstracted out into tools/build/ > and > reused by other tooling projects as well. > > I despise autotools with a passion, it's slow, bloated, and encourages all > sorts > of bad API/ABI practices that plagues many OSS projects. I know that Linus > explicitly did a Makefile based build system for Git for (I think) similar > reasons. > > It might be a good idea to not let autotools into the kernel tooling tree, not > because ndctl's use of autotools is bad in any fashion (it appears to be a > fairly > straightforward use), but to generally encourage good API/ABI practices in our > tooling space, and to encourage enhancements to the tools/build/ > infrastructure. That's a fair point. Regardless, autotools will be in the git history, but if you'd like to see the final merge product eliminate its use, I can't really argue otherwise. I was originally not concerned because tools/usb/usbip/ was an existing in tree autotools user. In any event if you want the autotools removal to be done out-of-tree I'll need to put this effort on the back burner until 4.15. ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 3/4] ndctl: switch to tools/include/linux/{kernel, list, bitmap}.h
* Dan Williams wrote: > On Tue, Jul 25, 2017 at 4:55 PM, Arnaldo Carvalho de Melo > wrote: > > Em Tue, Jul 25, 2017 at 03:36:26PM -0700, Dan Williams escreveu: > >> Replace the ccan implementation of list primitives, bitmap helpers and > >> small utility macros with the common definitions available in > >> tool/include/linux. > > > > You should first add what you need in separate patches, paving the way > > to then use it, and some stuff are already there, see below: > > Ok, I'll break out those changes separately. BTW., another general observation I have is that ndctl uses autotools - while perf uses its own build system, some of which is abstracted out into tools/build/ and reused by other tooling projects as well. I despise autotools with a passion, it's slow, bloated, and encourages all sorts of bad API/ABI practices that plagues many OSS projects. I know that Linus explicitly did a Makefile based build system for Git for (I think) similar reasons. It might be a good idea to not let autotools into the kernel tooling tree, not because ndctl's use of autotools is bad in any fashion (it appears to be a fairly straightforward use), but to generally encourage good API/ABI practices in our tooling space, and to encourage enhancements to the tools/build/ infrastructure. Thanks, Ingo ___ Linux-nvdimm mailing list Linux-nvdimm@lists.01.org https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 3/4] ndctl: switch to tools/include/linux/{kernel, list, bitmap}.h
On Tue, Jul 25, 2017 at 4:55 PM, Arnaldo Carvalho de Melo
wrote:
> Em Tue, Jul 25, 2017 at 03:36:26PM -0700, Dan Williams escreveu:
>> Replace the ccan implementation of list primitives, bitmap helpers and
>> small utility macros with the common definitions available in
>> tool/include/linux.
>
> You should first add what you need in separate patches, paving the way
> to then use it, and some stuff are already there, see below:
Ok, I'll break out those changes separately.
>
>> diff --git a/tools/include/linux/hashtable.h
>> b/tools/include/linux/hashtable.h
>> index c65cc0aa2659..251eabf2a05e 100644
>> --- a/tools/include/linux/hashtable.h
>> +++ b/tools/include/linux/hashtable.h
>> @@ -13,10 +13,6 @@
>> #include
>> #include
>>
>> -#ifndef ARRAY_SIZE
>> -#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>> -#endif
>> -
>
> This is already tools/include/linux/kernel.h as
>
> #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) +
> __must_be_array(arr))
>
> It was moved by:
>
> commit 68289cbd83eaa20faef7cc818121bc8e769065de
> Author: Arnaldo Carvalho de Melo
> Date: Mon Apr 17 12:01:36 2017 -0300
>
> tools include: Drop ARRAY_SIZE() definition from linux/hashtable.h
>
> As tools/include/linux/kernel.h has it now, with the goodies present in
> the kernel.h counterpart, i.e. checking that the parameter is an array
> at build time.
>
>
>
>
>> #define DEFINE_HASHTABLE(name, bits)
>> \
>> struct hlist_head name[1 << (bits)] =
>> \
>> { [0 ... ((1 << (bits)) - 1)] = HLIST_HEAD_INIT }
>> diff --git a/tools/include/linux/kernel.h b/tools/include/linux/kernel.h
>> index 28607db02bd3..ebe5211ce086 100644
>> --- a/tools/include/linux/kernel.h
>> +++ b/tools/include/linux/kernel.h
>> @@ -45,6 +45,10 @@
>> _min1 < _min2 ? _min1 : _min2; })
>> #endif
>>
>> +#ifndef clamp
>> +#define clamp(v, f, c) (max(min((v), (c)), (f)))
>> +#endif
>> +
>
> The include/linux/kernel.h definition is:
>
>
> /**
> * clamp - return a value clamped to a given range with strict typechecking
> * @val: current value
> * @lo: lowest allowable value
> * @hi: highest allowable value
> *
> * This macro does strict typechecking of lo/hi to make sure they are of the
> * same type as val. See the unnecessary pointer comparisons.
> */
> #define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi)
>
> I suggest we try to track the kernel counterparts as much as possible, and
> put the definitions in the same order as in the kernel header, this way we
> can even look at how different they are using plain old 'diff'.
Sounds good.
>
>> #ifndef roundup
>> #define roundup(x, y) (\
>> { \
>> @@ -54,6 +58,10 @@
>> )
>> #endif
>>
>> +#ifndef ARRAY_SIZE
>> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
>> +#endif
>> +
>> #ifndef BUG_ON
>> #ifdef NDEBUG
>> #define BUG_ON(cond) do { if (cond) {} } while (0)
>> @@ -66,8 +74,10 @@
>> * Both need more care to handle endianness
>> * (Don't use bitmap_copy_le() for now)
>> */
>> +#ifndef cpu_to_le64
>> #define cpu_to_le64(x) (x)
>> #define cpu_to_le32(x) (x)
>> +#endif
>
> This will not apply, there were changes here as well.
I should have mentioned that this tree was based on 4.10. I picked an
old merge base on the thought that it would reduce the chance of
people bisecting 4.13..4.14-rc1 ending up in the initial ndctl history
where only tools/ndctl/ exists. But, if I want to touch kernel.h this
old baseline is not going to work...
I'll play around with git bisect to see if the old merge base is
actually worth it, and roll forward otherwise.
___
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
Re: [PATCH 3/4] ndctl: switch to tools/include/linux/{kernel, list, bitmap}.h
Em Tue, Jul 25, 2017 at 03:36:26PM -0700, Dan Williams escreveu:
> Replace the ccan implementation of list primitives, bitmap helpers and
> small utility macros with the common definitions available in
> tool/include/linux.
You should first add what you need in separate patches, paving the way
to then use it, and some stuff are already there, see below:
> diff --git a/tools/include/linux/hashtable.h b/tools/include/linux/hashtable.h
> index c65cc0aa2659..251eabf2a05e 100644
> --- a/tools/include/linux/hashtable.h
> +++ b/tools/include/linux/hashtable.h
> @@ -13,10 +13,6 @@
> #include
> #include
>
> -#ifndef ARRAY_SIZE
> -#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> -#endif
> -
This is already tools/include/linux/kernel.h as
#define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
It was moved by:
commit 68289cbd83eaa20faef7cc818121bc8e769065de
Author: Arnaldo Carvalho de Melo
Date: Mon Apr 17 12:01:36 2017 -0300
tools include: Drop ARRAY_SIZE() definition from linux/hashtable.h
As tools/include/linux/kernel.h has it now, with the goodies present in
the kernel.h counterpart, i.e. checking that the parameter is an array
at build time.
> #define DEFINE_HASHTABLE(name, bits)
> \
> struct hlist_head name[1 << (bits)] =
> \
> { [0 ... ((1 << (bits)) - 1)] = HLIST_HEAD_INIT }
> diff --git a/tools/include/linux/kernel.h b/tools/include/linux/kernel.h
> index 28607db02bd3..ebe5211ce086 100644
> --- a/tools/include/linux/kernel.h
> +++ b/tools/include/linux/kernel.h
> @@ -45,6 +45,10 @@
> _min1 < _min2 ? _min1 : _min2; })
> #endif
>
> +#ifndef clamp
> +#define clamp(v, f, c) (max(min((v), (c)), (f)))
> +#endif
> +
The include/linux/kernel.h definition is:
/**
* clamp - return a value clamped to a given range with strict typechecking
* @val: current value
* @lo: lowest allowable value
* @hi: highest allowable value
*
* This macro does strict typechecking of lo/hi to make sure they are of the
* same type as val. See the unnecessary pointer comparisons.
*/
#define clamp(val, lo, hi) min((typeof(val))max(val, lo), hi)
I suggest we try to track the kernel counterparts as much as possible, and
put the definitions in the same order as in the kernel header, this way we
can even look at how different they are using plain old 'diff'.
> #ifndef roundup
> #define roundup(x, y) (\
> { \
> @@ -54,6 +58,10 @@
> )
> #endif
>
> +#ifndef ARRAY_SIZE
> +#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
> +#endif
> +
> #ifndef BUG_ON
> #ifdef NDEBUG
> #define BUG_ON(cond) do { if (cond) {} } while (0)
> @@ -66,8 +74,10 @@
> * Both need more care to handle endianness
> * (Don't use bitmap_copy_le() for now)
> */
> +#ifndef cpu_to_le64
> #define cpu_to_le64(x) (x)
> #define cpu_to_le32(x) (x)
> +#endif
This will not apply, there were changes here as well.
> int vscnprintf(char *buf, size_t size, const char *fmt, va_list args);
> int scnprintf(char * buf, size_t size, const char * fmt, ...);
> diff --git a/tools/ndctl/Makefile.am b/tools/ndctl/Makefile.am
> index ba81e8c3d5bb..2803ec51a0b2 100644
> --- a/tools/ndctl/Makefile.am
> +++ b/tools/ndctl/Makefile.am
> @@ -48,18 +48,15 @@ libccan_a_SOURCES = \
> ccan/str/str_debug.h \
> ccan/str/str.c \
> ccan/str/debug.c \
> - ccan/list/list.h \
> - ccan/list/list.c \
> ccan/container_of/container_of.h \
> ccan/check_type/check_type.h \
> ccan/build_assert/build_assert.h \
> - ccan/array_size/array_size.h \
> - ccan/minmax/minmax.h \
> ccan/short_types/short_types.h \
> ccan/endian/endian.h
>
> noinst_LIBRARIES += libutil.a
> libutil_a_SOURCES = \
> + ../lib/find_bit.c \
> util/parse-options.c \
> util/parse-options.h \
> util/usage.c \
> @@ -69,7 +66,6 @@ libutil_a_SOURCES = \
> util/strbuf.c \
> util/wrapper.c \
> util/filter.c \
> - util/fletcher.c\
> - util/bitmap.c
> + util/fletcher.c
>
> nobase_include_HEADERS = daxctl/libdaxctl.h
> diff --git a/tools/ndctl/Makefile.am.in b/tools/ndctl/Makefile.am.in
> index 9cb8d4a055c7..1680b738578d 100644
> --- a/tools/ndctl/Makefile.am.in
> +++ b/tools/ndctl/Makefile.am.in
> @@ -9,6 +9,7 @@ AM_CPPFLAGS = \
> -DLIBEXECDIR=\""$(libexecdir)"\" \
> -DPREFIX=\""$(prefix)"\" \
> -DNDCTL_MAN_PATH=\""$(mandir)"\" \
> + -I${top_srcdir}/../include \
> -I${top_srcdir}/ndctl/lib \
> -I${top_srcdir}/ndctl \
> -I${top_srcdir}/ \
> diff --git a/tools/ndctl/ccan/array_size/LICENSE
> b/tools/ndctl/ccan/array_size/LICENSE
> deleted file mode 12
> index b7951dabdc82..
> --- a/tools/ndctl/ccan/array_size/LICENSE
> +++ /dev/null
> @@ -1 +0,0 @@
> -../../license
[PATCH 3/4] ndctl: switch to tools/include/linux/{kernel, list, bitmap}.h
Replace the ccan implementation of list primitives, bitmap helpers and
small utility macros with the common definitions available in
tool/include/linux.
Cc: Peter Zijlstra
Cc: Ingo Molnar
Cc: Arnaldo Carvalho de Melo
Cc: Alexander Shishkin
Signed-off-by: Dan Williams
---
tools/include/linux/hashtable.h |4
tools/include/linux/kernel.h | 10
tools/ndctl/Makefile.am |8
tools/ndctl/Makefile.am.in |1
tools/ndctl/ccan/array_size/LICENSE |1
tools/ndctl/ccan/array_size/array_size.h | 26 -
tools/ndctl/ccan/container_of/LICENSE|1
tools/ndctl/ccan/container_of/container_of.h | 109
tools/ndctl/ccan/list/LICENSE|1
tools/ndctl/ccan/list/list.c | 43 --
tools/ndctl/ccan/list/list.h | 656 --
tools/ndctl/ccan/minmax/LICENSE |1
tools/ndctl/ccan/minmax/minmax.h | 65 ---
tools/ndctl/configure.ac |1
tools/ndctl/daxctl/daxctl.c |2
tools/ndctl/daxctl/lib/libdaxctl-private.h |4
tools/ndctl/daxctl/lib/libdaxctl.c | 20 -
tools/ndctl/daxctl/list.c|2
tools/ndctl/ndctl/check.c| 16 -
tools/ndctl/ndctl/create-nfit.c | 13 -
tools/ndctl/ndctl/dimm.c | 12
tools/ndctl/ndctl/lib/libndctl-private.h |6
tools/ndctl/ndctl/lib/libndctl-smart.c |5
tools/ndctl/ndctl/lib/libndctl.c | 116 ++---
tools/ndctl/ndctl/list.c |2
tools/ndctl/ndctl/namespace.c|3
tools/ndctl/ndctl/ndctl.c|2
tools/ndctl/ndctl/util/json-smart.c |2
tools/ndctl/test/blk_namespaces.c|2
tools/ndctl/test/core.c |2
tools/ndctl/test/daxdev-errors.c |2
tools/ndctl/test/device-dax.c|2
tools/ndctl/test/dpa-alloc.c |2
tools/ndctl/test/dsm-fail.c |2
tools/ndctl/test/libndctl.c |3
tools/ndctl/test/multi-pmem.c|2
tools/ndctl/test/pmem_namespaces.c |3
tools/ndctl/util/bitmap.c| 131 -
tools/ndctl/util/bitmap.h| 44 --
tools/ndctl/util/json.c |2
tools/ndctl/util/kernel.h|9
tools/ndctl/util/list.h | 24 +
tools/ndctl/util/parse-options.h |1
tools/ndctl/util/size.h |1
tools/ndctl/util/util.h |1
tools/perf/util/util.h |2
46 files changed, 163 insertions(+), 1204 deletions(-)
delete mode 12 tools/ndctl/ccan/array_size/LICENSE
delete mode 100644 tools/ndctl/ccan/array_size/array_size.h
delete mode 12 tools/ndctl/ccan/container_of/LICENSE
delete mode 100644 tools/ndctl/ccan/container_of/container_of.h
delete mode 12 tools/ndctl/ccan/list/LICENSE
delete mode 100644 tools/ndctl/ccan/list/list.c
delete mode 100644 tools/ndctl/ccan/list/list.h
delete mode 12 tools/ndctl/ccan/minmax/LICENSE
delete mode 100644 tools/ndctl/ccan/minmax/minmax.h
delete mode 100644 tools/ndctl/util/bitmap.c
delete mode 100644 tools/ndctl/util/bitmap.h
create mode 100644 tools/ndctl/util/kernel.h
create mode 100644 tools/ndctl/util/list.h
diff --git a/tools/include/linux/hashtable.h b/tools/include/linux/hashtable.h
index c65cc0aa2659..251eabf2a05e 100644
--- a/tools/include/linux/hashtable.h
+++ b/tools/include/linux/hashtable.h
@@ -13,10 +13,6 @@
#include
#include
-#ifndef ARRAY_SIZE
-#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
-#endif
-
#define DEFINE_HASHTABLE(name, bits)
\
struct hlist_head name[1 << (bits)] =
\
{ [0 ... ((1 << (bits)) - 1)] = HLIST_HEAD_INIT }
diff --git a/tools/include/linux/kernel.h b/tools/include/linux/kernel.h
index 28607db02bd3..ebe5211ce086 100644
--- a/tools/include/linux/kernel.h
+++ b/tools/include/linux/kernel.h
@@ -45,6 +45,10 @@
_min1 < _min2 ? _min1 : _min2; })
#endif
+#ifndef clamp
+#define clamp(v, f, c) (max(min((v), (c)), (f)))
+#endif
+
#ifndef roundup
#define roundup(x, y) (\
{ \
@@ -54,6 +58,10 @@
)
#endif
+#ifndef ARRAY_SIZE
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
+#endif
+
#ifndef BUG_ON
#ifdef NDEBUG
#define BUG_ON(cond) do { if (cond) {} } while (0)
@@ -66,8 +74,10 @@
* Both need more care to handle endianness
* (Don't use bitmap_copy_le() for now)
*/
+#ifndef cpu_to_le64
#define cpu_to_le64(x) (x)
#define cpu_to_le32(x) (x)
+#endif
int vscnprintf(char *buf, size_t siz
