| Message ID | 1511963726-34070-2-git-send-email-wei.w.wang@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Nov 29, 2017 at 09:55:17PM +0800, Wei Wang wrote: > The <linux/bug.h> was removed from radix-tree.h by the following commit: > f5bba9d11a256ad2a1c2f8e7fc6aabe6416b7890. > > Since that commit, tools/testing/radix-tree/ couldn't pass compilation > due to: tools/testing/radix-tree/idr.c:17: undefined reference to > WARN_ON_ONCE. This patch adds the bug.h header to idr.h to solve the > issue. Thanks; I sent this same patch out yesterday. Unfortunately, you didn't cc the author of this breakage, Masahiro Yamada. I want to highlight that these kinds of header cleanups are risky, and very low reward. I really don't want to see patches going all over the tree randomly touching header files. If we've got a real problem to solve, then sure. But I want to see a strong justification for any more header file cleanups.
On Wed 29-11-17 16:58:17, Matthew Wilcox wrote: > On Wed, Nov 29, 2017 at 09:55:17PM +0800, Wei Wang wrote: > > The <linux/bug.h> was removed from radix-tree.h by the following commit: > > f5bba9d11a256ad2a1c2f8e7fc6aabe6416b7890. > > > > Since that commit, tools/testing/radix-tree/ couldn't pass compilation > > due to: tools/testing/radix-tree/idr.c:17: undefined reference to > > WARN_ON_ONCE. This patch adds the bug.h header to idr.h to solve the > > issue. > > Thanks; I sent this same patch out yesterday. > > Unfortunately, you didn't cc the author of this breakage, Masahiro Yamada. > I want to highlight that these kinds of header cleanups are risky, > and very low reward. I really don't want to see patches going all over > the tree randomly touching header files. If we've got a real problem > to solve, then sure. But I want to see a strong justification for any > more header file cleanups. I agree. It usually requires unexpected combination of config options to uncover some nasty include dependencies. So these patches might break build while their additional value is quite questionable.
On Wed, 29 Nov 2017 16:58:17 -0800 Matthew Wilcox <willy@infradead.org> wrote: > On Wed, Nov 29, 2017 at 09:55:17PM +0800, Wei Wang wrote: > > The <linux/bug.h> was removed from radix-tree.h by the following commit: > > f5bba9d11a256ad2a1c2f8e7fc6aabe6416b7890. > > > > Since that commit, tools/testing/radix-tree/ couldn't pass compilation > > due to: tools/testing/radix-tree/idr.c:17: undefined reference to > > WARN_ON_ONCE. This patch adds the bug.h header to idr.h to solve the > > issue. > > Thanks; I sent this same patch out yesterday. > > Unfortunately, you didn't cc the author of this breakage, Masahiro Yamada. > I want to highlight that these kinds of header cleanups are risky, > and very low reward. I really don't want to see patches going all over > the tree randomly touching header files. If we've got a real problem > to solve, then sure. But I want to see a strong justification for any > more header file cleanups. I tend to disagree. We accumulate more and more cruft over time so it is good to be continually hacking away at it. These little build breaks happen occasionally but they are trivially and quickly fixed. If a small minority of these cleanups require a followup patch which consumes a global ten person minutes then that seems an acceptable price to pay. Says the guy who pays most of that price :)
diff --git a/include/linux/idr.h b/include/linux/idr.h index 7c3a365..fa14f83 100644 --- a/include/linux/idr.h +++ b/include/linux/idr.h @@ -15,6 +15,7 @@ #include <linux/radix-tree.h> #include <linux/gfp.h> #include <linux/percpu.h> +#include <linux/bug.h> struct idr { struct radix_tree_root idr_rt;
The <linux/bug.h> was removed from radix-tree.h by the following commit: f5bba9d11a256ad2a1c2f8e7fc6aabe6416b7890. Since that commit, tools/testing/radix-tree/ couldn't pass compilation due to: tools/testing/radix-tree/idr.c:17: undefined reference to WARN_ON_ONCE. This patch adds the bug.h header to idr.h to solve the issue. Signed-off-by: Wei Wang <wei.w.wang@intel.com> Cc: Matthew Wilcox <mawilcox@microsoft.com> Cc: Andrew Morton <akpm@linux-foundation.org> Cc: Jan Kara <jack@suse.cz> Cc: Eric Biggers <ebiggers@google.com> Cc: Tejun Heo <tj@kernel.org> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> --- include/linux/idr.h | 1 + 1 file changed, 1 insertion(+)