Appararently, it's possible to have a non-trivial TU include a few
headers, including linux/build_bug.h, without ending up with
linux/types.h. So the 0day bot sent me
config: um-x86_64_defconfig (attached as .config)
>> include/linux/compiler.h:316:3: error: unknown type name 'bool'; did you mean '_Bool'?
bool __cond = !(condition); \
for something I'm working on.
Rather than contributing to the #include madness and including
linux/types.h from compiler.h, just use int.
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Rasmus Villemoes <[email protected]>
Cc: Christopher Li <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
The Yocto build system does a 'make clean' when rebuilding due to
changed dependencies, and that consistently fails for me (causing the
whole BSP build to fail) with errors such as
| find: '[...]/perf/1.0-r9/perf-1.0/plugin_mac80211.so': No such file or directory
| find: '[...]/perf/1.0-r9/perf-1.0/plugin_mac80211.so': No such file or directory
| find: find: '[...]/perf/1.0-r9/perf-1.0/libtraceevent.a''[...]/perf/1.0-r9/perf-1.0/libtraceevent.a': No such file or directory: No such file or directory
|
[...]
| find: cannot delete '/mnt/xfs/devel/pil/yocto/tmp-glibc/work/wandboard-oe-linux-gnueabi/perf/1.0-r9/perf-1.0/util/.pstack.o.cmd': No such file or directory
Apparently (despite the comment), 'make clean' ends up launching
multiple sub-makes that all want to remove the same things - perhaps
this only happens in combination with a O=... parameter. In any case, we
don't lose much by explicitly disabling the parallelism for the clean
target, and it makes automated builds much more reliable.
Signed-off-by: Rasmus Villemoes <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
Obviously a+b==b+a and a*b==b*a, but the implementation of the fallback
checks are not entirely symmetric in how they treat a and b. So we might
as well check the (b,a,r,of) tuple as well as the (a,b,r,of) one for +
and *. Rather than more copy-paste, factor out the common part to
check_one_op.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
This adds a small module for testing that the check_*_overflow
functions work as expected, whether implemented in C or using gcc
builtins.
Example output:
test_overflow: u8 : 18 tests
test_overflow: s8 : 19 tests
test_overflow: u16: 17 tests
test_overflow: s16: 17 tests
test_overflow: u32: 17 tests
test_overflow: s32: 17 tests
test_overflow: u64: 17 tests
test_overflow: s64: 21 tests
Signed-off-by: Rasmus Villemoes <[email protected]>
[kees: add output to commit log, drop u64 tests on 32-bit]
Signed-off-by: Kees Cook <[email protected]>
This adds wrappers for the __builtin overflow checkers present in gcc
5.1+ as well as fallback implementations for earlier compilers. It's not
that easy to implement the fully generic __builtin_X_overflow(T1 a, T2
b, T3 *d) in macros, so the fallback code assumes that T1, T2 and T3 are
the same. We obviously don't want the wrappers to have different
semantics depending on $GCC_VERSION, so we also insist on that even when
using the builtins.
There are a few problems with the 'a+b < a' idiom for checking for
overflow: For signed types, it relies on undefined behaviour and is
not actually complete (it doesn't check underflow;
e.g. INT_MIN+INT_MIN == 0 isn't caught). Due to type promotion it
is wrong for all types (signed and unsigned) narrower than
int. Similarly, when a and b does not have the same type, there are
subtle cases like
u32 a;
if (a + sizeof(foo) < a)
return -EOVERFLOW;
a += sizeof(foo);
where the test is always false on 64 bit platforms. Add to that that it
is not always possible to determine the types involved at a glance.
The new overflow.h is somewhat bulky, but that's mostly a result of
trying to be type-generic, complete (e.g. catching not only overflow
but also signed underflow) and not relying on undefined behaviour.
Linus is of course right [1] that for unsigned subtraction a-b, the
right way to check for overflow (underflow) is "b > a" and not
"__builtin_sub_overflow(a, b, &d)", but that's just one out of six cases
covered here, and included mostly for completeness.
So is it worth it? I think it is, if nothing else for the documentation
value of seeing
if (check_add_overflow(a, b, &d))
return -EGOAWAY;
do_stuff_with(d);
instead of the open-coded (and possibly wrong and/or incomplete and/or
UBsan-tickling)
if (a+b < a)
return -EGOAWAY;
do_stuff_with(a+b);
While gcc does recognize the 'a+b < a' idiom for testing unsigned add
overflow, it doesn't do nearly as good for unsigned multiplication
(there's also no single well-established idiom). So using
check_mul_overflow in kcalloc and friends may also make gcc generate
slightly better code.
[1] https://lkml.org/lkml/2015/11/2/658
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
I managed to send some modpost patches to old addresses of both
Masahiro and Michal, and omitted linux-kbuild from cc, because my
tried and trusted scripts/get_maintainer wrapper failed me. Add the
modpost directory to the MAINTAINERS entry, and while at it make the
Makefile glob match scripts/Makefile itself, and add one matching the
Kbuild.include file as well.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
Commit 7840fea200 ("kbuild: Fix computing srcversion for modules")
fixed the comment above parse_source_files to refer to the new source_
line, but left this one behind that could still give the impression that
drivers/net/dummy.c appears in the deps_ variable.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
I tried building using a freshly built Make (4.2.1-69-g8a731d1), but
already the objtool build broke with
orc_dump.c: In function ‘orc_dump’:
orc_dump.c:106:2: error: ‘elf_getshnum’ is deprecated [-Werror=deprecated-declarations]
if (elf_getshdrnum(elf, &nr_sections)) {
Turns out that with that new Make, the backslash was not removed, so cpp
didn't see a #include directive, grep found nothing, and
-DLIBELF_USE_DEPRECATED was wrongly put in CFLAGS.
Now, that new Make behaviour is documented in their NEWS file:
* WARNING: Backward-incompatibility!
Number signs (#) appearing inside a macro reference or function invocation
no longer introduce comments and should not be escaped with backslashes:
thus a call such as:
foo := $(shell echo '#')
is legal. Previously the number sign needed to be escaped, for example:
foo := $(shell echo '\#')
Now this latter will resolve to "\#". If you want to write makefiles
portable to both versions, assign the number sign to a variable:
C := \#
foo := $(shell echo '$C')
This was claimed to be fixed in 3.81, but wasn't, for some reason.
To detect this change search for 'nocomment' in the .FEATURES variable.
This also fixes up the two make-cmd instances to replace # with $(pound)
rather than with \#. There might very well be other places that need
similar fixup in preparation for whatever future Make release contains
the above change, but at least this builds an x86_64 defconfig with the
new make.
Link: https://bugzilla.kernel.org/show_bug.cgi?id=197847
Cc: Randy Dunlap <[email protected]>
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
I noticed that offsetof(struct filename, iname) is actually 28 on 64
bit platforms, so we always pass an unaligned pointer to
strncpy_from_user. This is mostly a problem for those 64 bit platforms
without HAVE_EFFICIENT_UNALIGNED_ACCESS, but even on x86_64, unaligned
accesses carry a penalty.
A user-space microbenchmark doing nothing but strncpy_from_user from the
same (aligned) source string runs about 5% faster when the destination
is aligned. That number increases to 20% when the string is long
enough (~32 bytes) that we cross a cache line boundary - that's for
example the case for about half the files a "git status" in a kernel
tree ends up stat'ing.
This won't make any real-life workloads 5%, or even 1%, faster, but path
lookup is common enough that cutting even a few cycles should be
worthwhile. So ensure we always pass an aligned destination pointer to
strncpy_from_user. Instead of explicit padding, simply swap the refcnt
and aname members, as suggested by Al Viro.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Al Viro <[email protected]>
kconfig.h was excluded from consideration by fixdep by
6a5be57f0f (fixdep: fix extraneous dependencies) to avoid some false
positive hits
(1) include/config/.h
(2) include/config/h.h
(3) include/config/foo.h
(1) occurred because kconfig.h contains the string CONFIG_ in a
comment. However, since dee81e9886 (fixdep: faster CONFIG_ search), we
have a check that the part after CONFIG_ is non-empty, so this does not
happen anymore (and CONFIG_ appears by itself elsewhere, so that check
is worthwhile).
(2) comes from the include guard, __LINUX_KCONFIG_H. But with the
previous patch, we no longer match that either.
That leaves (3), which amounts to one [1] false dependency (aka stat() call
done by make), which I think we can live with:
We've already had one case [2] where the lack of include/linux/kconfig.h in
the .o.cmd file caused a missing rebuild, and while I originally thought
we should just put kconfig.h in the dependency list without parsing it
for the CONFIG_ pattern, we actually do have some real CONFIG_ symbols
mentioned in it, and one can imagine some translation unit that just
does '#ifdef __BIG_ENDIAN' but doesn't through some other header
actually depend on CONFIG_CPU_BIG_ENDIAN - so changing the target
endianness could end up rebuilding the world, minus that small
TU. Quoting Linus,
... when missing dependencies cause a missed re-compile, the resulting
bugs can be _really_ subtle.
[1] well, two, we now also have CONFIG_BOOGER/booger.h - we could change
that to FOO if we care
[2] https://lkml.org/lkml/2018/2/22/838
Cc: Linus Torvalds <[email protected]>
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
The string CONFIG_ quite often appears after other alphanumerics,
meaning that that instance cannot be referencing a Kconfig
symbol. Omitting these means make has fewer files to stat() when
deciding what needs to be rebuilt - for a defconfig build, this seems to
remove about 2% of the (wildcard ...) lines from the .o.cmd files.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
uml-config.h hasn't existed in this decade (87e299e5c7 - x86, um: get
rid of uml-config.h). The few remaining UML_CONFIG instances are defined
directly in terms of their real CONFIG symbol in common-offsets.h, so
unlike when the symbols got defined via a sed script, anything that uses
UML_CONFIG_FOO now should also automatically pick up a dependency on
CONFIG_FOO via the normal fixdep mechanism (since common-offsets.h
should at least recursively be a dependency). Hence I believe we should
actually be able to ignore the HELLO_CONFIG_BOOM cases.
Cc: Al Viro <[email protected]>
Cc: Richard Weinberger <[email protected]>
Cc: [email protected]
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Masahiro Yamada <[email protected]>
As far as I can tell, the only place the per-cpu ida_bitmap is populated
is in ida_pre_get. The pre-allocated element is stolen in two places in
ida_get_new_above, in both cases immediately followed by a memset(0).
Since ida_get_new_above is called with locks held, do the zeroing in
ida_pre_get, or rather let kmalloc() do it. Also, apparently gcc
generates ~44 bytes of code to do a memset(, 0, 128):
$ scripts/bloat-o-meter vmlinux.{0,1}
add/remove: 0/0 grow/shrink: 2/1 up/down: 5/-88 (-83)
Function old new delta
ida_pre_get 115 119 +4
vermagic 27 28 +1
ida_get_new_above 715 627 -88
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Rasmus Villemoes <[email protected]>
Acked-by: Matthew Wilcox <[email protected]>
Cc: Eric Biggers <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
The last expression in a statement expression need not be a bare
variable, quoting gcc docs
The last thing in the compound statement should be an expression
followed by a semicolon; the value of this subexpression serves as the
value of the entire construct.
and we already use that in e.g. the min/max macros which end with a
ternary expression.
This way, we can allow index to have const-qualified type, which will in
some cases avoid the need for introducing a local copy of index of
non-const qualified type. That, in turn, can prevent readers not
familiar with the internals of array_index_nospec from wondering about
the seemingly redundant extra variable, and I think that's worthwhile
considering how confusing the whole _nospec business is.
The expression _i&_mask has type unsigned long (since that is the type
of _mask, and the BUILD_BUG_ONs guarantee that _i will get promoted to
that), so in order not to change the type of the whole expression, add
a cast back to typeof(_i).
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Dan Williams <[email protected]>
Acked-by: Linus Torvalds <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Arjan van de Ven <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: Dave Hansen <[email protected]>
Cc: David Woodhouse <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Josh Poimboeuf <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/151881604837.17395.10812767547837568328.stgit@dwillia2-desk3.amr.corp.intel.com
Signed-off-by: Ingo Molnar <[email protected]>
This reverts commit 92266d6ef6 ("async: simplify lowest_in_progress()")
which was simply wrong: In the case where domain is NULL, we now use the
wrong offsetof() in the list_first_entry macro, so we don't actually
fetch the ->cookie value, but rather the eight bytes located
sizeof(struct list_head) further into the struct async_entry.
On 64 bit, that's the data member, while on 32 bit, that's a u64 built
from func and data in some order.
I think the bug happens to be harmless in practice: It obviously only
affects callers which pass a NULL domain, and AFAICT the only such
caller is
async_synchronize_full() ->
async_synchronize_full_domain(NULL) ->
async_synchronize_cookie_domain(ASYNC_COOKIE_MAX, NULL)
and the ASYNC_COOKIE_MAX means that in practice we end up waiting for
the async_global_pending list to be empty - but it would break if
somebody happened to pass (void*)-1 as the data element to
async_schedule, and of course also if somebody ever does a
async_synchronize_cookie_domain(, NULL) with a "finite" cookie value.
Maybe the "harmless in practice" means this isn't -stable material. But
I'm not completely confident my quick git grep'ing is enough, and there
might be affected code in one of the earlier kernels that has since been
removed, so I'll leave the decision to the stable guys.
Link: http://lkml.kernel.org/r/[email protected]
Fixes: 92266d6ef6 "async: simplify lowest_in_progress()"
Signed-off-by: Rasmus Villemoes <[email protected]>
Acked-by: Tejun Heo <[email protected]>
Cc: Arjan van de Ven <[email protected]>
Cc: Adam Wallis <[email protected]>
Cc: Lai Jiangshan <[email protected]>
Cc: <[email protected]> [3.10+]
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
This saves a little .text and gets rid of the unmotivated line break and
the sizeof(...) style inconsistency.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
While we do need macros to be able to return from the "calling"
function, we can still factor the checks done by the VALIDATE_DESC*
macros into a real helper function. This reduces the backslashtitis,
avoids duplicating the logic in the two macros and saves about 1K of
generated code:
$ scripts/bloat-o-meter drivers/gpio/gpiolib.o.{0,1}
add/remove: 1/0 grow/shrink: 0/15 up/down: 104/-1281 (-1177)
Function old new delta
validate_desc - 104 +104
gpiod_set_value 192 135 -57
gpiod_set_raw_value 125 67 -58
gpiod_direction_output 412 351 -61
gpiod_set_value_cansleep 150 70 -80
gpiod_set_raw_value_cansleep 132 52 -80
gpiod_get_raw_value 139 54 -85
gpiod_set_debounce 226 140 -86
gpiod_direction_output_raw 124 38 -86
gpiod_get_value 161 74 -87
gpiod_cansleep 126 39 -87
gpiod_get_raw_value_cansleep 130 39 -91
gpiod_get_value_cansleep 152 59 -93
gpiod_is_active_low 128 33 -95
gpiod_request 299 184 -115
gpiod_direction_input 386 266 -120
Total: Before=25460, After=24283, chg -4.62%
Signed-off-by: Rasmus Villemoes <[email protected]>
Reviewed-by: Vladimir Zapolskiy <[email protected]>
Signed-off-by: Linus Walleij <[email protected]>
gpio_free(gpio) simply does gpiod_free(gpio_to_desc(gpio)), so it's
simpler and cleaner to use gpiod_free directly.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
If name contains a %, it's easy to see that this patch doesn't change
anything (other than eliminate the duplicate dev_valid_name
call). Otherwise, we'll now just spend a little time in snprintf()
copying name to the stack buffer allocated in dev_alloc_name_ns, and do
the __dev_get_by_name using that buffer rather than name.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
If we're given format string with no %d, -EEXIST is a saner error code.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
We currently only exclude non-sysfs-friendly names via
dev_get_valid_name; there doesn't seem to be a reason to allow such
names when we're called via dev_alloc_name.
This does duplicate the dev_valid_name check in the dev_get_valid_name()
case; we'll fix that shortly.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
The only caller passes a stack buffer as buf, so it won't equal the
passed-in name. Moreover, we're already using buf as a scratch buffer
inside the if (p) {} block, so if buf and name were the same, that
snprintf() call would be overwriting its own format string.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
dev_alloc_name contained a BUG_ON(), which I moved to dev_alloc_name_ns;
the only other caller of that already has the same BUG_ON.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
__dev_alloc_name is called from the public (and exported)
dev_alloc_name(), so we don't have a guarantee that strlen(name) is at
most IFNAMSIZ. If somebody manages to get __dev_alloc_name called with a
% char beyond the 31st character, we'd be making a snprintf() call that
will very easily crash the kernel (using an appropriate %p extension,
we'll likely dereference some completely bogus pointer).
In the normal case where strlen() is sane, we don't even save anything
by limiting to IFNAMSIZ, so just use strchr().
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
If ffz() ever returns a value >= 31 then the following shift is undefined
behaviour because the literal 1 which gets shifted is treated as signed
integer.
In practice, the bug is probably harmless, since the first undefined shift
count is 31 which results - ignoring UB - in (int)(0x80000000). This gets
sign extended so bit 32-63 will be set as well and all subsequent
__setup_irq() calls would just end up hitting the -EBUSY branch.
However, a sufficiently aggressive optimizer may use the UB of 1<<31
to decide that doesn't happen, and hence elide the sign-extension
code, so that subsequent calls can indeed get ffz > 31.
In any case, the right thing to do is to make the literal 1UL.
[ tglx: For this to happen a single interrupt would have to be shared by 32
devices. Hardware like that does not exist and would have way more
problems than that. ]
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Thomas Gleixner <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
Calling sprintf in a loop is not very efficient, and in any case, we
already have an implementation of bin-to-hex conversion in lib/ which
we might as well use.
Note that ecryptfs_to_hex used to nul-terminate the destination (and
the kernel doc was wrong about the required output size), while
bin2hex doesn't. [All but one user of ecryptfs_to_hex explicitly
nul-terminates the result anyway.]
Signed-off-by: Rasmus Villemoes <[email protected]>
[tyhicks: Include <linux/kernel.h> in ecryptfs_kernel.h]
Signed-off-by: Tyler Hicks <[email protected]>
dprintk already provides a KERN_* prefix; this KERN_INFO just shows up
as some odd characters in the output.
Simplify the message a bit while we're there.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: J. Bruce Fields <[email protected]>
gcc isn't smart enough to realize it can share most of the argument
buildup and the actual function call between the two branches, so help
it a little.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
It takes more .text to initialize these on the stack than they occupy
in .rodata, so just make them static const.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
ibuf and rbuf in lme2510_int_response are always assigned to before they
are read, and their addresses do not escape the function, so they have
no reason to be static.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Using the %ph printf extension for hex dumps like this makes the
generated code quite a bit smaller.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Passing a gazillion arguments takes a lot of code:
add/remove: 0/0 grow/shrink: 0/2 up/down: 0/-253 (-253)
Signed-off-by: Rasmus Villemoes <[email protected]>
Reviewed-by: Pavel Shilovsky <[email protected]>
Signed-off-by: Steve French <[email protected]>
format_decode and vsnprintf occasionally show up in perf top, so I went
looking for places that might not need the full printf power. With the
help of kprobes, I gathered some statistics on which format strings we
mostly pass to vsnprintf. On a trivial desktop workload, I hit "%x" 25%
of the time, so something apparently reads /proc/pid/status (which does
5*16 printf("%x") calls) a lot.
With this patch, reading /proc/pid/status is 30% faster according to
this microbenchmark:
char buf[4096];
int i, fd;
for (i = 0; i < 10000; ++i) {
fd = open("/proc/self/status", O_RDONLY);
read(fd, buf, sizeof(buf));
close(fd);
}
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Rasmus Villemoes <[email protected]>
Acked-by: Andrei Vagin <[email protected]>
Acked-by: Kees Cook <[email protected]>
Cc: Alexey Dobriyan <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Passing one instead of 8 or 16 arguments reduces the size of the
generated code somewhat:
add/remove: 2/3 grow/shrink: 1/4 up/down: 1772/-2137 (-365)
There's one more candidate, unique_id_show, but that uses %02X, and I'm
not sure it would be ok to start using lowercase there, so I've left it
alone for now.
Signed-off-by: Rasmus Villemoes <[email protected]>
Acked-by: Don Brace <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
This is already using the %pM printf extension; might as well also use
%ph to make the code smaller.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
We have the %pU printf extension for doing exactly this. Saves some
.text, and is likely also a little faster.
Signed-off-by: Rasmus Villemoes <[email protected]>
Reviewed-by: Benjamin Tissoires <[email protected]>
Acked-by: Srinivas Pandruvada <[email protected]>
Signed-off-by: Jiri Kosina <[email protected]>
Using a local variable we can prevent gcc from reloading
aio_ring_file->f_inode->i_mapping twice, eliminating 2x2 dependent
loads.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Al Viro <[email protected]>
aDate is always the empty string, so entirely pointless. The aRevision
formatting might as well be done as part of the pr_debug() call - that
also avoids it altogether if pr_debug is compiled out.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Bin Liu <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
cmm_mem_going_offline() is (only) called from cmm_memory_cb(), which
sends the return value through notifier_from_errno(). The latter
expects 0 or -errno (notifier_to_errno(notifier_from_errno(x)) is 0
for any x >= 0, so passing a positive value cannot make sense). Hence
negate ENOMEM.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Michael Ellerman <[email protected]>
aux0_show and aux1_show consists of almost identical code. Pull that
into a common helper and make them thin wrappers. Similarly for
_store.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Darren Hart <[email protected]>
Using kasprintf to get the function name makes us look up the name
twice, along with all the vsnprintf overhead of parsing the format
string etc. It also means there is an allocation failure case to deal
with. Since symbol_string in vsprintf.c would anyway allocate an array
of size KSYM_SYMBOL_LEN on the stack, that might as well be done up
here.
Moreover, since this is a debug feature and the blacklisted_initcalls
list is usually empty, we might as well test that and thus avoid looking
up the symbol name even once in the common case.
Signed-off-by: Rasmus Villemoes <[email protected]>
Acked-by: Rusty Russell <[email protected]>
Acked-by: Prarit Bhargava <[email protected]>
Cc: Oleg Nesterov <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Attach the malloc attribute to a few allocation functions. This helps
gcc generate better code by telling it that the return value doesn't
alias any existing pointers (which is even more valuable given the
pessimizations implied by -fno-strict-aliasing).
A simple example of what this allows gcc to do can be seen by looking at
the last part of drm_atomic_helper_plane_reset:
plane->state = kzalloc(sizeof(*plane->state), GFP_KERNEL);
if (plane->state) {
plane->state->plane = plane;
plane->state->rotation = BIT(DRM_ROTATE_0);
}
which compiles to
e8 99 bf d6 ff callq ffffffff8116d540 <kmem_cache_alloc_trace>
48 85 c0 test %rax,%rax
48 89 83 40 02 00 00 mov %rax,0x240(%rbx)
74 11 je ffffffff814015c4 <drm_atomic_helper_plane_reset+0x64>
48 89 18 mov %rbx,(%rax)
48 8b 83 40 02 00 00 mov 0x240(%rbx),%rax [*]
c7 40 40 01 00 00 00 movl $0x1,0x40(%rax)
With this patch applied, the instruction at [*] is elided, since the
store to plane->state->plane is known to not alter the value of
plane->state.
[[email protected]: coding-style fixes]
Signed-off-by: Rasmus Villemoes <[email protected]>
Cc: Christoph Lameter <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: David Rientjes <[email protected]>
Cc: Joonsoo Kim <[email protected]>
Cc: Andi Kleen <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
gcc as far back as at least 3.04 documents the function attribute
__malloc__. Add a shorthand for attaching that to a function
declaration. This was also suggested by Andi Kleen way back in 2002
[1], but didn't get applied, perhaps because gcc at that time generated
the exact same code with and without this attribute.
This attribute tells the compiler that the return value (if non-NULL)
can be assumed not to alias any other valid pointers at the time of the
call.
Please note that the documentation for a range of gcc versions (starting
from around 4.7) contained a somewhat confusing and self-contradicting
text:
The malloc attribute is used to tell the compiler that a function may
be treated as if any non-NULL pointer it returns cannot alias any other
pointer valid when the function returns and *that the memory has
undefined content*. [...] Standard functions with this property include
malloc and *calloc*.
(emphasis mine). The intended meaning has later been clarified [2]:
This tells the compiler that a function is malloc-like, i.e., that the
pointer P returned by the function cannot alias any other pointer valid
when the function returns, and moreover no pointers to valid objects
occur in any storage addressed by P.
What this means is that we can apply the attribute to kmalloc and
friends, and it is ok for the returned memory to have well-defined
contents (__GFP_ZERO). But it is not ok to apply it to kmemdup(), nor
to other functions which both allocate and possibly initialize the
memory with existing pointers. So unless someone is doing something
pretty perverted kstrdup() should also be a fine candidate.
[1] http://thread.gmane.org/gmane.linux.kernel/57172
[2] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56955
Signed-off-by: Rasmus Villemoes <[email protected]>
Cc: Christoph Lameter <[email protected]>
Cc: Pekka Enberg <[email protected]>
Cc: David Rientjes <[email protected]>
Cc: Joonsoo Kim <[email protected]>
Cc: Andi Kleen <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Passing overlapping source and destination to snprintf is
fragile. Replace with a single (mostly) equivalent call. If one wants
to preserve the space preceding udev->product whether or not there was
a manufacturer, just remove udev->manufacturer from the && expression.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Appending to a string by doing snprintf(buf, bufsize, "%s...", buf,
...) is not guaranteed to work.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
On 64 bit, struct error_info has 6 bytes of padding, which amounts to
over 4k of wasted space in the additional[] array. We could easily get
rid of that by instead using separate arrays for the codes and the
pointers. However, we can do even better than that and save an
additional 6 bytes per entry: In the table, just store the sizeof()
the corresponding string literal. The cumulative sum of these is then
the appropriate offset into additional_text, which is built from the
concatenation (with '\0's inbetween) of the strings.
$ scripts/bloat-o-meter /tmp/vmlinux vmlinux
add/remove: 0/0 grow/shrink: 1/1 up/down: 24/-8488 (-8464)
function old new delta
scsi_extd_sense_format 136 160 +24
additional 11312 2824 -8488
The Kconfig help text used to say that CONFIG_SCSI_CONSTANTS=y costs
around 75 KB, but that was a little exaggerated. The actual number was
closer to 44K, and 36K with this patch.
Signed-off-by: Rasmus Villemoes <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
Tested-by: Douglas Gilbert <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
This is a purely mechanical move of the list of additional sense codes
to a separate file, in preparation for reducing the impact of choosing
CONFIG_SCSI_CONSTANTS=y by about 8k.
Signed-off-by: Rasmus Villemoes <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
There's little point in breaking these strings over multiple lines.
Signed-off-by: Rasmus Villemoes <[email protected]>
Reviewed-by: Hannes Reinecke <[email protected]>
Reviewed-by: Christoph Hellwig <[email protected]>
Signed-off-by: Martin K. Petersen <[email protected]>
There's no reason to duplicate the logic provided by scnprintf().
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Tomas Winkler <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
We need to use post-decrement to ensure that irq_dispose_mapping is
also called on priv->rxq[0]->irq_no; moreover, if one of the above for
loops failed already at i==0 (so we reach one of these labels with
that value of i), we'll enter an essentially infinite loop of
out-of-bounds accesses.
Signed-off-by: Rasmus Villemoes <[email protected]>
Reviewed-by: Francois Romieu <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
The fb_rotate method in struct fb_ops is never actually invoked, and
it's been that way in the entire history of git (in fact, the last
occurrence of the string '->fb_rotate' vanished over 10 years ago,
with b4d8aea6d6, and that merely tested whether the callback
existed). So remove some dead code and make struct fb_obs a little
smaller.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Tomi Valkeinen <[email protected]>
We need to use post-decrement to get the dma_map_page undone also for
i==0, and to avoid some very unpleasant behaviour if dma_map_page
failed already at i==0.
Signed-off-by: Rasmus Villemoes <[email protected]>
Reviewed-by: Ben Skeggs <[email protected]>
Signed-off-by: Dave Airlie <[email protected]>
We know that the avg variable actually ends up holding a 32 bit
quantity, since it's an average of such numbers. It is only a u64
because it is temporarily used to hold the sum. Making it an actual
u32 allows gcc to generate slightly better code, e.g. when computing
the square, it can do a 32x32->64 multiply.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
Computing the integer square root is a rather expensive operation, at
least compared to doing a 64x64 -> 64 multiply (avg*avg) and, on 64
bit platforms, doing an extra comparison to a constant (variance <=
U64_MAX/36).
On 64 bit platforms, this does mean that we add a restriction on the
range of the variance where we end up using the estimate (since
previously the stddev <= ULONG_MAX was a tautology), but on the other
hand, we extend the range quite substantially on 32 bit platforms - in
both cases, we now allow standard deviations up to 715 seconds, which
is for example guaranteed if all observations are less than 1430
seconds.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
We need to use post-decrement to get the pci_map_page undone also for
i==0, and to avoid some very unpleasant behaviour if pci_map_page
failed already at i==0.
Reviewed-by: Christian König <[email protected]>
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
We need to use post-decrement to get the pci_map_page undone also for
i==0, and to avoid some very unpleasant behaviour if pci_map_page
failed already at i==0.
Reviewed-by: Christian König <[email protected]>
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Alex Deucher <[email protected]>
Cc: [email protected]
The current code fails to nvmem_cell_drop(cells[0]) - even worse, if
the loop above fails already at i==0, we'll enter an essentially
infinite loop doing nvmem_cell_drop on cells[-1], cells[-2], ... which
is unlikely to end well.
Also, we're not freeing the temporary backing array cells on the error
path.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
We need to use post-decrement to get percpu_counter_destroy() called on
&wb->stat[0]. Moreover, the pre-decremebt would cause infinite
out-of-bounds accesses if the setup code failed at i==0.
Signed-off-by: Rasmus Villemoes <[email protected]>
Cc: Johannes Weiner <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Vladimir Davydov <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
The while loop after err_slaves should use post-decrement; otherwise
we'll fail to do the kfrees for i==0, and will run into out-of-bounds
accesses if the setup above failed already at i==0.
[I'm not sure why one even bothers populating the ->vlan_filter array:
mlx4.h isn't #included by anything outside
drivers/net/ethernet/mellanox/mlx4/, and "git grep -C2 -w vlan_filter
drivers/net/ethernet/mellanox/mlx4/" seems to suggest that the
vlan_filter elements aren't used at all.]
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Doug Ledford <[email protected]>
Ironically, 7d4020c3c4 ("[media] exynos4-is: fix some warnings when
compiling on arm64") fixed some format string bugs but introduced a
new one. buf_index is a simple int, so it should be printed with %d,
not %pad (which is correctly used for dma_addr_t).
Fixes: 7d4020c3c4 ("[media] exynos4-is: fix some warnings when compiling on arm64")
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
Almost all callers of the set_cpu_* functions pass an explicit true or
false. Making them static inline thus replaces the function calls with a
simple set_bit/clear_bit, saving some .text.
Signed-off-by: Rasmus Villemoes <[email protected]>
Acked-by: Rusty Russell <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Michael Ellerman <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Replace the variables cpu_possible_mask, cpu_online_mask, cpu_present_mask
and cpu_active_mask with macros expanding to expressions of the same type
and value, eliminating some indirection.
Signed-off-by: Rasmus Villemoes <[email protected]>
Acked-by: Rusty Russell <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Michael Ellerman <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
The only user of the lvalue-ness of the cpu_*_mask variables is in
drivers/base/cpu.c, and that is mostly a work-around for the fact that not
even const variables can be used in static initialization. Now that the
underlying struct cpumasks are exposed we can take their address.
Signed-off-by: Rasmus Villemoes <[email protected]>
Acked-by: Rusty Russell <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
Cc: Michael Ellerman <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Exporting the cpumasks __cpu_possible_mask and friends will allow us to
remove the extra indirection through the cpu_*_mask variables. It will
also allow the set_cpu_* functions to become static inlines, which will
give a .text reduction.
Signed-off-by: Rasmus Villemoes <[email protected]>
Acked-by: Rusty Russell <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Michael Ellerman <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Change cpu_possible_bits and friends (online, present, active) from being
bitmaps that happen to have the right size to actually being struct
cpumasks. Also rename them to __cpu_xyz_mask. This is mostly a small
cleanup in preparation for exporting them and, eventually, eliminating the
extra indirection through the cpu_xyz_mask variables.
Signed-off-by: Rasmus Villemoes <[email protected]>
Acked-by: Rusty Russell <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Michael Ellerman <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
The four cpumasks cpu_{possible,online,present,active}_bits are exposed
readonly via the corresponding const variables cpu_xyz_mask. But they are
also accessible for arbitrary writing via the exposed functions
set_cpu_xyz. There's quite a bit of code throughout the kernel which
iterates over or otherwise accesses these bitmaps, and having the access
go via the cpu_xyz_mask variables is nowadays [1] simply a useless
indirection.
It may be that any problem in CS can be solved by an extra level of
indirection, but that doesn't mean every extra indirection solves a
problem. In this case, it even necessitates some minor ugliness (see
4/6).
Patch 1/6 is new in v2, and fixes a build failure on ppc by renaming a
struct member, to avoid problems when the identifier cpu_online_mask
becomes a macro later in the series. The next four patches eliminate the
cpu_xyz_mask variables by simply exposing the actual bitmaps, after
renaming them to discourage direct access - that still happens through
cpu_xyz_mask, which are now simply macros with the same type and value as
they used to have.
After that, there's no longer any reason to have the setter functions be
out-of-line: The boolean parameter is almost always a literal true or
false, so by making them static inlines they will usually compile to one
or two instructions.
For a defconfig build on x86_64, bloat-o-meter says we save ~3000 bytes.
We also save a little stack (stackdelta says 127 functions have a 16 byte
smaller stack frame, while two grow by that amount). Mostly because, when
iterating over the mask, gcc typically loads the value of cpu_xyz_mask
into a callee-saved register and from there into %rdi before each
find_next_bit call - now it can just load the appropriate immediate
address into %rdi before each call.
[1] See Rusty's kind explanation
http://thread.gmane.org/gmane.linux.kernel/2047078/focus=2047722 for
some historic context.
This patch (of 6):
As preparation for eliminating the indirect access to the various global
cpu_*_bits bitmaps via the pointer variables cpu_*_mask, rename the
cpu_online_mask member of struct fadump_crash_info_header to simply
online_mask, thus allowing cpu_online_mask to become a macro.
Signed-off-by: Rasmus Villemoes <[email protected]>
Acked-by: Michael Ellerman <[email protected]>
Cc: Greg Kroah-Hartman <[email protected]>
Cc: Rusty Russell <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
%pT for task->comm has been proposed (several times, I think), but is
not actually implemented. Remove it from printk-formats.txt and add it
back if/when it gets implemented.
Signed-off-by: Rasmus Villemoes <[email protected]>
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
kasprintf relies on being able to replay the formatting and getting the
same result (in particular, the same length). This will almost always
work, but it is possible that the object pointed to by a %s or %p
argument changed under us (so we might get truncated output). Add a
somewhat paranoid sanity check and let's see if it ever triggers.
Signed-off-by: Rasmus Villemoes <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Joe Perches <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Maurizio Lombardi <[email protected]>
Cc: Tejun Heo <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
The field width is overloaded to pass some extra information for some %p
extensions (e.g. #bits for %pb). But we might silently truncate the
passed value when we stash it in struct printf_spec (see e.g.
"lib/vsprintf.c: expand field_width to 24 bits"). Hopefully 23 value
bits should now be enough for everybody, but if not, let's make some
noise.
Do the same for the precision. In both cases, clamping seems more
sensible than truncating. While, according to POSIX, "A negative
precision is taken as if the precision were omitted.", the kernel's
printf has always treated that case as if the precision was 0, so we use
that as lower bound. For the field width, the smallest representable
value is actually -(1<<23), but a negative field width means 'set the
LEFT flag and use the absolute value', so we want the absolute value to
fit.
Signed-off-by: Rasmus Villemoes <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Joe Perches <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Maurizio Lombardi <[email protected]>
Cc: Tejun Heo <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
One consequence of the reorganization of struct printf_spec to make
field_width 24 bits was that number() gained about 180 bytes. Since
spec is never passed to other functions, we can help gcc make number()
lose most of that extra weight by using local variables for the field
width and precision.
Reviewed-by: Andy Shevchenko <[email protected]>
Signed-off-by: Rasmus Villemoes <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Joe Perches <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Maurizio Lombardi <[email protected]>
Cc: Tejun Heo <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
Maurizio Lombardi reported a problem [1] with the %pb extension: It
doesn't work for sufficiently large bitmaps, since the size is stashed
in the field_width field of the struct printf_spec, which is currently
an s16. Concretely, this manifested itself in
/sys/bus/pseudo/drivers/scsi_debug/map being empty, since the bitmap
printer got a size of 0, which is the 16 bit truncation of the actual
bitmap size.
We do want to keep struct printf_spec at 8 bytes so that it can cheaply
be passed by value. The qualifier field is only used for internal
bookkeeping in format_decode, so we might as well use a local variable
for that. This gives us an additional 8 bits, which we can then use for
the field width.
To stay in 8 bytes, we need to do a little rearranging and make the type
member a bitfield as well. For consistency, change all the members to
bit fields. gcc doesn't generate much worse code with these changes (in
fact, bloat-o-meter says we save 300 bytes - which I think is a little
surprising).
I didn't find a BUILD_BUG/compiletime_assertion/... which would work
outside function context, so for now I just open-coded it.
[1] http://thread.gmane.org/gmane.linux.kernel/2034835
[[email protected]: avoid open-coded BUILD_BUG_ON]
Signed-off-by: Rasmus Villemoes <[email protected]>
Reported-by: Maurizio Lombardi <[email protected]>
Acked-by: Tejun Heo <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Joe Perches <[email protected]>
Cc: Kees Cook <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
If the string corresponding to a %s specifier can change under us, we
might end up copying a \0 byte to the output buffer. There might be
callers who expect the output buffer to contain a genuine C string whose
length is exactly the snprintf return value (assuming truncation hasn't
happened or has been checked for).
We can avoid this by only passing over the source string once, stopping
the first time we meet a nul byte (or when we reach the given
precision), and then letting widen_string() handle left/right space
padding. As a small bonus, this code reuse also makes the generated
code slightly smaller.
Signed-off-by: Rasmus Villemoes <[email protected]>
Cc: Al Viro <[email protected]>
Cc: Andy Shevchenko <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Joe Perches <[email protected]>
Cc: Kees Cook <[email protected]>
Cc: Maurizio Lombardi <[email protected]>
Cc: Tejun Heo <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
...and don't do it wrong.
"not ok or N/A" has length 13. Add the trailing newline, and the
snprintf return value will be 14. However, we lied to snprintf and
told it that only 13 bytes were available. Hence snprintf has only
written "not ok or N/" and a trailing '\0' to the buffer. Next we
continue lying, this time to the upper sysfs layer, claiming that we
wrote 14 meaningful bytes to the buffer. That'll make the upper layer
copy "not ok or N/" plus two nul bytes to user space (one nul byte
from snprintf, the other since sysfs takes care to clear the buffer
before giving it to the ->show method).
In the other cases, the claimed buffer size is closer to sufficient,
but we'll still get a nul byte instead of a newline written to user
space. There's absolutely no reason to try to predict the output
size, and there's plenty of room in the buffer, so just use sprintf.
Signed-off-by: Rasmus Villemoes <[email protected]>
Signed-off-by: Alexandre Belloni <[email protected]>
wm831x_unique_id_show currently displays an interesting pattern of '0'
and '3' characters which isn't very useful (figuring out why is left
as an exercise for the reader). Presumably "buf[i]" should have been
"id[i] & 0xff".
But while there, it is much simpler to simply use %phN and do all the
formatting at once.
Signed-off-by: Rasmus Villemoes <[email protected]>
Acked-by: Charles Keepax <[email protected]>
Signed-off-by: Lee Jones <[email protected]>
Rasmus Villemoes