clang-7 has a new warning (-Wreturn-stack-address) for warning when a function returns the address of a local variable. This is in general a good warning, but the kernel has a few places where GNU statement expressions return the address of a label in order to get the current instruction pointer (see _THIS_IP_ and current_text_addr).
In order to disable a warning at a single call site, the kernel already has __diag macros for inserting compiler and compiler-version specific _Pragma's.
This series adds CLANG_VERSION macros necessary for proper __diag support, and whitelists the case in _THIS_IP_. current_text_addr will be consolidated in a follow up series.
Nick Desaulniers (2): compiler-clang.h: Add CLANG_VERSION and __diag macros kernel.h: Disable -Wreturn-stack-address for _THIS_IP_
include/linux/compiler-clang.h | 19 +++++++++++++++++++ include/linux/compiler_types.h | 4 ++++ include/linux/kernel.h | 10 +++++++++- 3 files changed, 32 insertions(+), 1 deletion(-)
These are needed for doing proper version checks, though feature detection via __has_attribute, __has_builtin, and __has_feature should be preferred, see: https://clang.llvm.org/docs/LanguageExtensions.html#feature-checking-macros
Also adds __diag support, for generating compiler version specific _Pragma()'s.
__diag support based on commit 8793bb7f4a9d ("kbuild: add macro for controlling warnings to linux/compiler.h")
Cc: stable@vger.kernel.org # 4.17, 4.14, 4.9, 4.4 Suggested-by: Nathan Chancellor natechancellor@gmail.com Signed-off-by: Nick Desaulniers ndesaulniers@google.com --- include/linux/compiler-clang.h | 19 +++++++++++++++++++ include/linux/compiler_types.h | 4 ++++ 2 files changed, 23 insertions(+)
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h index 7087446c24c8..9442e07a361e 100644 --- a/include/linux/compiler-clang.h +++ b/include/linux/compiler-clang.h @@ -7,6 +7,10 @@ * for Clang compiler */
+#define CLANG_VERSION (__clang_major__ * 10000 \ + + __clang_minor__ * 100 \ + + __clang_patchlevel__) + #ifdef uninitialized_var #undef uninitialized_var #define uninitialized_var(x) x = *(&(x)) @@ -46,3 +50,18 @@ __has_builtin(__builtin_sub_overflow) #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1 #endif + +#define __diag_str1(s) #s +#define __diag_str(s) __diag_str1(s) +#define __diag(s) _Pragma(__diag_str(clang diagnostic s)) +#define __diag_CLANG_ignore ignored +#define __diag_CLANG_warn warning +#define __diag_CLANG_error error +#define __diag_CLANG(version, severity, s) \ + __diag_CLANG_ ## version(__diag_CLANG_ ## severity s) + +#if CLANG_VERSION >= 70000 +#define __diag_CLANG_7(s) __diag(s) +#else +#define __diag_CLANG_7(s) +#endif diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index a8ba6b04152c..a04e6bd63476 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -279,6 +279,10 @@ struct ftrace_likely_data { #define __diag_GCC(version, severity, string) #endif
+#ifndef __diag_CLANG +#define __diag_CLANG(version, severity, string) +#endif + #define __diag_push() __diag(push) #define __diag_pop() __diag(pop)
On Mon, Jul 30, 2018 at 02:34:11PM -0700, Nick Desaulniers wrote:
These are needed for doing proper version checks, though feature detection via __has_attribute, __has_builtin, and __has_feature should be preferred, see: https://clang.llvm.org/docs/LanguageExtensions.html#feature-checking-macros
Also adds __diag support, for generating compiler version specific _Pragma()'s.
__diag support based on commit 8793bb7f4a9d ("kbuild: add macro for controlling warnings to linux/compiler.h")
Cc: stable@vger.kernel.org # 4.17, 4.14, 4.9, 4.4 Suggested-by: Nathan Chancellor natechancellor@gmail.com Signed-off-by: Nick Desaulniers ndesaulniers@google.com
Tested-and-reviewed-by: Nathan Chancellor natechancellor@gmail.com
include/linux/compiler-clang.h | 19 +++++++++++++++++++ include/linux/compiler_types.h | 4 ++++ 2 files changed, 23 insertions(+)
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h index 7087446c24c8..9442e07a361e 100644 --- a/include/linux/compiler-clang.h +++ b/include/linux/compiler-clang.h @@ -7,6 +7,10 @@
- for Clang compiler
*/ +#define CLANG_VERSION (__clang_major__ * 10000 \
+ __clang_minor__ * 100 \+ __clang_patchlevel__)
I know this was done to be consistent with GCC_VERSION but certain toolchains may set the patch level to the SVN revision (at least that's what it appears to be) like AOSP's clang-4053586, which would make CLANG_VERSION 350080. For this particular warning, it doesn't matter since Clang has supported it since 2011 and Clang 5.0 isn't supported in this tree anyways but maybe relying on just __clang_major__ below is more ideal.
Other than that, patch functions just as it should.
#ifdef uninitialized_var #undef uninitialized_var #define uninitialized_var(x) x = *(&(x)) @@ -46,3 +50,18 @@ __has_builtin(__builtin_sub_overflow) #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1 #endif
+#define __diag_str1(s) #s +#define __diag_str(s) __diag_str1(s) +#define __diag(s) _Pragma(__diag_str(clang diagnostic s)) +#define __diag_CLANG_ignore ignored +#define __diag_CLANG_warn warning +#define __diag_CLANG_error error +#define __diag_CLANG(version, severity, s) \
- __diag_CLANG_ ## version(__diag_CLANG_ ## severity s)
+#if CLANG_VERSION >= 70000 +#define __diag_CLANG_7(s) __diag(s) +#else +#define __diag_CLANG_7(s) +#endif diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index a8ba6b04152c..a04e6bd63476 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -279,6 +279,10 @@ struct ftrace_likely_data { #define __diag_GCC(version, severity, string) #endif +#ifndef __diag_CLANG +#define __diag_CLANG(version, severity, string) +#endif
#define __diag_push() __diag(push) #define __diag_pop() __diag(pop) -- 2.18.0.345.g5c9ce644c3-goog
+ Miguel and Joe
This is the old patch I had sent for detecting Clang version. If we wanted to set a minimal version, this plus the actual version should do it. Probably could drop the diag stuff, as we changed clang to not warn in this case, so my solution using _Pragma is obsolete. On Mon, Jul 30, 2018 at 2:34 PM Nick Desaulniers ndesaulniers@google.com wrote:
These are needed for doing proper version checks, though feature detection via __has_attribute, __has_builtin, and __has_feature should be preferred, see: https://clang.llvm.org/docs/LanguageExtensions.html#feature-checking-macros
Also adds __diag support, for generating compiler version specific _Pragma()'s.
__diag support based on commit 8793bb7f4a9d ("kbuild: add macro for controlling warnings to linux/compiler.h")
Cc: stable@vger.kernel.org # 4.17, 4.14, 4.9, 4.4 Suggested-by: Nathan Chancellor natechancellor@gmail.com Signed-off-by: Nick Desaulniers ndesaulniers@google.com
include/linux/compiler-clang.h | 19 +++++++++++++++++++ include/linux/compiler_types.h | 4 ++++ 2 files changed, 23 insertions(+)
diff --git a/include/linux/compiler-clang.h b/include/linux/compiler-clang.h index 7087446c24c8..9442e07a361e 100644 --- a/include/linux/compiler-clang.h +++ b/include/linux/compiler-clang.h @@ -7,6 +7,10 @@
- for Clang compiler
*/
+#define CLANG_VERSION (__clang_major__ * 10000 \
+ __clang_minor__ * 100 \+ __clang_patchlevel__)#ifdef uninitialized_var #undef uninitialized_var #define uninitialized_var(x) x = *(&(x)) @@ -46,3 +50,18 @@ __has_builtin(__builtin_sub_overflow) #define COMPILER_HAS_GENERIC_BUILTIN_OVERFLOW 1 #endif
+#define __diag_str1(s) #s +#define __diag_str(s) __diag_str1(s) +#define __diag(s) _Pragma(__diag_str(clang diagnostic s)) +#define __diag_CLANG_ignore ignored +#define __diag_CLANG_warn warning +#define __diag_CLANG_error error +#define __diag_CLANG(version, severity, s) \
__diag_CLANG_ ## version(__diag_CLANG_ ## severity s)+#if CLANG_VERSION >= 70000 +#define __diag_CLANG_7(s) __diag(s) +#else +#define __diag_CLANG_7(s) +#endif diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index a8ba6b04152c..a04e6bd63476 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -279,6 +279,10 @@ struct ftrace_likely_data { #define __diag_GCC(version, severity, string) #endif
+#ifndef __diag_CLANG +#define __diag_CLANG(version, severity, string) +#endif
#define __diag_push() __diag(push) #define __diag_pop() __diag(pop)
-- 2.18.0.345.g5c9ce644c3-goog
Hi Nick,
On Fri, Aug 31, 2018 at 11:50 PM, Nick Desaulniers ndesaulniers@google.com wrote:
- Miguel and Joe
This is the old patch I had sent for detecting Clang version. If we wanted to set a minimal version, this plus the actual version should do it. Probably could drop the diag stuff, as we changed clang to not warn in this case, so my solution using _Pragma is obsolete.
Nitpick: do we want to use the CLANG_VERSION notation? In some cases I think it is easier to simply use directly the original values (like I did in compiler_attributes.h for gcc).
Consider that we almost never need the patchlevel (and if we do, it is better to *see* we are explicitly testing for it), and given the versioning schemes that compilers nowadays use (i.e. only major versions), I would bet not even the minor (except maybe for GCC in compiler_attributes.h until we drop gcc < 5).
Cheers, Miguel
Starting with Clang-7.0, _THIS_IP_ generates -Wreturn-stack-address warnings for almost every translation unit. In general, I'd prefer to leave this on (returning the address of a stack allocated variable is in general a bad idea) and disable it only at whitelisted call sites.
We can't do something like: #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wreturn-stack-address" <code> #pragma clang diagnostic pop
in a GNU Statement Expression or macro, hence we use _Pragma, which is its raison d'être: https://gcc.gnu.org/onlinedocs/cpp/Pragmas.html
We also can't use compiler specific pragma's without triggering -Werror=unknown-pragmas in other compilers, so use __diag.
Cc: stable@vger.kernel.org # 4.17, 4.14, 4.9, 4.4 Suggested-by: Nathan Chancellor natechancellor@gmail.com Signed-off-by: Nick Desaulniers ndesaulniers@google.com --- include/linux/kernel.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 941dc0a5a877..909bb771c0ca 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -168,7 +168,15 @@
#define _RET_IP_ (unsigned long)__builtin_return_address(0) -#define _THIS_IP_ ({ __label__ __here; __here: (unsigned long)&&__here; }) +#define _THIS_IP_ ( \ +{ \ + __label__ __here; \ + __diag_push() \ + __diag_ignore(CLANG, 7, "-Wreturn-stack-address", "") \ +__here: (unsigned long)&&__here; \ + __diag_pop() \ +} \ +)
#ifdef CONFIG_LBDAF # include <asm/div64.h>
On Mon, Jul 30, 2018 at 02:34:12PM -0700, Nick Desaulniers wrote:
Starting with Clang-7.0, _THIS_IP_ generates -Wreturn-stack-address warnings for almost every translation unit. In general, I'd prefer to leave this on (returning the address of a stack allocated variable is in general a bad idea) and disable it only at whitelisted call sites.
We can't do something like: #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wreturn-stack-address"
<code> #pragma clang diagnostic pop
in a GNU Statement Expression or macro, hence we use _Pragma, which is its raison d'être: https://gcc.gnu.org/onlinedocs/cpp/Pragmas.html
We also can't use compiler specific pragma's without triggering -Werror=unknown-pragmas in other compilers, so use __diag.
Cc: stable@vger.kernel.org # 4.17, 4.14, 4.9, 4.4 Suggested-by: Nathan Chancellor natechancellor@gmail.com Signed-off-by: Nick Desaulniers ndesaulniers@google.com
Tested-and-reviewed-by: Nathan Chancellor natechancellor@gmail.com
include/linux/kernel.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/include/linux/kernel.h b/include/linux/kernel.h index 941dc0a5a877..909bb771c0ca 100644 --- a/include/linux/kernel.h +++ b/include/linux/kernel.h @@ -168,7 +168,15 @@ #define _RET_IP_ (unsigned long)__builtin_return_address(0) -#define _THIS_IP_ ({ __label__ __here; __here: (unsigned long)&&__here; }) +#define _THIS_IP_ ( \ +{ \
- __label__ __here; \
- __diag_push() \
- __diag_ignore(CLANG, 7, "-Wreturn-stack-address", "") \
+__here: (unsigned long)&&__here; \
- __diag_pop() \
+} \ +) #ifdef CONFIG_LBDAF
# include <asm/div64.h>
2.18.0.345.g5c9ce644c3-goog
Hi Nick,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master] [also build test WARNING on v4.18-rc7 next-20180727] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Nick-Desaulniers/compiler-clang-h-A... config: x86_64-fedora-25 (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64
All warnings (new ones prefixed by >>):
drivers/net//wireless/intel/iwlwifi/iwl-trans.c: In function 'iwl_trans_send_cmd':
drivers/net//wireless/intel/iwlwifi/iwl-trans.c:137:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if (!(cmd->flags & CMD_ASYNC)) ^~ drivers/net//wireless/intel/iwlwifi/iwl-trans.c:138:1: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if' lock_map_acquire_read(&trans->sync_cmd_lockdep_map); ^ ~
vim +/if +137 drivers/net//wireless/intel/iwlwifi/iwl-trans.c
92fe8343 Emmanuel Grumbach 2015-12-01 116 92fe8343 Emmanuel Grumbach 2015-12-01 117 int iwl_trans_send_cmd(struct iwl_trans *trans, struct iwl_host_cmd *cmd) 92fe8343 Emmanuel Grumbach 2015-12-01 118 { 92fe8343 Emmanuel Grumbach 2015-12-01 119 int ret; 92fe8343 Emmanuel Grumbach 2015-12-01 120 92fe8343 Emmanuel Grumbach 2015-12-01 121 if (unlikely(!(cmd->flags & CMD_SEND_IN_RFKILL) && 326477e4 Johannes Berg 2017-04-25 122 test_bit(STATUS_RFKILL_OPMODE, &trans->status))) 92fe8343 Emmanuel Grumbach 2015-12-01 123 return -ERFKILL; 92fe8343 Emmanuel Grumbach 2015-12-01 124 92fe8343 Emmanuel Grumbach 2015-12-01 125 if (unlikely(test_bit(STATUS_FW_ERROR, &trans->status))) 92fe8343 Emmanuel Grumbach 2015-12-01 126 return -EIO; 92fe8343 Emmanuel Grumbach 2015-12-01 127 92fe8343 Emmanuel Grumbach 2015-12-01 128 if (unlikely(trans->state != IWL_TRANS_FW_ALIVE)) { 92fe8343 Emmanuel Grumbach 2015-12-01 129 IWL_ERR(trans, "%s bad state = %d\n", __func__, trans->state); 92fe8343 Emmanuel Grumbach 2015-12-01 130 return -EIO; 92fe8343 Emmanuel Grumbach 2015-12-01 131 } 92fe8343 Emmanuel Grumbach 2015-12-01 132 92fe8343 Emmanuel Grumbach 2015-12-01 133 if (WARN_ON((cmd->flags & CMD_WANT_ASYNC_CALLBACK) && 92fe8343 Emmanuel Grumbach 2015-12-01 134 !(cmd->flags & CMD_ASYNC))) 92fe8343 Emmanuel Grumbach 2015-12-01 135 return -EINVAL; 92fe8343 Emmanuel Grumbach 2015-12-01 136 92fe8343 Emmanuel Grumbach 2015-12-01 @137 if (!(cmd->flags & CMD_ASYNC)) 92fe8343 Emmanuel Grumbach 2015-12-01 138 lock_map_acquire_read(&trans->sync_cmd_lockdep_map); 92fe8343 Emmanuel Grumbach 2015-12-01 139 5b88792c Sara Sharon 2016-08-15 140 if (trans->wide_cmd_header && !iwl_cmd_groupid(cmd->id)) 5b88792c Sara Sharon 2016-08-15 141 cmd->id = DEF_ID(cmd->id); 5b88792c Sara Sharon 2016-08-15 142 92fe8343 Emmanuel Grumbach 2015-12-01 143 ret = trans->ops->send_cmd(trans, cmd); 92fe8343 Emmanuel Grumbach 2015-12-01 144 92fe8343 Emmanuel Grumbach 2015-12-01 145 if (!(cmd->flags & CMD_ASYNC)) 92fe8343 Emmanuel Grumbach 2015-12-01 146 lock_map_release(&trans->sync_cmd_lockdep_map); 92fe8343 Emmanuel Grumbach 2015-12-01 147 0ec971fd Johannes Berg 2017-04-10 148 if (WARN_ON((cmd->flags & CMD_WANT_SKB) && !ret && !cmd->resp_pkt)) 0ec971fd Johannes Berg 2017-04-10 149 return -EIO; 0ec971fd Johannes Berg 2017-04-10 150 92fe8343 Emmanuel Grumbach 2015-12-01 151 return ret; 92fe8343 Emmanuel Grumbach 2015-12-01 152 } 92fe8343 Emmanuel Grumbach 2015-12-01 153 IWL_EXPORT_SYMBOL(iwl_trans_send_cmd); 39bdb17e Sharon Dvir 2015-10-15 154
:::::: The code at line 137 was first introduced by commit :::::: 92fe83430b899b786c837e5b716a328220d47ae5 iwlwifi: uninline iwl_trans_send_cmd
:::::: TO: Emmanuel Grumbach emmanuel.grumbach@intel.com :::::: CC: Emmanuel Grumbach emmanuel.grumbach@intel.com
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Does anyone understand this error? The code looks just fine to me, and the source file doesn't conflict with any of the macros I've added (certainly not in any way that could cause an indentation error as reported here). On Tue, Jul 31, 2018 at 3:27 AM kbuild test robot lkp@intel.com wrote:
Hi Nick,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master] [also build test WARNING on v4.18-rc7 next-20180727] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Nick-Desaulniers/compiler-clang-h-A... config: x86_64-fedora-25 (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64
All warnings (new ones prefixed by >>):
drivers/net//wireless/intel/iwlwifi/iwl-trans.c: In function 'iwl_trans_send_cmd':
drivers/net//wireless/intel/iwlwifi/iwl-trans.c:137:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if (!(cmd->flags & CMD_ASYNC)) ^~drivers/net//wireless/intel/iwlwifi/iwl-trans.c:138:1: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if' lock_map_acquire_read(&trans->sync_cmd_lockdep_map); ^ ~
vim +/if +137 drivers/net//wireless/intel/iwlwifi/iwl-trans.c
92fe8343 Emmanuel Grumbach 2015-12-01 116 92fe8343 Emmanuel Grumbach 2015-12-01 117 int iwl_trans_send_cmd(struct iwl_trans *trans, struct iwl_host_cmd *cmd) 92fe8343 Emmanuel Grumbach 2015-12-01 118 { 92fe8343 Emmanuel Grumbach 2015-12-01 119 int ret; 92fe8343 Emmanuel Grumbach 2015-12-01 120 92fe8343 Emmanuel Grumbach 2015-12-01 121 if (unlikely(!(cmd->flags & CMD_SEND_IN_RFKILL) && 326477e4 Johannes Berg 2017-04-25 122 test_bit(STATUS_RFKILL_OPMODE, &trans->status))) 92fe8343 Emmanuel Grumbach 2015-12-01 123 return -ERFKILL; 92fe8343 Emmanuel Grumbach 2015-12-01 124 92fe8343 Emmanuel Grumbach 2015-12-01 125 if (unlikely(test_bit(STATUS_FW_ERROR, &trans->status))) 92fe8343 Emmanuel Grumbach 2015-12-01 126 return -EIO; 92fe8343 Emmanuel Grumbach 2015-12-01 127 92fe8343 Emmanuel Grumbach 2015-12-01 128 if (unlikely(trans->state != IWL_TRANS_FW_ALIVE)) { 92fe8343 Emmanuel Grumbach 2015-12-01 129 IWL_ERR(trans, "%s bad state = %d\n", __func__, trans->state); 92fe8343 Emmanuel Grumbach 2015-12-01 130 return -EIO; 92fe8343 Emmanuel Grumbach 2015-12-01 131 } 92fe8343 Emmanuel Grumbach 2015-12-01 132 92fe8343 Emmanuel Grumbach 2015-12-01 133 if (WARN_ON((cmd->flags & CMD_WANT_ASYNC_CALLBACK) && 92fe8343 Emmanuel Grumbach 2015-12-01 134 !(cmd->flags & CMD_ASYNC))) 92fe8343 Emmanuel Grumbach 2015-12-01 135 return -EINVAL; 92fe8343 Emmanuel Grumbach 2015-12-01 136 92fe8343 Emmanuel Grumbach 2015-12-01 @137 if (!(cmd->flags & CMD_ASYNC)) 92fe8343 Emmanuel Grumbach 2015-12-01 138 lock_map_acquire_read(&trans->sync_cmd_lockdep_map); 92fe8343 Emmanuel Grumbach 2015-12-01 139 5b88792c Sara Sharon 2016-08-15 140 if (trans->wide_cmd_header && !iwl_cmd_groupid(cmd->id)) 5b88792c Sara Sharon 2016-08-15 141 cmd->id = DEF_ID(cmd->id); 5b88792c Sara Sharon 2016-08-15 142 92fe8343 Emmanuel Grumbach 2015-12-01 143 ret = trans->ops->send_cmd(trans, cmd); 92fe8343 Emmanuel Grumbach 2015-12-01 144 92fe8343 Emmanuel Grumbach 2015-12-01 145 if (!(cmd->flags & CMD_ASYNC)) 92fe8343 Emmanuel Grumbach 2015-12-01 146 lock_map_release(&trans->sync_cmd_lockdep_map); 92fe8343 Emmanuel Grumbach 2015-12-01 147 0ec971fd Johannes Berg 2017-04-10 148 if (WARN_ON((cmd->flags & CMD_WANT_SKB) && !ret && !cmd->resp_pkt)) 0ec971fd Johannes Berg 2017-04-10 149 return -EIO; 0ec971fd Johannes Berg 2017-04-10 150 92fe8343 Emmanuel Grumbach 2015-12-01 151 return ret; 92fe8343 Emmanuel Grumbach 2015-12-01 152 } 92fe8343 Emmanuel Grumbach 2015-12-01 153 IWL_EXPORT_SYMBOL(iwl_trans_send_cmd); 39bdb17e Sharon Dvir 2015-10-15 154
:::::: The code at line 137 was first introduced by commit :::::: 92fe83430b899b786c837e5b716a328220d47ae5 iwlwifi: uninline iwl_trans_send_cmd
:::::: TO: Emmanuel Grumbach emmanuel.grumbach@intel.com :::::: CC: Emmanuel Grumbach emmanuel.grumbach@intel.com
0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
On Tue, Jul 31, 2018 at 9:48 AM, Nick Desaulniers ndesaulniers@google.com wrote:
Does anyone understand this error? The code looks just fine to me, and the source file doesn't conflict with any of the macros I've added (certainly not in any way that could cause an indentation error as reported here).
It does _use_ the macro, but yeah, I'm stumped...
On Tue, Jul 31, 2018 at 3:27 AM kbuild test robot lkp@intel.com wrote:
drivers/net//wireless/intel/iwlwifi/iwl-trans.c: In function 'iwl_trans_send_cmd':
drivers/net//wireless/intel/iwlwifi/iwl-trans.c:137:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if (!(cmd->flags & CMD_ASYNC)) ^~drivers/net//wireless/intel/iwlwifi/iwl-trans.c:138:1: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if' lock_map_acquire_read(&trans->sync_cmd_lockdep_map); ^ ~
vim +/if +137 drivers/net//wireless/intel/iwlwifi/iwl-trans.c
92fe8343 Emmanuel Grumbach 2015-12-01 @137 if (!(cmd->flags & CMD_ASYNC)) 92fe8343 Emmanuel Grumbach 2015-12-01 138 lock_map_acquire_read(&trans->sync_cmd_lockdep_map);
#define lock_map_acquire_read(l) lock_acquire_shared_recursive(l, 0, 0, NULL, _THIS_IP_)
#define lock_acquire_shared_recursive(l, s, t, n, i) lock_acquire(l, s, t, 2, 1, n, i)
The config doesn't have CONFIG_LOCKDEP, so it's not:
extern void lock_acquire(struct lockdep_map *lock, unsigned int subclass, int trylock, int read, int check, struct lockdep_map *nest_lock, unsigned long ip);
but rather:
# define lock_acquire(l, s, t, r, c, n, i) do { } while (0)
If you build with the same gcc and config, does it reproduce for you?
-Kees
On Tue, Jul 31, 2018 at 10:02 AM Kees Cook keescook@chromium.org wrote:
On Tue, Jul 31, 2018 at 9:48 AM, Nick Desaulniers ndesaulniers@google.com wrote:
Does anyone understand this error? The code looks just fine to me, and the source file doesn't conflict with any of the macros I've added (certainly not in any way that could cause an indentation error as reported here).
It does _use_ the macro, but yeah, I'm stumped...
On Tue, Jul 31, 2018 at 3:27 AM kbuild test robot lkp@intel.com wrote:
drivers/net//wireless/intel/iwlwifi/iwl-trans.c: In function 'iwl_trans_send_cmd':
drivers/net//wireless/intel/iwlwifi/iwl-trans.c:137:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if (!(cmd->flags & CMD_ASYNC)) ^~drivers/net//wireless/intel/iwlwifi/iwl-trans.c:138:1: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if' lock_map_acquire_read(&trans->sync_cmd_lockdep_map); ^ ~
vim +/if +137 drivers/net//wireless/intel/iwlwifi/iwl-trans.c
92fe8343 Emmanuel Grumbach 2015-12-01 @137 if (!(cmd->flags & CMD_ASYNC)) 92fe8343 Emmanuel Grumbach 2015-12-01 138 lock_map_acquire_read(&trans->sync_cmd_lockdep_map);
#define lock_map_acquire_read(l) lock_acquire_shared_recursive(l, 0, 0, NULL, _THIS_IP_)
#define lock_acquire_shared_recursive(l, s, t, n, i) lock_acquire(l, s, t, 2, 1, n, i)
The config doesn't have CONFIG_LOCKDEP, so it's not:
extern void lock_acquire(struct lockdep_map *lock, unsigned int subclass, int trylock, int read, int check, struct lockdep_map *nest_lock, unsigned long ip);
but rather:
# define lock_acquire(l, s, t, r, c, n, i) do { } while (0)
If you build with the same gcc and config, does it reproduce for you?
Indeed, gcc 6, 7, or 8 + the provided config reproduce the issue.
On Tue, Jul 31, 2018 at 10:02 AM Kees Cook keescook@chromium.org wrote:
On Tue, Jul 31, 2018 at 9:48 AM, Nick Desaulniers
On Tue, Jul 31, 2018 at 3:27 AM kbuild test robot lkp@intel.com wrote:
drivers/net//wireless/intel/iwlwifi/iwl-trans.c: In function 'iwl_trans_send_cmd':
drivers/net//wireless/intel/iwlwifi/iwl-trans.c:137:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if (!(cmd->flags & CMD_ASYNC)) ^~drivers/net//wireless/intel/iwlwifi/iwl-trans.c:138:1: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if' lock_map_acquire_read(&trans->sync_cmd_lockdep_map); ^ ~
vim +/if +137 drivers/net//wireless/intel/iwlwifi/iwl-trans.c
92fe8343 Emmanuel Grumbach 2015-12-01 @137 if (!(cmd->flags & CMD_ASYNC)) 92fe8343 Emmanuel Grumbach 2015-12-01 138 lock_map_acquire_read(&trans->sync_cmd_lockdep_map);
#define lock_map_acquire_read(l) lock_acquire_shared_recursive(l, 0, 0, NULL, _THIS_IP_)
#define lock_acquire_shared_recursive(l, s, t, n, i) lock_acquire(l, s, t, 2, 1, n, i)
The config doesn't have CONFIG_LOCKDEP, so it's not:
extern void lock_acquire(struct lockdep_map *lock, unsigned int subclass, int trylock, int read, int check, struct lockdep_map *nest_lock, unsigned long ip);
but rather:
# define lock_acquire(l, s, t, r, c, n, i) do { } while (0)
This is tricky, if I preprocess that translation unit with the exact flags used during compilation, I get:
``` if (!(cmd->flags & CMD_ASYNC))
#pragma GCC diagnostic push
#pragma GCC diagnostic pop do { } while (0); ```
Which is not enough to trigger -Wmisleading-indentation alone. It is curious that if we add braces to that if statement (as Nathan notes in a sibling post) or removing the pop (not shippable) seems to fix the warning.
On Tue, Jul 31, 2018 at 11:58 AM Nick Desaulniers ndesaulniers@google.com wrote:
On Tue, Jul 31, 2018 at 10:02 AM Kees Cook keescook@chromium.org wrote:
On Tue, Jul 31, 2018 at 9:48 AM, Nick Desaulniers
On Tue, Jul 31, 2018 at 3:27 AM kbuild test robot lkp@intel.com wrote:
drivers/net//wireless/intel/iwlwifi/iwl-trans.c: In function 'iwl_trans_send_cmd':
drivers/net//wireless/intel/iwlwifi/iwl-trans.c:137:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if (!(cmd->flags & CMD_ASYNC)) ^~drivers/net//wireless/intel/iwlwifi/iwl-trans.c:138:1: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if' lock_map_acquire_read(&trans->sync_cmd_lockdep_map); ^ ~
vim +/if +137 drivers/net//wireless/intel/iwlwifi/iwl-trans.c
92fe8343 Emmanuel Grumbach 2015-12-01 @137 if (!(cmd->flags & CMD_ASYNC)) 92fe8343 Emmanuel Grumbach 2015-12-01 138 lock_map_acquire_read(&trans->sync_cmd_lockdep_map);
#define lock_map_acquire_read(l) lock_acquire_shared_recursive(l, 0, 0, NULL, _THIS_IP_)
#define lock_acquire_shared_recursive(l, s, t, n, i) lock_acquire(l, s, t, 2, 1, n, i)
The config doesn't have CONFIG_LOCKDEP, so it's not:
extern void lock_acquire(struct lockdep_map *lock, unsigned int subclass, int trylock, int read, int check, struct lockdep_map *nest_lock, unsigned long ip);
but rather:
# define lock_acquire(l, s, t, r, c, n, i) do { } while (0)
This is tricky, if I preprocess that translation unit with the exact flags used during compilation, I get:
if (!(cmd->flags & CMD_ASYNC)) #pragma GCC diagnostic push #pragma GCC diagnostic pop do { } while (0);Which is not enough to trigger -Wmisleading-indentation alone. It is curious that if we add braces to that if statement (as Nathan notes in a sibling post) or removing the pop (not shippable) seems to fix the warning.
Something fishy is going on here: https://godbolt.org/g/b5dsqH
It seems that gcc's warning is technically correct, but it seems to be a miscompile as puts() in my reduced test case is called unconditionally. I've filed: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=86765
In the meanwhile, I've reworked the patch to change _THIS_IP_ to a only contain a function call, to a new static inline function which does what the statement expression used to. This now triggers -Wreturn-local-addr warnings in gcc, which is a warning added in gcc-4.8, so I need to add another __diag_ignore, and case for gcc 4.8 to include/linux/compiler-gcc.h.
At this point, I think I might as well consolidate current_text_addr() and _THIS_IP_. Stay tuned for v3.
-- Thanks, ~Nick Desaulniers
On Tue, Jul 31, 2018 at 09:48:57AM -0700, Nick Desaulniers wrote:
Does anyone understand this error? The code looks just fine to me, and the source file doesn't conflict with any of the macros I've added (certainly not in any way that could cause an indentation error as reported here).
Unfortunately, I've been trying to work through it this morning and I can't understand it either. lock_map_acquire_read's defintion does include _THIS_IP_ and adding curly braces to the if statement fixes it but that doesn't explain why it's happening.
Maybe this is a GCC bug/issue?
Cheers, Nathan
On Tue, Jul 31, 2018 at 3:27 AM kbuild test robot lkp@intel.com wrote:
Hi Nick,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master] [also build test WARNING on v4.18-rc7 next-20180727] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Nick-Desaulniers/compiler-clang-h-A... config: x86_64-fedora-25 (attached as .config) compiler: gcc-7 (Debian 7.3.0-16) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=x86_64
All warnings (new ones prefixed by >>):
drivers/net//wireless/intel/iwlwifi/iwl-trans.c: In function 'iwl_trans_send_cmd':
drivers/net//wireless/intel/iwlwifi/iwl-trans.c:137:2: warning: this 'if' clause does not guard... [-Wmisleading-indentation]
if (!(cmd->flags & CMD_ASYNC)) ^~drivers/net//wireless/intel/iwlwifi/iwl-trans.c:138:1: note: ...this statement, but the latter is misleadingly indented as if it were guarded by the 'if' lock_map_acquire_read(&trans->sync_cmd_lockdep_map); ^ ~
vim +/if +137 drivers/net//wireless/intel/iwlwifi/iwl-trans.c
92fe8343 Emmanuel Grumbach 2015-12-01 116 92fe8343 Emmanuel Grumbach 2015-12-01 117 int iwl_trans_send_cmd(struct iwl_trans *trans, struct iwl_host_cmd *cmd) 92fe8343 Emmanuel Grumbach 2015-12-01 118 { 92fe8343 Emmanuel Grumbach 2015-12-01 119 int ret; 92fe8343 Emmanuel Grumbach 2015-12-01 120 92fe8343 Emmanuel Grumbach 2015-12-01 121 if (unlikely(!(cmd->flags & CMD_SEND_IN_RFKILL) && 326477e4 Johannes Berg 2017-04-25 122 test_bit(STATUS_RFKILL_OPMODE, &trans->status))) 92fe8343 Emmanuel Grumbach 2015-12-01 123 return -ERFKILL; 92fe8343 Emmanuel Grumbach 2015-12-01 124 92fe8343 Emmanuel Grumbach 2015-12-01 125 if (unlikely(test_bit(STATUS_FW_ERROR, &trans->status))) 92fe8343 Emmanuel Grumbach 2015-12-01 126 return -EIO; 92fe8343 Emmanuel Grumbach 2015-12-01 127 92fe8343 Emmanuel Grumbach 2015-12-01 128 if (unlikely(trans->state != IWL_TRANS_FW_ALIVE)) { 92fe8343 Emmanuel Grumbach 2015-12-01 129 IWL_ERR(trans, "%s bad state = %d\n", __func__, trans->state); 92fe8343 Emmanuel Grumbach 2015-12-01 130 return -EIO; 92fe8343 Emmanuel Grumbach 2015-12-01 131 } 92fe8343 Emmanuel Grumbach 2015-12-01 132 92fe8343 Emmanuel Grumbach 2015-12-01 133 if (WARN_ON((cmd->flags & CMD_WANT_ASYNC_CALLBACK) && 92fe8343 Emmanuel Grumbach 2015-12-01 134 !(cmd->flags & CMD_ASYNC))) 92fe8343 Emmanuel Grumbach 2015-12-01 135 return -EINVAL; 92fe8343 Emmanuel Grumbach 2015-12-01 136 92fe8343 Emmanuel Grumbach 2015-12-01 @137 if (!(cmd->flags & CMD_ASYNC)) 92fe8343 Emmanuel Grumbach 2015-12-01 138 lock_map_acquire_read(&trans->sync_cmd_lockdep_map); 92fe8343 Emmanuel Grumbach 2015-12-01 139 5b88792c Sara Sharon 2016-08-15 140 if (trans->wide_cmd_header && !iwl_cmd_groupid(cmd->id)) 5b88792c Sara Sharon 2016-08-15 141 cmd->id = DEF_ID(cmd->id); 5b88792c Sara Sharon 2016-08-15 142 92fe8343 Emmanuel Grumbach 2015-12-01 143 ret = trans->ops->send_cmd(trans, cmd); 92fe8343 Emmanuel Grumbach 2015-12-01 144 92fe8343 Emmanuel Grumbach 2015-12-01 145 if (!(cmd->flags & CMD_ASYNC)) 92fe8343 Emmanuel Grumbach 2015-12-01 146 lock_map_release(&trans->sync_cmd_lockdep_map); 92fe8343 Emmanuel Grumbach 2015-12-01 147 0ec971fd Johannes Berg 2017-04-10 148 if (WARN_ON((cmd->flags & CMD_WANT_SKB) && !ret && !cmd->resp_pkt)) 0ec971fd Johannes Berg 2017-04-10 149 return -EIO; 0ec971fd Johannes Berg 2017-04-10 150 92fe8343 Emmanuel Grumbach 2015-12-01 151 return ret; 92fe8343 Emmanuel Grumbach 2015-12-01 152 } 92fe8343 Emmanuel Grumbach 2015-12-01 153 IWL_EXPORT_SYMBOL(iwl_trans_send_cmd); 39bdb17e Sharon Dvir 2015-10-15 154
:::::: The code at line 137 was first introduced by commit :::::: 92fe83430b899b786c837e5b716a328220d47ae5 iwlwifi: uninline iwl_trans_send_cmd
:::::: TO: Emmanuel Grumbach emmanuel.grumbach@intel.com :::::: CC: Emmanuel Grumbach emmanuel.grumbach@intel.com
0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
-- Thanks, ~Nick Desaulniers
Hi Nick,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master] [also build test ERROR on v4.18-rc7 next-20180727] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Nick-Desaulniers/compiler-clang-h-A... config: x86_64-randconfig-s1-07312048 (attached as .config) compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026 reproduce: # save the attached .config to linux build tree make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/gpu//drm/i915/i915_gem.c: In function '__i915_gem_object_set_pages':
drivers/gpu//drm/i915/i915_gem.c:2697:1: error: expected expression before '#pragma'
GEM_BUG_ON(!HAS_PAGE_SIZES(i915, obj->mm.page_sizes.sg)); ^~~
vim +2697 drivers/gpu//drm/i915/i915_gem.c
03ac84f18 Chris Wilson 2016-10-28 2658 03ac84f18 Chris Wilson 2016-10-28 2659 void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj, a5c081662 Matthew Auld 2017-10-06 2660 struct sg_table *pages, 84e8978e6 Matthew Auld 2017-10-09 2661 unsigned int sg_page_sizes) 03ac84f18 Chris Wilson 2016-10-28 2662 { a5c081662 Matthew Auld 2017-10-06 2663 struct drm_i915_private *i915 = to_i915(obj->base.dev); a5c081662 Matthew Auld 2017-10-06 2664 unsigned long supported = INTEL_INFO(i915)->page_sizes; a5c081662 Matthew Auld 2017-10-06 2665 int i; a5c081662 Matthew Auld 2017-10-06 2666 1233e2db1 Chris Wilson 2016-10-28 2667 lockdep_assert_held(&obj->mm.lock); 03ac84f18 Chris Wilson 2016-10-28 2668 03ac84f18 Chris Wilson 2016-10-28 2669 obj->mm.get_page.sg_pos = pages->sgl; 03ac84f18 Chris Wilson 2016-10-28 2670 obj->mm.get_page.sg_idx = 0; 03ac84f18 Chris Wilson 2016-10-28 2671 03ac84f18 Chris Wilson 2016-10-28 2672 obj->mm.pages = pages; 2c3a3f44d Chris Wilson 2016-11-04 2673 2c3a3f44d Chris Wilson 2016-11-04 2674 if (i915_gem_object_is_tiled(obj) && f2123818f Chris Wilson 2017-10-16 2675 i915->quirks & QUIRK_PIN_SWIZZLED_PAGES) { 2c3a3f44d Chris Wilson 2016-11-04 2676 GEM_BUG_ON(obj->mm.quirked); 2c3a3f44d Chris Wilson 2016-11-04 2677 __i915_gem_object_pin_pages(obj); 2c3a3f44d Chris Wilson 2016-11-04 2678 obj->mm.quirked = true; 2c3a3f44d Chris Wilson 2016-11-04 2679 } a5c081662 Matthew Auld 2017-10-06 2680 84e8978e6 Matthew Auld 2017-10-09 2681 GEM_BUG_ON(!sg_page_sizes); 84e8978e6 Matthew Auld 2017-10-09 2682 obj->mm.page_sizes.phys = sg_page_sizes; a5c081662 Matthew Auld 2017-10-06 2683 a5c081662 Matthew Auld 2017-10-06 2684 /* 84e8978e6 Matthew Auld 2017-10-09 2685 * Calculate the supported page-sizes which fit into the given 84e8978e6 Matthew Auld 2017-10-09 2686 * sg_page_sizes. This will give us the page-sizes which we may be able 84e8978e6 Matthew Auld 2017-10-09 2687 * to use opportunistically when later inserting into the GTT. For 84e8978e6 Matthew Auld 2017-10-09 2688 * example if phys=2G, then in theory we should be able to use 1G, 2M, 84e8978e6 Matthew Auld 2017-10-09 2689 * 64K or 4K pages, although in practice this will depend on a number of 84e8978e6 Matthew Auld 2017-10-09 2690 * other factors. a5c081662 Matthew Auld 2017-10-06 2691 */ a5c081662 Matthew Auld 2017-10-06 2692 obj->mm.page_sizes.sg = 0; a5c081662 Matthew Auld 2017-10-06 2693 for_each_set_bit(i, &supported, ilog2(I915_GTT_MAX_PAGE_SIZE) + 1) { a5c081662 Matthew Auld 2017-10-06 2694 if (obj->mm.page_sizes.phys & ~0u << i) a5c081662 Matthew Auld 2017-10-06 2695 obj->mm.page_sizes.sg |= BIT(i); a5c081662 Matthew Auld 2017-10-06 2696 } a5c081662 Matthew Auld 2017-10-06 @2697 GEM_BUG_ON(!HAS_PAGE_SIZES(i915, obj->mm.page_sizes.sg)); f2123818f Chris Wilson 2017-10-16 2698 f2123818f Chris Wilson 2017-10-16 2699 spin_lock(&i915->mm.obj_lock); f2123818f Chris Wilson 2017-10-16 2700 list_add(&obj->mm.link, &i915->mm.unbound_list); f2123818f Chris Wilson 2017-10-16 2701 spin_unlock(&i915->mm.obj_lock); 03ac84f18 Chris Wilson 2016-10-28 2702 } 03ac84f18 Chris Wilson 2016-10-28 2703
:::::: The code at line 2697 was first introduced by commit :::::: a5c08166265adc172a4cbde8ed26a1a96ce77fb7 drm/i915: introduce page_size members
:::::: TO: Matthew Auld matthew.auld@intel.com :::::: CC: Chris Wilson chris@chris-wilson.co.uk
--- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Another strange error from 0-day bot. The source file does not make use of my added macros, so not sure how they could generate such a warning. On Tue, Jul 31, 2018 at 6:54 AM kbuild test robot lkp@intel.com wrote:
Hi Nick,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on linus/master] [also build test ERROR on v4.18-rc7 next-20180727] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Nick-Desaulniers/compiler-clang-h-A... config: x86_64-randconfig-s1-07312048 (attached as .config) compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026 reproduce: # save the attached .config to linux build tree make ARCH=x86_64
All errors (new ones prefixed by >>):
drivers/gpu//drm/i915/i915_gem.c: In function '__i915_gem_object_set_pages':
drivers/gpu//drm/i915/i915_gem.c:2697:1: error: expected expression before '#pragma'
GEM_BUG_ON(!HAS_PAGE_SIZES(i915, obj->mm.page_sizes.sg)); ^~~vim +2697 drivers/gpu//drm/i915/i915_gem.c
03ac84f18 Chris Wilson 2016-10-28 2658 03ac84f18 Chris Wilson 2016-10-28 2659 void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj, a5c081662 Matthew Auld 2017-10-06 2660 struct sg_table *pages, 84e8978e6 Matthew Auld 2017-10-09 2661 unsigned int sg_page_sizes) 03ac84f18 Chris Wilson 2016-10-28 2662 { a5c081662 Matthew Auld 2017-10-06 2663 struct drm_i915_private *i915 = to_i915(obj->base.dev); a5c081662 Matthew Auld 2017-10-06 2664 unsigned long supported = INTEL_INFO(i915)->page_sizes; a5c081662 Matthew Auld 2017-10-06 2665 int i; a5c081662 Matthew Auld 2017-10-06 2666 1233e2db1 Chris Wilson 2016-10-28 2667 lockdep_assert_held(&obj->mm.lock); 03ac84f18 Chris Wilson 2016-10-28 2668 03ac84f18 Chris Wilson 2016-10-28 2669 obj->mm.get_page.sg_pos = pages->sgl; 03ac84f18 Chris Wilson 2016-10-28 2670 obj->mm.get_page.sg_idx = 0; 03ac84f18 Chris Wilson 2016-10-28 2671 03ac84f18 Chris Wilson 2016-10-28 2672 obj->mm.pages = pages; 2c3a3f44d Chris Wilson 2016-11-04 2673 2c3a3f44d Chris Wilson 2016-11-04 2674 if (i915_gem_object_is_tiled(obj) && f2123818f Chris Wilson 2017-10-16 2675 i915->quirks & QUIRK_PIN_SWIZZLED_PAGES) { 2c3a3f44d Chris Wilson 2016-11-04 2676 GEM_BUG_ON(obj->mm.quirked); 2c3a3f44d Chris Wilson 2016-11-04 2677 __i915_gem_object_pin_pages(obj); 2c3a3f44d Chris Wilson 2016-11-04 2678 obj->mm.quirked = true; 2c3a3f44d Chris Wilson 2016-11-04 2679 } a5c081662 Matthew Auld 2017-10-06 2680 84e8978e6 Matthew Auld 2017-10-09 2681 GEM_BUG_ON(!sg_page_sizes); 84e8978e6 Matthew Auld 2017-10-09 2682 obj->mm.page_sizes.phys = sg_page_sizes; a5c081662 Matthew Auld 2017-10-06 2683 a5c081662 Matthew Auld 2017-10-06 2684 /* 84e8978e6 Matthew Auld 2017-10-09 2685 * Calculate the supported page-sizes which fit into the given 84e8978e6 Matthew Auld 2017-10-09 2686 * sg_page_sizes. This will give us the page-sizes which we may be able 84e8978e6 Matthew Auld 2017-10-09 2687 * to use opportunistically when later inserting into the GTT. For 84e8978e6 Matthew Auld 2017-10-09 2688 * example if phys=2G, then in theory we should be able to use 1G, 2M, 84e8978e6 Matthew Auld 2017-10-09 2689 * 64K or 4K pages, although in practice this will depend on a number of 84e8978e6 Matthew Auld 2017-10-09 2690 * other factors. a5c081662 Matthew Auld 2017-10-06 2691 */ a5c081662 Matthew Auld 2017-10-06 2692 obj->mm.page_sizes.sg = 0; a5c081662 Matthew Auld 2017-10-06 2693 for_each_set_bit(i, &supported, ilog2(I915_GTT_MAX_PAGE_SIZE) + 1) { a5c081662 Matthew Auld 2017-10-06 2694 if (obj->mm.page_sizes.phys & ~0u << i) a5c081662 Matthew Auld 2017-10-06 2695 obj->mm.page_sizes.sg |= BIT(i); a5c081662 Matthew Auld 2017-10-06 2696 } a5c081662 Matthew Auld 2017-10-06 @2697 GEM_BUG_ON(!HAS_PAGE_SIZES(i915, obj->mm.page_sizes.sg)); f2123818f Chris Wilson 2017-10-16 2698 f2123818f Chris Wilson 2017-10-16 2699 spin_lock(&i915->mm.obj_lock); f2123818f Chris Wilson 2017-10-16 2700 list_add(&obj->mm.link, &i915->mm.unbound_list); f2123818f Chris Wilson 2017-10-16 2701 spin_unlock(&i915->mm.obj_lock); 03ac84f18 Chris Wilson 2016-10-28 2702 } 03ac84f18 Chris Wilson 2016-10-28 2703
:::::: The code at line 2697 was first introduced by commit :::::: a5c08166265adc172a4cbde8ed26a1a96ce77fb7 drm/i915: introduce page_size members
:::::: TO: Matthew Auld matthew.auld@intel.com :::::: CC: Chris Wilson chris@chris-wilson.co.uk
0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
linux-stable-mirror@lists.linaro.org
-
kbuild test robot -
Kees Cook -
Miguel Ojeda -
Nathan Chancellor -
Nick Desaulniers