Re: kernel BUG at crypto/asymmetric_keys/public_key.c:80

[Johannes Berg]

On Thu, 2017-11-23 at 09:47 -0800, Florian Fainelli wrote:

> Absolutely, please find it enclosed.

Thanks.

This is a bit odd. I didn't think the most likely reason is that you
have

CONFIG_CRYPTO_SHA256=m

but everything else built-in. Thus, when loading the certificate,
there's no way to calculate the digest since that requires sha-256,
hence

BUG_ON(!sig->digest);

If you make CONFIG_CRYPTO_SHA256=y then it should go away.

I guess I'll do this:

diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig
index da91bb547db3..1abcc4fc4df1 100644
--- a/net/wireless/Kconfig
+++ b/net/wireless/Kconfig
@@ -20,6 +20,10 @@ config CFG80211
tristate "cfg80211 - wireless configuration API"
depends on RFKILL || !RFKILL
select FW_LOADER
+   # may need to update this when certificates are changed and are
+   # using a different algorithm, though right now they shouldn't
+   # (this is here rather than below to allow it to be a module)
+   select CRYPTO_SHA256 if CFG80211_USE_KERNEL_REGDB_KEYS
---help---
  cfg80211 is the Linux wireless LAN (802.11) configuration API.
  Enable this if you have a wireless device.
@@ -113,6 +117,9 @@ config CFG80211_EXTRA_REGDB_KEYDIR
  certificates like in the kernel sources (net/wireless/certs/)
  that shall be accepted for a signed regulatory database.
 
+ Note that you need to also select the correct CRYPTO_ modules
+ for your certificates, and if cfg80211 is built-in they also must be.
+
 config CFG80211_REG_CELLULAR_HINTS
bool "cfg80211 regulatory support for cellular base station hints"
depends on CFG80211_CERTIFICATION_ONUS


Can you try if that fixes your config for you?

johannes

Re: kernel BUG at crypto/asymmetric_keys/public_key.c:80

[Johannes Berg]

On Thu, 2017-11-23 at 09:47 -0800, Florian Fainelli wrote:

> Absolutely, please find it enclosed.

Thanks.

This is a bit odd. I didn't think the most likely reason is that you
have

CONFIG_CRYPTO_SHA256=m

but everything else built-in. Thus, when loading the certificate,
there's no way to calculate the digest since that requires sha-256,
hence

BUG_ON(!sig->digest);

If you make CONFIG_CRYPTO_SHA256=y then it should go away.

I guess I'll do this:

diff --git a/net/wireless/Kconfig b/net/wireless/Kconfig
index da91bb547db3..1abcc4fc4df1 100644
--- a/net/wireless/Kconfig
+++ b/net/wireless/Kconfig
@@ -20,6 +20,10 @@ config CFG80211
tristate "cfg80211 - wireless configuration API"
depends on RFKILL || !RFKILL
select FW_LOADER
+   # may need to update this when certificates are changed and are
+   # using a different algorithm, though right now they shouldn't
+   # (this is here rather than below to allow it to be a module)
+   select CRYPTO_SHA256 if CFG80211_USE_KERNEL_REGDB_KEYS
---help---
  cfg80211 is the Linux wireless LAN (802.11) configuration API.
  Enable this if you have a wireless device.
@@ -113,6 +117,9 @@ config CFG80211_EXTRA_REGDB_KEYDIR
  certificates like in the kernel sources (net/wireless/certs/)
  that shall be accepted for a signed regulatory database.
 
+ Note that you need to also select the correct CRYPTO_ modules
+ for your certificates, and if cfg80211 is built-in they also must be.
+
 config CFG80211_REG_CELLULAR_HINTS
bool "cfg80211 regulatory support for cellular base station hints"
depends on CFG80211_CERTIFICATION_ONUS


Can you try if that fixes your config for you?

johannes

Re: [PATCH 1/1] Input: ims-pcu - fix typo in an error log

[Leizhen (ThunderTown)]

On 2017/11/24 15:17, Joe Perches wrote:
> On Fri, 2017-11-24 at 14:59 +0800, Zhen Lei wrote:
>> Tiny typo fixed in an error log.
>>
>> I found this when I backported the CVE-2017-16645 patch:
>> ea04efee7635 ("Input: ims-psu - check if CDC union descriptor is sane")
>>
>> Signed-off-by: Zhen Lei 
>> ---
>>  drivers/input/misc/ims-pcu.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
> []
>> @@ -1651,7 +1651,7 @@ static void ims_pcu_buffers_free(struct ims_pcu *pcu)
>>  return union_desc;
>>
>>  dev_err(>dev,
>> -"Union descriptor to short (%d vs %zd\n)",
>> +"Union descriptor too short (%d vs %zd\n)",
> 
> And this format is incorrect too.  It should be:
> 
> + "Union descriptor too short (%d vs %zd)\n",
> 
> with the close parenthesis before the newline, not after.
You are very observant. Do I need to post v2? It seems that we can simply 
modify it directly.

> 
> 
> .
> 

-- 
Thanks!
BestRegards

Re: [PATCH 1/1] Input: ims-pcu - fix typo in an error log

[Leizhen (ThunderTown)]

On 2017/11/24 15:17, Joe Perches wrote:
> On Fri, 2017-11-24 at 14:59 +0800, Zhen Lei wrote:
>> Tiny typo fixed in an error log.
>>
>> I found this when I backported the CVE-2017-16645 patch:
>> ea04efee7635 ("Input: ims-psu - check if CDC union descriptor is sane")
>>
>> Signed-off-by: Zhen Lei 
>> ---
>>  drivers/input/misc/ims-pcu.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
> []
>> @@ -1651,7 +1651,7 @@ static void ims_pcu_buffers_free(struct ims_pcu *pcu)
>>  return union_desc;
>>
>>  dev_err(>dev,
>> -"Union descriptor to short (%d vs %zd\n)",
>> +"Union descriptor too short (%d vs %zd\n)",
> 
> And this format is incorrect too.  It should be:
> 
> + "Union descriptor too short (%d vs %zd)\n",
> 
> with the close parenthesis before the newline, not after.
You are very observant. Do I need to post v2? It seems that we can simply 
modify it directly.

> 
> 
> .
> 

-- 
Thanks!
BestRegards

Re: [PATCH v2 18/18] arm64: select ARCH_SUPPORTS_LTO_CLANG

[Dmitry Vyukov]

On Thu, Nov 23, 2017 at 2:42 PM, Alexander Potapenko  wrote:
 > >> > Ideally we'd get the toolchain people to commit to supporting the 
 > >> > kernel
 > >> > memory model along side the C11 one. That would help a ton.
 > >>
 > >> Does anyone from the kernel side participate in the C standardization 
 > >> process?
 > >
 > > Yes, Paul McKenney and Will Deacon. Doesn't mean these two can still be
 > > reconciled though. From what I understand C11 (and onwards) are
 > > incompatible with the kernel model on a number of subtle points.
 >
 > It would be good to have these incompatibilities written down, then
 > for the sake of argument, they can be cited both for discussions on
 > LKML and in the C standardization process.  For example, a running
 > list in Documentation/ or something would make it so that anyone could
 > understand and cite current issues with the latest C standard.

 Will should be able to produce this list; I know he's done before, I
 just can't find it -- my Google-foo isn't strong today.
>>>
>>> Here you go:
>>>
>>> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0124r4.html
>>
>> Great, thanks! Will take some time to digest, but happy to refer
>> others to this important work in the future.
>>
>> I wonder if we have anything like a case study that shows specifically
>> how a compiler generated a subtle bug due to specifics of the memory
>> model, like a "if you do this, here's the problematic code that will
>> get generated, and why it's problematic due to the memory model."
>> That may be a good way to raise issues with toolchain developers with
>> concrete and actionable examples.
>>
 > I don't understand why we'd block patches for enabling experimental
 > features.  We've been running this patch-set on actual devices for
 > months and would love to provide them to the community for further
 > testing.  If bugs are found, then there's more evidence to bring to
 > the C standards committee.  Otherwise we're shutting down feature
 > development for the sake of potential bugs in a C standard we're not
 > even using.

 So the problem is that its very very hard (and painful) to find these
 bugs. Getting the tools people to comment on these specific
 optimizations would really help lots.
>>
>> No doubt; I do not disagree with you.  Kernel developers have very
>> important use cases for the language.
>>
>> But the core point I'm trying to make is "do we need to block this
>> patch set until issues with the C standards body in regards to the
>> kernels memory model are resolved?"  I would hope the two are
>> orthogonal and that we'd take them and then test them even more
>> extensively than the developer has in order to find out.
>>
>>> It would be good to get something similar to LKMM into KTSAN, for
>>> example.  There would probably be a few differences due to efficiency
>>> concerns, but closer is better than less close.  ;-)
>>
>> + glider, who may be able to comment on the state of KTSAN.
> We haven't touched KTSAN for a while, so it's probably broken right now.
> It should be possible to revive it, the question is how much code will
> need to be annotated to prevent the tool from overwhelming the
> developers with reports.
> +Dima and Andrey, who should know better.

Hi,

KTSAN checks acquire/release pairs, and that's very useful. But as far
as I understand this thread is about more subtle things and areas of
kernel/compiler tension. I afraid this in this context KTSAN is in the
same boat as compiler. Because, well, we need to write code that
implements precise algorithms. And if control-dependencies are defined
as "extreme care is required to avoid control-dependency-destroying
compiler optimizations" (that is, code is correct if it does not break
against the current set of enabled optimizations in the current
compiler, what?) and data-dependencies are defined akin to C11
definition (read -- non-implementable, unicorns); then KTSAN can't
help.

When/if C provides implementable rules for data-dependencies
(_Dependency) and that's implemented in compilers and kernel sticks to
this model, then I guess it should be possible to extract that info
from compiler and check against it in KTSAN (e.g. 2 synchronization
domains, one for dependent accesses and one for everything else).
Kernel could as well define own model, and KTSAN could check against
it. But that model must be implemented in compilers first anyway.
Because (1) doing it just for KTSAN does not look reasonable, (2)
until compiler supports that model there is little point in checking
(the fact that compiler uses a different model is the major gaping
hole and we are aware of it without tooling help).

And, yes, I agree that we should not block this LTO patch. All
problems are already there and are orthogonal to LTO. Compiler sees
enough code already (large TUs, lots of code in headers) 

Re: [PATCH v2 18/18] arm64: select ARCH_SUPPORTS_LTO_CLANG

[Dmitry Vyukov]

On Thu, Nov 23, 2017 at 2:42 PM, Alexander Potapenko  wrote:
 > >> > Ideally we'd get the toolchain people to commit to supporting the 
 > >> > kernel
 > >> > memory model along side the C11 one. That would help a ton.
 > >>
 > >> Does anyone from the kernel side participate in the C standardization 
 > >> process?
 > >
 > > Yes, Paul McKenney and Will Deacon. Doesn't mean these two can still be
 > > reconciled though. From what I understand C11 (and onwards) are
 > > incompatible with the kernel model on a number of subtle points.
 >
 > It would be good to have these incompatibilities written down, then
 > for the sake of argument, they can be cited both for discussions on
 > LKML and in the C standardization process.  For example, a running
 > list in Documentation/ or something would make it so that anyone could
 > understand and cite current issues with the latest C standard.

 Will should be able to produce this list; I know he's done before, I
 just can't find it -- my Google-foo isn't strong today.
>>>
>>> Here you go:
>>>
>>> http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0124r4.html
>>
>> Great, thanks! Will take some time to digest, but happy to refer
>> others to this important work in the future.
>>
>> I wonder if we have anything like a case study that shows specifically
>> how a compiler generated a subtle bug due to specifics of the memory
>> model, like a "if you do this, here's the problematic code that will
>> get generated, and why it's problematic due to the memory model."
>> That may be a good way to raise issues with toolchain developers with
>> concrete and actionable examples.
>>
 > I don't understand why we'd block patches for enabling experimental
 > features.  We've been running this patch-set on actual devices for
 > months and would love to provide them to the community for further
 > testing.  If bugs are found, then there's more evidence to bring to
 > the C standards committee.  Otherwise we're shutting down feature
 > development for the sake of potential bugs in a C standard we're not
 > even using.

 So the problem is that its very very hard (and painful) to find these
 bugs. Getting the tools people to comment on these specific
 optimizations would really help lots.
>>
>> No doubt; I do not disagree with you.  Kernel developers have very
>> important use cases for the language.
>>
>> But the core point I'm trying to make is "do we need to block this
>> patch set until issues with the C standards body in regards to the
>> kernels memory model are resolved?"  I would hope the two are
>> orthogonal and that we'd take them and then test them even more
>> extensively than the developer has in order to find out.
>>
>>> It would be good to get something similar to LKMM into KTSAN, for
>>> example.  There would probably be a few differences due to efficiency
>>> concerns, but closer is better than less close.  ;-)
>>
>> + glider, who may be able to comment on the state of KTSAN.
> We haven't touched KTSAN for a while, so it's probably broken right now.
> It should be possible to revive it, the question is how much code will
> need to be annotated to prevent the tool from overwhelming the
> developers with reports.
> +Dima and Andrey, who should know better.

Hi,

KTSAN checks acquire/release pairs, and that's very useful. But as far
as I understand this thread is about more subtle things and areas of
kernel/compiler tension. I afraid this in this context KTSAN is in the
same boat as compiler. Because, well, we need to write code that
implements precise algorithms. And if control-dependencies are defined
as "extreme care is required to avoid control-dependency-destroying
compiler optimizations" (that is, code is correct if it does not break
against the current set of enabled optimizations in the current
compiler, what?) and data-dependencies are defined akin to C11
definition (read -- non-implementable, unicorns); then KTSAN can't
help.

When/if C provides implementable rules for data-dependencies
(_Dependency) and that's implemented in compilers and kernel sticks to
this model, then I guess it should be possible to extract that info
from compiler and check against it in KTSAN (e.g. 2 synchronization
domains, one for dependent accesses and one for everything else).
Kernel could as well define own model, and KTSAN could check against
it. But that model must be implemented in compilers first anyway.
Because (1) doing it just for KTSAN does not look reasonable, (2)
until compiler supports that model there is little point in checking
(the fact that compiler uses a different model is the major gaping
hole and we are aware of it without tooling help).

And, yes, I agree that we should not block this LTO patch. All
problems are already there and are orthogonal to LTO. Compiler sees
enough code already (large TUs, lots of code in headers) and we move
code. I 

Re: [PATCH] media: coda: fix comparision of decoded frames' indexes

[Martin Kepplinger]

Am 22.11.2017 14:43 schrieb Philipp Zabel:

Hi Martin,

On Fri, 2017-11-17 at 15:30 +0100, Martin Kepplinger wrote:

At this point the driver looks the currently decoded frame's index
and compares is to VPU-specific state values. Directly before this
if and else statements the indexes are read (index for decoded and
for displayed frame).

Now what is saved in ctx->display_idx is an older value at this point!


Yes. The rotator that copies out the decoded frame runs in parallel 
with

the decoding of the next frame. So the decoder signals with display_idx
which decoded frame should be presented next, but it is only copied out
into the vb2 buffer during the following run. The same happens with the
VDOA, but manually, in prepare_decode.

That means that at this point display_idx is the index of the 
previously

decoded internal frame that should be presented next, and ctx-

display_idx is the index of the internal frame that was just copied

into the externally visible vb2 buffer.

The logic reads someting like this:

if (no frame was decoded) {
if (a frame will be copied out next time) {
adapt sequence number offset;
} else if (no frame was copied out this time) {
hold until further input;
}
}

Basically, it will just wait one more run until it stops the stream,
assuming that there is really nothing useful in the bitstream
ringbuffer.


During these index checks, the current values apply, so fix this by
taking display_idx instead of ctx->display_idx.


display_idx is already checked later in the same function.
According to the i.MX6 VPU API document, it can be -2 (never seen) or 
-3

during sequence start (if there is frame reordering, depending on
whether decoder skip is enabled), and I think I've seen -3 as prescan
failure on i.MX5. -1 means EOS according to that document, that's why 
we

always hold in that case.


ctx->display_idx is updated later in the same function.

Signed-off-by: Martin Kepplinger 
---

Please review this thoroughly, but in case I am wrong here, this is
at least very strange to read and *should* be accompanied with a
comment about what's going on with that index value!


Maybe it would be less confusing to move this into the display_idx
checks below, given that we already hold unconditionally
on display_idx == -1.


I don't think it matter that much here because at least playing h264
worked before and works with this change, but I've tested it anyways.


I think this shouldn't happen at all if you feed it a well formed h.264
stream. But if you have a skip because of broken data while there is
still no display frame at the beginning of the stream or after an IDR,
this might be hit.


Right. Let's leave it this way. In case of real changes, one can think 
about

cleanup.

Thanks for clarification and helping to understand this thing! I 
appreciate it.


 martin

Re: [PATCH] media: coda: fix comparision of decoded frames' indexes

[Martin Kepplinger]

Am 22.11.2017 14:43 schrieb Philipp Zabel:

Hi Martin,

On Fri, 2017-11-17 at 15:30 +0100, Martin Kepplinger wrote:

At this point the driver looks the currently decoded frame's index
and compares is to VPU-specific state values. Directly before this
if and else statements the indexes are read (index for decoded and
for displayed frame).

Now what is saved in ctx->display_idx is an older value at this point!


Yes. The rotator that copies out the decoded frame runs in parallel 
with

the decoding of the next frame. So the decoder signals with display_idx
which decoded frame should be presented next, but it is only copied out
into the vb2 buffer during the following run. The same happens with the
VDOA, but manually, in prepare_decode.

That means that at this point display_idx is the index of the 
previously

decoded internal frame that should be presented next, and ctx-

display_idx is the index of the internal frame that was just copied

into the externally visible vb2 buffer.

The logic reads someting like this:

if (no frame was decoded) {
if (a frame will be copied out next time) {
adapt sequence number offset;
} else if (no frame was copied out this time) {
hold until further input;
}
}

Basically, it will just wait one more run until it stops the stream,
assuming that there is really nothing useful in the bitstream
ringbuffer.


During these index checks, the current values apply, so fix this by
taking display_idx instead of ctx->display_idx.


display_idx is already checked later in the same function.
According to the i.MX6 VPU API document, it can be -2 (never seen) or 
-3

during sequence start (if there is frame reordering, depending on
whether decoder skip is enabled), and I think I've seen -3 as prescan
failure on i.MX5. -1 means EOS according to that document, that's why 
we

always hold in that case.


ctx->display_idx is updated later in the same function.

Signed-off-by: Martin Kepplinger 
---

Please review this thoroughly, but in case I am wrong here, this is
at least very strange to read and *should* be accompanied with a
comment about what's going on with that index value!


Maybe it would be less confusing to move this into the display_idx
checks below, given that we already hold unconditionally
on display_idx == -1.


I don't think it matter that much here because at least playing h264
worked before and works with this change, but I've tested it anyways.


I think this shouldn't happen at all if you feed it a well formed h.264
stream. But if you have a skip because of broken data while there is
still no display frame at the beginning of the stream or after an IDR,
this might be hit.


Right. Let's leave it this way. In case of real changes, one can think 
about

cleanup.

Thanks for clarification and helping to understand this thing! I 
appreciate it.


 martin

Re: [PATCH] mm: Do not stall register_shrinker

[Michal Hocko]

On Fri 24-11-17 09:04:59, Minchan Kim wrote:
> Shakeel Butt reported, he have observed in production system that
> the job loader gets stuck for 10s of seconds while doing mount
> operation. It turns out that it was stuck in register_shrinker()
> and some unrelated job was under memory pressure and spending time
> in shrink_slab(). Machines have a lot of shrinkers registered and
> jobs under memory pressure has to traverse all of those memcg-aware
> shrinkers and do affect unrelated jobs which want to register their
> own shrinkers.
> 
> To solve the issue, this patch simply bails out slab shrinking
> once it found someone want to register shrinker in parallel.
> A downside is it could cause unfair shrinking between shrinkers.
> However, it should be rare and we can add compilcated logic once
> we found it's not enough.
> 
> Link: http://lkml.kernel.org/r/20171115005602.GB23810@bbox
> Cc: Michal Hocko 
> Cc: Tetsuo Handa 
> Acked-by: Johannes Weiner 
> Reported-and-tested-by: Shakeel Butt 
> Signed-off-by: Shakeel Butt 
> Signed-off-by: Minchan Kim 

Acked-by: Michal Hocko 

> ---
>  mm/vmscan.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 6a5a72baccd5..6698001787bd 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -486,6 +486,14 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid,
>   sc.nid = 0;
>  
>   freed += do_shrink_slab(, shrinker, priority);
> + /*
> +  * bail out if someone want to register a new shrinker to
> +  * prevent long time stall by parallel ongoing shrinking.
> +  */
> + if (rwsem_is_contended(_rwsem)) {
> + freed = freed ? : 1;
> + break;
> + }
>   }
>  
>   up_read(_rwsem);
> -- 
> 2.7.4
> 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm: Do not stall register_shrinker

[Michal Hocko]

On Fri 24-11-17 09:04:59, Minchan Kim wrote:
> Shakeel Butt reported, he have observed in production system that
> the job loader gets stuck for 10s of seconds while doing mount
> operation. It turns out that it was stuck in register_shrinker()
> and some unrelated job was under memory pressure and spending time
> in shrink_slab(). Machines have a lot of shrinkers registered and
> jobs under memory pressure has to traverse all of those memcg-aware
> shrinkers and do affect unrelated jobs which want to register their
> own shrinkers.
> 
> To solve the issue, this patch simply bails out slab shrinking
> once it found someone want to register shrinker in parallel.
> A downside is it could cause unfair shrinking between shrinkers.
> However, it should be rare and we can add compilcated logic once
> we found it's not enough.
> 
> Link: http://lkml.kernel.org/r/20171115005602.GB23810@bbox
> Cc: Michal Hocko 
> Cc: Tetsuo Handa 
> Acked-by: Johannes Weiner 
> Reported-and-tested-by: Shakeel Butt 
> Signed-off-by: Shakeel Butt 
> Signed-off-by: Minchan Kim 

Acked-by: Michal Hocko 

> ---
>  mm/vmscan.c | 8 
>  1 file changed, 8 insertions(+)
> 
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 6a5a72baccd5..6698001787bd 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -486,6 +486,14 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid,
>   sc.nid = 0;
>  
>   freed += do_shrink_slab(, shrinker, priority);
> + /*
> +  * bail out if someone want to register a new shrinker to
> +  * prevent long time stall by parallel ongoing shrinking.
> +  */
> + if (rwsem_is_contended(_rwsem)) {
> + freed = freed ? : 1;
> + break;
> + }
>   }
>  
>   up_read(_rwsem);
> -- 
> 2.7.4
> 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH v2] fs: handle shrinker registration failure in sget_userns

[Michal Hocko]

On Fri 24-11-17 06:51:09, Tetsuo Handa wrote:
> Al Viro wrote:
> > On Fri, Nov 24, 2017 at 12:35:29AM +0900, Tetsuo Handa wrote:
> > > Al Viro wrote:
> > > > On Fri, Nov 24, 2017 at 12:04:23AM +0900, Tetsuo Handa wrote:
> > > > > Al Viro wrote:
> > > > > > On Thu, Nov 23, 2017 at 03:35:37PM +0100, Michal Hocko wrote:
> > > > > > > Hopefully less screwed version.  But as I've said I am not really
> > > > > > > familiar with the code and do not feel competent to change it so 
> > > > > > > please
> > > > > > > be very careful here. I've moved the shrinker registration to
> > > > > > > alloc_super which turned out to be simpler.
> > > > > > 
> > > > > > I don't get it.  Why the hell do we need all that PITA in the first 
> > > > > > place?
> > > > > > Just make sget_userns() end with
> > > > > > if (unlikely(regsiter_shrinker(>s_shrink) != 0)) {
> > > > > > deactivate_locked_super(s);
> > > > > > s = ERR_PTR(-ENOMEM);
> > > > > > }
> > > > > > return s;
> > > > > > and be done with that.  All there is to it...
> > > > > > 
> > > > > 
> > > > > Doesn't deactivate_locked_super() call unregister_shrinker() ?
> > > > 
> > > > And?  unregister_shrinker() will do list_del() on empty list_head
> > > > and kfree(NULL); where's the problem with that?
> > > > 
> > > The problem is that calling unregister_shrinker() without successful
> > > register_shrinker() causes crash due to s_shrink.list.{prev,next} == NULL.
> > 
> > *shrug*
> > shrinker->nr_deferred = kzalloc(size, GFP_KERNEL);
> > if (!shrinker->nr_deferred) {
> > /* make sure it's in consistent state */
> > INIT_LIST_HEAD(>list);
> > return -ENOMEM;
> > }
> > 
> > 
> 
> Yes, that will work.
> 
> Michal, like Al thinks, making unregister_shrinker() no-op if
> register_shrinker() failed simplifies things. Can we go with
> http://lkml.kernel.org/r/1511265853-15654-1-git-send-email-penguin-ker...@i-love.sakura.ne.jp
> with patch description updated to include Syzbot report?

I am not opposed to that patch. I just want it to make sure callers _do_
handle the error because a missing shrinker can cause memory pressure
realated issues. unregister_shrinker definitely shouldn't blow up but
I wanted it to warn. This would however trigger a false positive in this
path, right? It is true that the allocation failure would already
trigger warning so the clean up path could be more relaxed. It can be
still quite some time between those two events.

In any case. I do not have a strong preference. If relying on
deactivate_locked_super is really seem much better for the vfs code then 
let's go with your patch without warning.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH v2] fs: handle shrinker registration failure in sget_userns

[Michal Hocko]

On Fri 24-11-17 06:51:09, Tetsuo Handa wrote:
> Al Viro wrote:
> > On Fri, Nov 24, 2017 at 12:35:29AM +0900, Tetsuo Handa wrote:
> > > Al Viro wrote:
> > > > On Fri, Nov 24, 2017 at 12:04:23AM +0900, Tetsuo Handa wrote:
> > > > > Al Viro wrote:
> > > > > > On Thu, Nov 23, 2017 at 03:35:37PM +0100, Michal Hocko wrote:
> > > > > > > Hopefully less screwed version.  But as I've said I am not really
> > > > > > > familiar with the code and do not feel competent to change it so 
> > > > > > > please
> > > > > > > be very careful here. I've moved the shrinker registration to
> > > > > > > alloc_super which turned out to be simpler.
> > > > > > 
> > > > > > I don't get it.  Why the hell do we need all that PITA in the first 
> > > > > > place?
> > > > > > Just make sget_userns() end with
> > > > > > if (unlikely(regsiter_shrinker(>s_shrink) != 0)) {
> > > > > > deactivate_locked_super(s);
> > > > > > s = ERR_PTR(-ENOMEM);
> > > > > > }
> > > > > > return s;
> > > > > > and be done with that.  All there is to it...
> > > > > > 
> > > > > 
> > > > > Doesn't deactivate_locked_super() call unregister_shrinker() ?
> > > > 
> > > > And?  unregister_shrinker() will do list_del() on empty list_head
> > > > and kfree(NULL); where's the problem with that?
> > > > 
> > > The problem is that calling unregister_shrinker() without successful
> > > register_shrinker() causes crash due to s_shrink.list.{prev,next} == NULL.
> > 
> > *shrug*
> > shrinker->nr_deferred = kzalloc(size, GFP_KERNEL);
> > if (!shrinker->nr_deferred) {
> > /* make sure it's in consistent state */
> > INIT_LIST_HEAD(>list);
> > return -ENOMEM;
> > }
> > 
> > 
> 
> Yes, that will work.
> 
> Michal, like Al thinks, making unregister_shrinker() no-op if
> register_shrinker() failed simplifies things. Can we go with
> http://lkml.kernel.org/r/1511265853-15654-1-git-send-email-penguin-ker...@i-love.sakura.ne.jp
> with patch description updated to include Syzbot report?

I am not opposed to that patch. I just want it to make sure callers _do_
handle the error because a missing shrinker can cause memory pressure
realated issues. unregister_shrinker definitely shouldn't blow up but
I wanted it to warn. This would however trigger a false positive in this
path, right? It is true that the allocation failure would already
trigger warning so the clean up path could be more relaxed. It can be
still quite some time between those two events.

In any case. I do not have a strong preference. If relying on
deactivate_locked_super is really seem much better for the vfs code then 
let's go with your patch without warning.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH] Add slowpath enter/exit trace events

[peter enderborg]

On 11/23/2017 02:43 PM, Tetsuo Handa wrote:
> Please see my attempt at
> http://lkml.kernel.org/r/1510833448-19918-1-git-send-email-penguin-ker...@i-love.sakura.ne.jp
>  .
> Printing just current thread is not sufficient for me.
>
>
Seems to  me that it is a lot more overhead with timers and stuff.
My probe is for the health of the system trying to capture how get the penalty 
and how much. A slowpath alloc in a audio stream can causes drop-outs. And they 
are very hard to debug in userspace.

Re: [PATCH] Add slowpath enter/exit trace events

[peter enderborg]

On 11/23/2017 02:43 PM, Tetsuo Handa wrote:
> Please see my attempt at
> http://lkml.kernel.org/r/1510833448-19918-1-git-send-email-penguin-ker...@i-love.sakura.ne.jp
>  .
> Printing just current thread is not sufficient for me.
>
>
Seems to  me that it is a lot more overhead with timers and stuff.
My probe is for the health of the system trying to capture how get the penalty 
and how much. A slowpath alloc in a audio stream can causes drop-outs. And they 
are very hard to debug in userspace.

[PATCH 4/4] ASoC: wm2000: Improve a size determination in wm2000_i2c_probe()

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 24 Nov 2017 08:18:14 +0100

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 sound/soc/codecs/wm2000.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/codecs/wm2000.c b/sound/soc/codecs/wm2000.c
index 2151e75ee5c6..86e7f9ebab44 100644
--- a/sound/soc/codecs/wm2000.c
+++ b/sound/soc/codecs/wm2000.c
@@ -826,8 +826,7 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
int reg;
u16 id;
 
-   wm2000 = devm_kzalloc(>dev, sizeof(struct wm2000_priv),
- GFP_KERNEL);
+   wm2000 = devm_kzalloc(>dev, sizeof(*wm2000), GFP_KERNEL);
if (!wm2000)
return -ENOMEM;
 
-- 
2.15.0

[PATCH 4/4] ASoC: wm2000: Improve a size determination in wm2000_i2c_probe()

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 24 Nov 2017 08:18:14 +0100

Replace the specification of a data structure by a pointer dereference
as the parameter for the operator "sizeof" to make the corresponding size
determination a bit safer according to the Linux coding style convention.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 sound/soc/codecs/wm2000.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/sound/soc/codecs/wm2000.c b/sound/soc/codecs/wm2000.c
index 2151e75ee5c6..86e7f9ebab44 100644
--- a/sound/soc/codecs/wm2000.c
+++ b/sound/soc/codecs/wm2000.c
@@ -826,8 +826,7 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
int reg;
u16 id;
 
-   wm2000 = devm_kzalloc(>dev, sizeof(struct wm2000_priv),
- GFP_KERNEL);
+   wm2000 = devm_kzalloc(>dev, sizeof(*wm2000), GFP_KERNEL);
if (!wm2000)
return -ENOMEM;
 
-- 
2.15.0

[PATCH v2] xfs: handle register_shrinker error

[Michal Hocko]

On Fri 24-11-17 09:00:46, Dave Chinner wrote:
> On Thu, Nov 23, 2017 at 05:11:37PM +0100, Michal Hocko wrote:
> > On Fri 24-11-17 01:01:10, Tetsuo Handa wrote:
> > > Michal Hocko wrote:
> > > > On Thu 23-11-17 05:26:33, Christoph Hellwig wrote:
> > > > > Looks good,
> > > > > 
> > > > > Reviewed-by: Christoph Hellwig 
> > > > 
> > > > Thanks!
> > > > 
> > > > > I can take a stab at the quota one.
> > > > 
> > > > That would be really great!
> > > > 
> > > Again, it does not look good. Since kmem_free() does only kvfree(),
> > > nothing will release memory allocated by list_lru_init().
> > 
> > Hmm, you are right. I have (blindly) followed the current code flow
> > which is wrong as well. The following should do the trick. Should I
> > split that into two patches?
> 
> One is fine by me - if we're need to backport one fix, then we need
> to backport both :/

OK

> > ---
> > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> > index dd0e18af990c..4c6e86d861fd 100644
> > --- a/fs/xfs/xfs_buf.c
> > +++ b/fs/xfs/xfs_buf.c
> > @@ -1815,25 +1815,27 @@ xfs_alloc_buftarg(
> > btp->bt_daxdev = dax_dev;
> >  
> > if (xfs_setsize_buftarg_early(btp, bdev))
> > -   goto error;
> > +   goto error_free;
> >  
> > if (list_lru_init(>bt_lru))
> > -   goto error;
> > +   goto error_free;
> >  
> > if (percpu_counter_init(>bt_io_count, 0, GFP_KERNEL))
> > -   goto error;
> > +   goto error_lru;
> >  
> > btp->bt_shrinker.count_objects = xfs_buftarg_shrink_count;
> > btp->bt_shrinker.scan_objects = xfs_buftarg_shrink_scan;
> > btp->bt_shrinker.seeks = DEFAULT_SEEKS;
> > btp->bt_shrinker.flags = SHRINKER_NUMA_AWARE;
> > -   if (register_shrinker(>bt_shrinker)) {
> > -   percpu_counter_destroy(>bt_io_count);
> > -   goto error;
> > -   }
> > +   if (register_shrinker(>bt_shrinker))
> > +   goto error_pcpu;
> > return btp;
> >  
> > -error:
> > +error_pcpu:
> > +   percpu_counter_destroy(>bt_io_count);
> > +error_lru:
> > +   list_lru_destroy(>bt_lru);
> > +error_free:
> > kmem_free(btp);
> > return NULL;
> 
> That should do the trick.
> 
> Acked-by: Dave Chinner 

Thanks. Updated patch below
---
>From 1009db61988c48c9a9e327a9d076945b29b02eee Mon Sep 17 00:00:00 2001
From: Michal Hocko 
Date: Thu, 23 Nov 2017 17:13:40 +0100
Subject: [PATCH] xfs: fortify xfs_alloc_buftarg error handling

percpu_counter_init failure path doesn't clean up >bt_lru list.
Call list_lru_destroy in that error path. Similarly register_shrinker
error path is not handled.

While it is unlikely to trigger these error path, it is not impossible
especially the later might fail with large NUMAs.  Let's handle the
failure to make the code more robust.

Acked-by: Dave Chinner 
Noticed-by: Tetsuo Handa 
Signed-off-by: Michal Hocko 
---
 fs/xfs/xfs_buf.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 4db6e8d780f6..4c6e86d861fd 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1815,22 +1815,27 @@ xfs_alloc_buftarg(
btp->bt_daxdev = dax_dev;
 
if (xfs_setsize_buftarg_early(btp, bdev))
-   goto error;
+   goto error_free;
 
if (list_lru_init(>bt_lru))
-   goto error;
+   goto error_free;
 
if (percpu_counter_init(>bt_io_count, 0, GFP_KERNEL))
-   goto error;
+   goto error_lru;
 
btp->bt_shrinker.count_objects = xfs_buftarg_shrink_count;
btp->bt_shrinker.scan_objects = xfs_buftarg_shrink_scan;
btp->bt_shrinker.seeks = DEFAULT_SEEKS;
btp->bt_shrinker.flags = SHRINKER_NUMA_AWARE;
-   register_shrinker(>bt_shrinker);
+   if (register_shrinker(>bt_shrinker))
+   goto error_pcpu;
return btp;
 
-error:
+error_pcpu:
+   percpu_counter_destroy(>bt_io_count);
+error_lru:
+   list_lru_destroy(>bt_lru);
+error_free:
kmem_free(btp);
return NULL;
 }
-- 
2.15.0

-- 
Michal Hocko
SUSE Labs

[PATCH v2] xfs: handle register_shrinker error

[Michal Hocko]

On Fri 24-11-17 09:00:46, Dave Chinner wrote:
> On Thu, Nov 23, 2017 at 05:11:37PM +0100, Michal Hocko wrote:
> > On Fri 24-11-17 01:01:10, Tetsuo Handa wrote:
> > > Michal Hocko wrote:
> > > > On Thu 23-11-17 05:26:33, Christoph Hellwig wrote:
> > > > > Looks good,
> > > > > 
> > > > > Reviewed-by: Christoph Hellwig 
> > > > 
> > > > Thanks!
> > > > 
> > > > > I can take a stab at the quota one.
> > > > 
> > > > That would be really great!
> > > > 
> > > Again, it does not look good. Since kmem_free() does only kvfree(),
> > > nothing will release memory allocated by list_lru_init().
> > 
> > Hmm, you are right. I have (blindly) followed the current code flow
> > which is wrong as well. The following should do the trick. Should I
> > split that into two patches?
> 
> One is fine by me - if we're need to backport one fix, then we need
> to backport both :/

OK

> > ---
> > diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
> > index dd0e18af990c..4c6e86d861fd 100644
> > --- a/fs/xfs/xfs_buf.c
> > +++ b/fs/xfs/xfs_buf.c
> > @@ -1815,25 +1815,27 @@ xfs_alloc_buftarg(
> > btp->bt_daxdev = dax_dev;
> >  
> > if (xfs_setsize_buftarg_early(btp, bdev))
> > -   goto error;
> > +   goto error_free;
> >  
> > if (list_lru_init(>bt_lru))
> > -   goto error;
> > +   goto error_free;
> >  
> > if (percpu_counter_init(>bt_io_count, 0, GFP_KERNEL))
> > -   goto error;
> > +   goto error_lru;
> >  
> > btp->bt_shrinker.count_objects = xfs_buftarg_shrink_count;
> > btp->bt_shrinker.scan_objects = xfs_buftarg_shrink_scan;
> > btp->bt_shrinker.seeks = DEFAULT_SEEKS;
> > btp->bt_shrinker.flags = SHRINKER_NUMA_AWARE;
> > -   if (register_shrinker(>bt_shrinker)) {
> > -   percpu_counter_destroy(>bt_io_count);
> > -   goto error;
> > -   }
> > +   if (register_shrinker(>bt_shrinker))
> > +   goto error_pcpu;
> > return btp;
> >  
> > -error:
> > +error_pcpu:
> > +   percpu_counter_destroy(>bt_io_count);
> > +error_lru:
> > +   list_lru_destroy(>bt_lru);
> > +error_free:
> > kmem_free(btp);
> > return NULL;
> 
> That should do the trick.
> 
> Acked-by: Dave Chinner 

Thanks. Updated patch below
---
>From 1009db61988c48c9a9e327a9d076945b29b02eee Mon Sep 17 00:00:00 2001
From: Michal Hocko 
Date: Thu, 23 Nov 2017 17:13:40 +0100
Subject: [PATCH] xfs: fortify xfs_alloc_buftarg error handling

percpu_counter_init failure path doesn't clean up >bt_lru list.
Call list_lru_destroy in that error path. Similarly register_shrinker
error path is not handled.

While it is unlikely to trigger these error path, it is not impossible
especially the later might fail with large NUMAs.  Let's handle the
failure to make the code more robust.

Acked-by: Dave Chinner 
Noticed-by: Tetsuo Handa 
Signed-off-by: Michal Hocko 
---
 fs/xfs/xfs_buf.c | 15 ++-
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/fs/xfs/xfs_buf.c b/fs/xfs/xfs_buf.c
index 4db6e8d780f6..4c6e86d861fd 100644
--- a/fs/xfs/xfs_buf.c
+++ b/fs/xfs/xfs_buf.c
@@ -1815,22 +1815,27 @@ xfs_alloc_buftarg(
btp->bt_daxdev = dax_dev;
 
if (xfs_setsize_buftarg_early(btp, bdev))
-   goto error;
+   goto error_free;
 
if (list_lru_init(>bt_lru))
-   goto error;
+   goto error_free;
 
if (percpu_counter_init(>bt_io_count, 0, GFP_KERNEL))
-   goto error;
+   goto error_lru;
 
btp->bt_shrinker.count_objects = xfs_buftarg_shrink_count;
btp->bt_shrinker.scan_objects = xfs_buftarg_shrink_scan;
btp->bt_shrinker.seeks = DEFAULT_SEEKS;
btp->bt_shrinker.flags = SHRINKER_NUMA_AWARE;
-   register_shrinker(>bt_shrinker);
+   if (register_shrinker(>bt_shrinker))
+   goto error_pcpu;
return btp;
 
-error:
+error_pcpu:
+   percpu_counter_destroy(>bt_io_count);
+error_lru:
+   list_lru_destroy(>bt_lru);
+error_free:
kmem_free(btp);
return NULL;
 }
-- 
2.15.0

-- 
Michal Hocko
SUSE Labs

[PATCH 3/4] ASoC: wm2000: Fix a typo in a comment line

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 24 Nov 2017 08:02:57 +0100

Delete a duplicate character in a word of this description.

Signed-off-by: Markus Elfring 
---
 sound/soc/codecs/wm2000.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/wm2000.c b/sound/soc/codecs/wm2000.c
index 0ed2a8992df4..2151e75ee5c6 100644
--- a/sound/soc/codecs/wm2000.c
+++ b/sound/soc/codecs/wm2000.c
@@ -13,7 +13,7 @@
  * 'wm2000_anc.bin' by default (overridable via platform data) at
  * runtime and is expected to be in flat binary format.  This is
  * generated by Wolfson configuration tools and includes
- * system-specific callibration information.  If supplied as a
+ * system-specific calibration information.  If supplied as a
  * sequence of ASCII-encoded hexidecimal bytes this can be converted
  * into a flat binary with a command such as this on the command line:
  *
-- 
2.15.0

[PATCH 3/4] ASoC: wm2000: Fix a typo in a comment line

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 24 Nov 2017 08:02:57 +0100

Delete a duplicate character in a word of this description.

Signed-off-by: Markus Elfring 
---
 sound/soc/codecs/wm2000.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/wm2000.c b/sound/soc/codecs/wm2000.c
index 0ed2a8992df4..2151e75ee5c6 100644
--- a/sound/soc/codecs/wm2000.c
+++ b/sound/soc/codecs/wm2000.c
@@ -13,7 +13,7 @@
  * 'wm2000_anc.bin' by default (overridable via platform data) at
  * runtime and is expected to be in flat binary format.  This is
  * generated by Wolfson configuration tools and includes
- * system-specific callibration information.  If supplied as a
+ * system-specific calibration information.  If supplied as a
  * sequence of ASCII-encoded hexidecimal bytes this can be converted
  * into a flat binary with a command such as this on the command line:
  *
-- 
2.15.0

[PATCH 2/4] ASoC: wm2000: One function call less in wm2000_i2c_probe() after error detection

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 24 Nov 2017 07:45:59 +0100

The release_firmware() function was called in a few cases by the
wm2000_i2c_probe() function during error handling even if
the passed variable contained a null pointer.

* Adjust jump targets according to the Linux coding style convention.

* Delete the label "out" and an initialisation for the variable "fw"
  at the beginning which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring 
---
 sound/soc/codecs/wm2000.c | 22 ++
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/sound/soc/codecs/wm2000.c b/sound/soc/codecs/wm2000.c
index ce936deed7e3..0ed2a8992df4 100644
--- a/sound/soc/codecs/wm2000.c
+++ b/sound/soc/codecs/wm2000.c
@@ -821,7 +821,7 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
struct wm2000_priv *wm2000;
struct wm2000_platform_data *pdata;
const char *filename;
-   const struct firmware *fw = NULL;
+   const struct firmware *fw;
int ret, i;
int reg;
u16 id;
@@ -840,7 +840,7 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
ret = PTR_ERR(wm2000->regmap);
dev_err(>dev, "Failed to allocate register map: %d\n",
ret);
-   goto out;
+   return ret;
}
 
for (i = 0; i < WM2000_NUM_SUPPLIES; i++)
@@ -868,7 +868,7 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
if (id != 0x2000) {
dev_err(>dev, "Device is not a WM2000 - ID %x\n", id);
ret = -ENODEV;
-   goto err_supplies;
+   goto disable_regulator;
}
 
reg = wm2000_read(i2c, WM2000_REG_REVISON);
@@ -878,7 +878,7 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
if (IS_ERR(wm2000->mclk)) {
ret = PTR_ERR(wm2000->mclk);
dev_err(>dev, "Failed to get MCLK: %d\n", ret);
-   goto err_supplies;
+   goto disable_regulator;
}
 
filename = "wm2000_anc.bin";
@@ -893,7 +893,7 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
ret = request_firmware(, filename, >dev);
if (ret != 0) {
dev_err(>dev, "Failed to acquire ANC data: %d\n", ret);
-   goto err_supplies;
+   goto disable_regulator;
}
 
/* Pre-cook the concatenation of the register address onto the image */
@@ -901,9 +901,9 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
wm2000->anc_download = devm_kzalloc(>dev,
wm2000->anc_download_size,
GFP_KERNEL);
-   if (wm2000->anc_download == NULL) {
+   if (!wm2000->anc_download) {
ret = -ENOMEM;
-   goto err_supplies;
+   goto release_firmware;
}
 
wm2000->anc_download[0] = 0x80;
@@ -918,12 +918,10 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
wm2000_reset(wm2000);
 
ret = snd_soc_register_codec(>dev, _codec_dev_wm2000, NULL, 0);
-
-err_supplies:
-   regulator_bulk_disable(WM2000_NUM_SUPPLIES, wm2000->supplies);
-
-out:
+release_firmware:
release_firmware(fw);
+disable_regulator:
+   regulator_bulk_disable(WM2000_NUM_SUPPLIES, wm2000->supplies);
return ret;
 }
 
-- 
2.15.0

[PATCH 2/4] ASoC: wm2000: One function call less in wm2000_i2c_probe() after error detection

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 24 Nov 2017 07:45:59 +0100

The release_firmware() function was called in a few cases by the
wm2000_i2c_probe() function during error handling even if
the passed variable contained a null pointer.

* Adjust jump targets according to the Linux coding style convention.

* Delete the label "out" and an initialisation for the variable "fw"
  at the beginning which became unnecessary with this refactoring.

Signed-off-by: Markus Elfring 
---
 sound/soc/codecs/wm2000.c | 22 ++
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/sound/soc/codecs/wm2000.c b/sound/soc/codecs/wm2000.c
index ce936deed7e3..0ed2a8992df4 100644
--- a/sound/soc/codecs/wm2000.c
+++ b/sound/soc/codecs/wm2000.c
@@ -821,7 +821,7 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
struct wm2000_priv *wm2000;
struct wm2000_platform_data *pdata;
const char *filename;
-   const struct firmware *fw = NULL;
+   const struct firmware *fw;
int ret, i;
int reg;
u16 id;
@@ -840,7 +840,7 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
ret = PTR_ERR(wm2000->regmap);
dev_err(>dev, "Failed to allocate register map: %d\n",
ret);
-   goto out;
+   return ret;
}
 
for (i = 0; i < WM2000_NUM_SUPPLIES; i++)
@@ -868,7 +868,7 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
if (id != 0x2000) {
dev_err(>dev, "Device is not a WM2000 - ID %x\n", id);
ret = -ENODEV;
-   goto err_supplies;
+   goto disable_regulator;
}
 
reg = wm2000_read(i2c, WM2000_REG_REVISON);
@@ -878,7 +878,7 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
if (IS_ERR(wm2000->mclk)) {
ret = PTR_ERR(wm2000->mclk);
dev_err(>dev, "Failed to get MCLK: %d\n", ret);
-   goto err_supplies;
+   goto disable_regulator;
}
 
filename = "wm2000_anc.bin";
@@ -893,7 +893,7 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
ret = request_firmware(, filename, >dev);
if (ret != 0) {
dev_err(>dev, "Failed to acquire ANC data: %d\n", ret);
-   goto err_supplies;
+   goto disable_regulator;
}
 
/* Pre-cook the concatenation of the register address onto the image */
@@ -901,9 +901,9 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
wm2000->anc_download = devm_kzalloc(>dev,
wm2000->anc_download_size,
GFP_KERNEL);
-   if (wm2000->anc_download == NULL) {
+   if (!wm2000->anc_download) {
ret = -ENOMEM;
-   goto err_supplies;
+   goto release_firmware;
}
 
wm2000->anc_download[0] = 0x80;
@@ -918,12 +918,10 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
wm2000_reset(wm2000);
 
ret = snd_soc_register_codec(>dev, _codec_dev_wm2000, NULL, 0);
-
-err_supplies:
-   regulator_bulk_disable(WM2000_NUM_SUPPLIES, wm2000->supplies);
-
-out:
+release_firmware:
release_firmware(fw);
+disable_regulator:
+   regulator_bulk_disable(WM2000_NUM_SUPPLIES, wm2000->supplies);
return ret;
 }
 
-- 
2.15.0

Re: [PATCH] schedule: use unlikely()

[Ingo Molnar]

* Mikulas Patocka  wrote:

> A small patch for schedule(), so that the code goes straght in the common
> case.
> 
> Signed-off-by: Mikulas Patocka 
> 
> ---
>  include/linux/blkdev.h |2 +-
>  kernel/sched/core.c|2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/include/linux/blkdev.h
> ===
> --- linux-2.6.orig/include/linux/blkdev.h
> +++ linux-2.6/include/linux/blkdev.h
> @@ -1308,7 +1308,7 @@ static inline bool blk_needs_flush_plug(
>  {
>   struct blk_plug *plug = tsk->plug;
>  
> - return plug &&
> + return unlikely(plug != NULL) &&
>   (!list_empty(>list) ||
>!list_empty(>mq_list) ||
>!list_empty(>cb_list));

That's an unrelated change.

> Index: linux-2.6/kernel/sched/core.c
> ===
> --- linux-2.6.orig/kernel/sched/core.c
> +++ linux-2.6/kernel/sched/core.c
> @@ -3405,7 +3405,7 @@ void __noreturn do_task_dead(void)
>  
>  static inline void sched_submit_work(struct task_struct *tsk)
>  {
> - if (!tsk->state || tsk_is_pi_blocked(tsk))
> + if (!tsk->state || unlikely(tsk_is_pi_blocked(tsk)))
>   return;
>   /*
>* If we are going to sleep and we have plugged IO queued,

Do these changes actually change the generated assembly code?

Thanks,

Ingo

Re: [PATCH] schedule: use unlikely()

[Ingo Molnar]

* Mikulas Patocka  wrote:

> A small patch for schedule(), so that the code goes straght in the common
> case.
> 
> Signed-off-by: Mikulas Patocka 
> 
> ---
>  include/linux/blkdev.h |2 +-
>  kernel/sched/core.c|2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> Index: linux-2.6/include/linux/blkdev.h
> ===
> --- linux-2.6.orig/include/linux/blkdev.h
> +++ linux-2.6/include/linux/blkdev.h
> @@ -1308,7 +1308,7 @@ static inline bool blk_needs_flush_plug(
>  {
>   struct blk_plug *plug = tsk->plug;
>  
> - return plug &&
> + return unlikely(plug != NULL) &&
>   (!list_empty(>list) ||
>!list_empty(>mq_list) ||
>!list_empty(>cb_list));

That's an unrelated change.

> Index: linux-2.6/kernel/sched/core.c
> ===
> --- linux-2.6.orig/kernel/sched/core.c
> +++ linux-2.6/kernel/sched/core.c
> @@ -3405,7 +3405,7 @@ void __noreturn do_task_dead(void)
>  
>  static inline void sched_submit_work(struct task_struct *tsk)
>  {
> - if (!tsk->state || tsk_is_pi_blocked(tsk))
> + if (!tsk->state || unlikely(tsk_is_pi_blocked(tsk)))
>   return;
>   /*
>* If we are going to sleep and we have plugged IO queued,

Do these changes actually change the generated assembly code?

Thanks,

Ingo

[PATCH 1/4] ASoC: wm2000: Delete an error message for a failed memory allocation in wm2000_i2c_probe()

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 23 Nov 2017 22:28:00 +0100

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 sound/soc/codecs/wm2000.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/codecs/wm2000.c b/sound/soc/codecs/wm2000.c
index 23cde3a0dc11..ce936deed7e3 100644
--- a/sound/soc/codecs/wm2000.c
+++ b/sound/soc/codecs/wm2000.c
@@ -902,7 +902,6 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
wm2000->anc_download_size,
GFP_KERNEL);
if (wm2000->anc_download == NULL) {
-   dev_err(>dev, "Out of memory\n");
ret = -ENOMEM;
goto err_supplies;
}
-- 
2.15.0

[PATCH 1/4] ASoC: wm2000: Delete an error message for a failed memory allocation in wm2000_i2c_probe()

[SF Markus Elfring]

From: Markus Elfring 
Date: Thu, 23 Nov 2017 22:28:00 +0100

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring 
---
 sound/soc/codecs/wm2000.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/sound/soc/codecs/wm2000.c b/sound/soc/codecs/wm2000.c
index 23cde3a0dc11..ce936deed7e3 100644
--- a/sound/soc/codecs/wm2000.c
+++ b/sound/soc/codecs/wm2000.c
@@ -902,7 +902,6 @@ static int wm2000_i2c_probe(struct i2c_client *i2c,
wm2000->anc_download_size,
GFP_KERNEL);
if (wm2000->anc_download == NULL) {
-   dev_err(>dev, "Out of memory\n");
ret = -ENOMEM;
goto err_supplies;
}
-- 
2.15.0

RE: [dm-devel] [PATCH 3/4] dm: convert dm_dev_internal.count from atomic_t to refcount_t

[Reshetova, Elena]

> On Fri, Oct 20, 2017 at 10:37:38AM +0300, Elena Reshetova wrote:
> > } else if (dd->dm_dev->mode != (mode | dd->dm_dev->mode)) {
> > r = upgrade_mode(dd, mode, t->md);
> > if (r)
> > return r;
> > +   refcount_inc(>count);
> > }
> 
> Missing here:
> 
> else
>   refcount_inc(>count);
> 
> ?

Oh, yes, thanks for catching this! I think this got unnoticed so far and patch 
was merged, so I am going to send a followup patch now. 

Best Regards,
Elena.

> 
> Alasdair

RE: [dm-devel] [PATCH 3/4] dm: convert dm_dev_internal.count from atomic_t to refcount_t

[Reshetova, Elena]

> On Fri, Oct 20, 2017 at 10:37:38AM +0300, Elena Reshetova wrote:
> > } else if (dd->dm_dev->mode != (mode | dd->dm_dev->mode)) {
> > r = upgrade_mode(dd, mode, t->md);
> > if (r)
> > return r;
> > +   refcount_inc(>count);
> > }
> 
> Missing here:
> 
> else
>   refcount_inc(>count);
> 
> ?

Oh, yes, thanks for catching this! I think this got unnoticed so far and patch 
was merged, so I am going to send a followup patch now. 

Best Regards,
Elena.

> 
> Alasdair

[PATCH 0/4] ASoC: wm2000: Adjustments for wm2000_i2c_probe()

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 24 Nov 2017 08:26:56 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (4):
  Delete an error message for a failed memory allocation
  One function call less in wm2000_i2c_probe() after error detection
  Fix a typo in a comment line
  Improve a size determination

 sound/soc/codecs/wm2000.c | 28 
 1 file changed, 12 insertions(+), 16 deletions(-)

-- 
2.15.0

[PATCH 0/4] ASoC: wm2000: Adjustments for wm2000_i2c_probe()

[SF Markus Elfring]

From: Markus Elfring 
Date: Fri, 24 Nov 2017 08:26:56 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (4):
  Delete an error message for a failed memory allocation
  One function call less in wm2000_i2c_probe() after error detection
  Fix a typo in a comment line
  Improve a size determination

 sound/soc/codecs/wm2000.c | 28 
 1 file changed, 12 insertions(+), 16 deletions(-)

-- 
2.15.0

Re: [PATCH 00/23] [v4] KAISER: unmap most of the kernel from userspace page tables

[Ingo Molnar]

* Dave Hansen  wrote:

> On 11/23/2017 10:35 PM, Ingo Molnar wrote:
> > So the pteval_t changes break the build on most non-x86 architectures 
> > (alpha, arm, 
> > arm64, etc.), because most of them don't have an asm/pgtable_types.h file.
> > 
> > pteval_t is an x86-ism.
> > 
> > So I left out the changes below.
> 
> There was a warning on the non-PAE 32-bit builds saying that there was a
> shift larger than the type.  I assumed this was because of a reference
> to _PAGE_NX, and thus we needed a change to pteval_t.
> 
> But, now that I think about it more, that doesn't make sense since
> _PAGE_NX should be #defined down to a 0 on those configs unless
> something is wrong.

If pte flags need to be passed around then the canonical way to do it is to 
pass 
around a pte_t, and use pte_val() on it and such.

But please investigate the warning.

One other detail: I see you fixed some of the commit titles to use standard x86 
tags - could you please also capitalize sentences? I.e.:

  - x86/mm/kaiser: allow flushing for future ASID switches
  + x86/mm/kaiser: Allow flushing for future ASID switches

Could you please also double-check whether the merges I did in the latest 
WIP.x86/mm branch are OK? Andy changed the entry stack code a bit under Kaiser, 
which created about 3 new conflicts.

The key resolutions that I did were:

.macro interrupt func
cld

testb   $3, CS-ORIG_RAX(%rsp)
jz  1f
SWAPGS
SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
callswitch_to_thread_stack
1:

Plus I also dropped the extra switch_to_thread_stack call done in:

  x86/mm/kaiser: Prepare assembly for entry/exit CR3 switching

Because Andy's latest preparatory patch does it now:

  x86/entry/64: Use a percpu trampoline stack for IDT entries

Thanks,

Ingo

Re: [PATCH 00/23] [v4] KAISER: unmap most of the kernel from userspace page tables

[Ingo Molnar]

* Dave Hansen  wrote:

> On 11/23/2017 10:35 PM, Ingo Molnar wrote:
> > So the pteval_t changes break the build on most non-x86 architectures 
> > (alpha, arm, 
> > arm64, etc.), because most of them don't have an asm/pgtable_types.h file.
> > 
> > pteval_t is an x86-ism.
> > 
> > So I left out the changes below.
> 
> There was a warning on the non-PAE 32-bit builds saying that there was a
> shift larger than the type.  I assumed this was because of a reference
> to _PAGE_NX, and thus we needed a change to pteval_t.
> 
> But, now that I think about it more, that doesn't make sense since
> _PAGE_NX should be #defined down to a 0 on those configs unless
> something is wrong.

If pte flags need to be passed around then the canonical way to do it is to 
pass 
around a pte_t, and use pte_val() on it and such.

But please investigate the warning.

One other detail: I see you fixed some of the commit titles to use standard x86 
tags - could you please also capitalize sentences? I.e.:

  - x86/mm/kaiser: allow flushing for future ASID switches
  + x86/mm/kaiser: Allow flushing for future ASID switches

Could you please also double-check whether the merges I did in the latest 
WIP.x86/mm branch are OK? Andy changed the entry stack code a bit under Kaiser, 
which created about 3 new conflicts.

The key resolutions that I did were:

.macro interrupt func
cld

testb   $3, CS-ORIG_RAX(%rsp)
jz  1f
SWAPGS
SWITCH_TO_KERNEL_CR3 scratch_reg=%rax
callswitch_to_thread_stack
1:

Plus I also dropped the extra switch_to_thread_stack call done in:

  x86/mm/kaiser: Prepare assembly for entry/exit CR3 switching

Because Andy's latest preparatory patch does it now:

  x86/entry/64: Use a percpu trampoline stack for IDT entries

Thanks,

Ingo

NULL pointer dereference in process_one_work

[baiyaowei]

Hi,tj and jiangshan,

I build a ceph storage pool to run some benchmarks with 3.10 kernel.
Occasionally, when the cpus' load is very high, some nodes crash with
message below.

[292273.612014] BUG: unable to handle kernel NULL pointer dereference at
0008
[292273.612057] IP: [] process_one_work+0x31/0x470
[292273.612087] PGD 0 
[292273.612099] Oops:  [#1] SMP 
[292273.612117] Modules linked in: rbd(OE) bcache(OE) ip_vs xfs
xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4
iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4
xt_conntrack nf_conntrack ipt_REJECT tun bridge stp llc ebtable_filter
ebtables ip6table_filter ip6_tables iptable_filter bonding
intel_powerclamp coretemp intel_rapl kvm_intel kvm crc32_pclmul
ghash_clmulni_intel aesni_intel lrw gf128mul glue_helper ablk_helper
cryptd mxm_wmi iTCO_wdt iTCO_vendor_support dcdbas ipmi_devintf pcspkr
ipmi_ssif mei_me sg lpc_ich mei sb_edac ipmi_si mfd_core edac_core
ipmi_msghandler shpchp wmi acpi_power_meter nfsd auth_rpcgss nfs_acl
lockd grace sunrpc ip_tables ext4 mbcache jbd2 sd_mod crc_t10dif
crct10dif_generic mgag200 syscopyarea sysfillrect sysimgblt i2c_algo_bit
drm_kms_helper
[292273.612495]  crct10dif_pclmul crct10dif_common ttm crc32c_intel drm
ahci nvme bnx2x libahci i2c_core libata mdio libcrc32c megaraid_sas ptp
pps_core dm_mirror dm_region_hash dm_log dm_mod
[292273.612580] CPU: 16 PID: 353223 Comm: kworker/16:2 Tainted: G
OE     3.10.0-327.el7.x86_64 #1
[292273.612620] Hardware name: Dell Inc. PowerEdge R730xd/0WCJNT, BIOS
2.4.3 01/17/2017
[292273.612655] task: 8801f55e6780 ti: 882a199b task.ti:
882a199b
[292273.612685] RIP: 0010:[]  []
process_one_work+0x31/0x470
[292273.612721] RSP: 0018:882a199b3e28  EFLAGS: 00010046
[292273.612743] RAX:  RBX: 88088b273028 RCX:
882a199b3fd8
[292273.612771] RDX:  RSI: 88088b273028 RDI:
88088b273000
[292273.612799] RBP: 882a199b3e60 R08:  R09:
0770
[292273.612827] R10: 8822a3bb1f80 R11: 8822a3bb1f80 R12:
88088b273000
[292273.612855] R13: 881fff313fc0 R14:  R15:
881fff313fc0
[292273.612883] FS:  () GS:881fff30()
knlGS:
[292273.612914] CS:  0010 DS:  ES:  CR0: 80050033
[292273.612937] CR2: 00b8 CR3: 0194a000 CR4:
003407e0
[292273.612965] DR0:  DR1:  DR2:

[292273.612994] DR3:  DR6: fffe0ff0 DR7:
0400
[292273.613021] Stack:
[292273.613031]  ff313fd8  881fff313fd8
000188088b273030
[292273.613069]  8801f55e6780 88088b273000 881fff313fc0
882a199b3ec0
[292273.613108]  8109e4cc 882a199b3fd8 882a199b3fd8
8801f55e6780
[292273.613146] Call Trace:
[292273.613160]  [] worker_thread+0x21c/0x400
[292273.613185]  [] ? rescuer_thread+0x400/0x400
[292273.613212]  [] kthread+0xcf/0xe0
[292273.613234]  [] ?
kthread_create_on_node+0x140/0x140
[292273.613263]  [] ret_from_fork+0x58/0x90
[292273.613287]  [] ?
kthread_create_on_node+0x140/0x140
[292273.614303] Code: 48 89 e5 41 57 41 56 45 31 f6 41 55 41 54 49 89 fc
53 48 89 f3 48 83 ec 10 48 8b 06 4c 8b 6f 48 48 89 c2 30 d2 a8 04 4c 0f
45 f2 <49> 8b 46 08 44 8b b8 00 01 00 00 41 c1 ef 05 44 89 f8 83 e0 01 
[292273.617971] RIP  [] process_one_work+0x31/0x470
[292273.620011]  RSP 
[292273.621940] CR2: 0008

Some crash messsages:

crash> sys
  KERNEL: /usr/lib/debug/lib/modules/3.10.0-327.el7.x86_64/vmlinux
DUMPFILE: vmcore  [PARTIAL DUMP]
CPUS: 32
DATE: Wed Oct 18 05:21:14 2017
  UPTIME: 3 days, 09:07:25
LOAD AVERAGE: 221.70, 222.22, 224.96
   TASKS: 3115
NODENAME: node121
 RELEASE: 3.10.0-327.el7.x86_64
 VERSION: #1 SMP Thu Nov 19 22:10:57 UTC 2015
 MACHINE: x86_64  (2099 Mhz)
  MEMORY: 255.9 GB
   PANIC: "BUG: unable to handle kernel NULL pointer dereference at
0008"
crash> bt
PID: 353223  TASK: 8801f55e6780  CPU: 16  COMMAND: "kworker/16:2"
 #0 [882a199b3af0] machine_kexec at 81051beb
 #1 [882a199b3b50] crash_kexec at 810f2542
 #2 [882a199b3c20] oops_end at 8163e1a8
 #3 [882a199b3c48] no_context at 8162e2b8
 #4 [882a199b3c98] __bad_area_nosemaphore at 8162e34e
 #5 [882a199b3ce0] bad_area_nosemaphore at 8162e4b8
 #6 [882a199b3cf0] __do_page_fault at 81640fce
 #7 [882a199b3d48] do_page_fault at 81641113
 #8 [882a199b3d70] page_fault at 8163d408
[exception RIP: process_one_work+49]
RIP: 8109d4b1  RSP: 882a199b3e28  RFLAGS: 00010046
RAX:   RBX: 88088b273028  RCX: 882a199b3fd8
RDX:   RSI: 88088b273028  RDI: 88088b273000
RBP: 882a199b3e60   R8:    R9: 0770

NULL pointer dereference in process_one_work

[baiyaowei]

Hi,tj and jiangshan,

I build a ceph storage pool to run some benchmarks with 3.10 kernel.
Occasionally, when the cpus' load is very high, some nodes crash with
message below.

[292273.612014] BUG: unable to handle kernel NULL pointer dereference at
0008
[292273.612057] IP: [] process_one_work+0x31/0x470
[292273.612087] PGD 0 
[292273.612099] Oops:  [#1] SMP 
[292273.612117] Modules linked in: rbd(OE) bcache(OE) ip_vs xfs
xt_CHECKSUM iptable_mangle ipt_MASQUERADE nf_nat_masquerade_ipv4
iptable_nat nf_nat_ipv4 nf_nat nf_conntrack_ipv4 nf_defrag_ipv4
xt_conntrack nf_conntrack ipt_REJECT tun bridge stp llc ebtable_filter
ebtables ip6table_filter ip6_tables iptable_filter bonding
intel_powerclamp coretemp intel_rapl kvm_intel kvm crc32_pclmul
ghash_clmulni_intel aesni_intel lrw gf128mul glue_helper ablk_helper
cryptd mxm_wmi iTCO_wdt iTCO_vendor_support dcdbas ipmi_devintf pcspkr
ipmi_ssif mei_me sg lpc_ich mei sb_edac ipmi_si mfd_core edac_core
ipmi_msghandler shpchp wmi acpi_power_meter nfsd auth_rpcgss nfs_acl
lockd grace sunrpc ip_tables ext4 mbcache jbd2 sd_mod crc_t10dif
crct10dif_generic mgag200 syscopyarea sysfillrect sysimgblt i2c_algo_bit
drm_kms_helper
[292273.612495]  crct10dif_pclmul crct10dif_common ttm crc32c_intel drm
ahci nvme bnx2x libahci i2c_core libata mdio libcrc32c megaraid_sas ptp
pps_core dm_mirror dm_region_hash dm_log dm_mod
[292273.612580] CPU: 16 PID: 353223 Comm: kworker/16:2 Tainted: G
OE     3.10.0-327.el7.x86_64 #1
[292273.612620] Hardware name: Dell Inc. PowerEdge R730xd/0WCJNT, BIOS
2.4.3 01/17/2017
[292273.612655] task: 8801f55e6780 ti: 882a199b task.ti:
882a199b
[292273.612685] RIP: 0010:[]  []
process_one_work+0x31/0x470
[292273.612721] RSP: 0018:882a199b3e28  EFLAGS: 00010046
[292273.612743] RAX:  RBX: 88088b273028 RCX:
882a199b3fd8
[292273.612771] RDX:  RSI: 88088b273028 RDI:
88088b273000
[292273.612799] RBP: 882a199b3e60 R08:  R09:
0770
[292273.612827] R10: 8822a3bb1f80 R11: 8822a3bb1f80 R12:
88088b273000
[292273.612855] R13: 881fff313fc0 R14:  R15:
881fff313fc0
[292273.612883] FS:  () GS:881fff30()
knlGS:
[292273.612914] CS:  0010 DS:  ES:  CR0: 80050033
[292273.612937] CR2: 00b8 CR3: 0194a000 CR4:
003407e0
[292273.612965] DR0:  DR1:  DR2:

[292273.612994] DR3:  DR6: fffe0ff0 DR7:
0400
[292273.613021] Stack:
[292273.613031]  ff313fd8  881fff313fd8
000188088b273030
[292273.613069]  8801f55e6780 88088b273000 881fff313fc0
882a199b3ec0
[292273.613108]  8109e4cc 882a199b3fd8 882a199b3fd8
8801f55e6780
[292273.613146] Call Trace:
[292273.613160]  [] worker_thread+0x21c/0x400
[292273.613185]  [] ? rescuer_thread+0x400/0x400
[292273.613212]  [] kthread+0xcf/0xe0
[292273.613234]  [] ?
kthread_create_on_node+0x140/0x140
[292273.613263]  [] ret_from_fork+0x58/0x90
[292273.613287]  [] ?
kthread_create_on_node+0x140/0x140
[292273.614303] Code: 48 89 e5 41 57 41 56 45 31 f6 41 55 41 54 49 89 fc
53 48 89 f3 48 83 ec 10 48 8b 06 4c 8b 6f 48 48 89 c2 30 d2 a8 04 4c 0f
45 f2 <49> 8b 46 08 44 8b b8 00 01 00 00 41 c1 ef 05 44 89 f8 83 e0 01 
[292273.617971] RIP  [] process_one_work+0x31/0x470
[292273.620011]  RSP 
[292273.621940] CR2: 0008

Some crash messsages:

crash> sys
  KERNEL: /usr/lib/debug/lib/modules/3.10.0-327.el7.x86_64/vmlinux
DUMPFILE: vmcore  [PARTIAL DUMP]
CPUS: 32
DATE: Wed Oct 18 05:21:14 2017
  UPTIME: 3 days, 09:07:25
LOAD AVERAGE: 221.70, 222.22, 224.96
   TASKS: 3115
NODENAME: node121
 RELEASE: 3.10.0-327.el7.x86_64
 VERSION: #1 SMP Thu Nov 19 22:10:57 UTC 2015
 MACHINE: x86_64  (2099 Mhz)
  MEMORY: 255.9 GB
   PANIC: "BUG: unable to handle kernel NULL pointer dereference at
0008"
crash> bt
PID: 353223  TASK: 8801f55e6780  CPU: 16  COMMAND: "kworker/16:2"
 #0 [882a199b3af0] machine_kexec at 81051beb
 #1 [882a199b3b50] crash_kexec at 810f2542
 #2 [882a199b3c20] oops_end at 8163e1a8
 #3 [882a199b3c48] no_context at 8162e2b8
 #4 [882a199b3c98] __bad_area_nosemaphore at 8162e34e
 #5 [882a199b3ce0] bad_area_nosemaphore at 8162e4b8
 #6 [882a199b3cf0] __do_page_fault at 81640fce
 #7 [882a199b3d48] do_page_fault at 81641113
 #8 [882a199b3d70] page_fault at 8163d408
[exception RIP: process_one_work+49]
RIP: 8109d4b1  RSP: 882a199b3e28  RFLAGS: 00010046
RAX:   RBX: 88088b273028  RCX: 882a199b3fd8
RDX:   RSI: 88088b273028  RDI: 88088b273000
RBP: 882a199b3e60   R8:    R9: 0770

[PATCH 1/2] powerpc/lib/code-patching: refactor patch_instruction()

[Christophe Leroy]

patch_instruction() uses almost the same sequence as
__patch_instruction()

This patch refactor it so that patch_instruction() uses
__patch_instruction() instead of duplicating code.

Signed-off-by: Christophe Leroy 
---
 arch/powerpc/lib/code-patching.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index d469224c4ada..80954c910b66 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -23,19 +23,26 @@
 #include 
 #include 
 
-static int __patch_instruction(unsigned int *addr, unsigned int instr)
+static int __patch_instruction(unsigned int *exec_addr, unsigned int instr,
+  unsigned int *patch_addr)
 {
int err;
 
-   __put_user_size(instr, addr, 4, err);
+   __put_user_size(instr, patch_addr, 4, err);
if (err)
return err;
 
-   asm ("dcbst 0, %0; sync; icbi 0,%0; sync; isync" :: "r" (addr));
+   asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr),
+   "r" (exec_addr));
 
return 0;
 }
 
+static int raw_patch_instruction(unsigned int *addr, unsigned int instr)
+{
+   return __patch_instruction(addr, instr, addr);
+}
+
 #ifdef CONFIG_STRICT_KERNEL_RWX
 static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
 
@@ -138,7 +145,7 @@ static inline int unmap_patch_area(unsigned long addr)
 int patch_instruction(unsigned int *addr, unsigned int instr)
 {
int err;
-   unsigned int *dest = NULL;
+   unsigned int *patch_addr = NULL;
unsigned long flags;
unsigned long text_poke_addr;
unsigned long kaddr = (unsigned long)addr;
@@ -149,7 +156,7 @@ int patch_instruction(unsigned int *addr, unsigned int 
instr)
 * to allow patching. We just do the plain old patching
 */
if (!this_cpu_read(*PTRRELOC(_poke_area)))
-   return __patch_instruction(addr, instr);
+   return raw_patch_instruction(addr, instr);
 
local_irq_save(flags);
 
@@ -159,17 +166,10 @@ int patch_instruction(unsigned int *addr, unsigned int 
instr)
goto out;
}
 
-   dest = (unsigned int *)(text_poke_addr) +
+   patch_addr = (unsigned int *)(text_poke_addr) +
((kaddr & ~PAGE_MASK) / sizeof(unsigned int));
 
-   /*
-* We use __put_user_size so that we can handle faults while
-* writing to dest and return err to handle faults gracefully
-*/
-   __put_user_size(instr, dest, 4, err);
-   if (!err)
-   asm ("dcbst 0, %0; sync; icbi 0,%0; icbi 0,%1; sync; isync"
-   ::"r" (dest), "r"(addr));
+   __patch_instruction(addr, instr, patch_addr);
 
err = unmap_patch_area(text_poke_addr);
if (err)
@@ -184,7 +184,7 @@ int patch_instruction(unsigned int *addr, unsigned int 
instr)
 
 int patch_instruction(unsigned int *addr, unsigned int instr)
 {
-   return __patch_instruction(addr, instr);
+   return raw_patch_instruction(addr, instr);
 }
 
 #endif /* CONFIG_STRICT_KERNEL_RWX */
-- 
2.13.3

[PATCH 1/2] powerpc/lib/code-patching: refactor patch_instruction()

[Christophe Leroy]

patch_instruction() uses almost the same sequence as
__patch_instruction()

This patch refactor it so that patch_instruction() uses
__patch_instruction() instead of duplicating code.

Signed-off-by: Christophe Leroy 
---
 arch/powerpc/lib/code-patching.c | 30 +++---
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index d469224c4ada..80954c910b66 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -23,19 +23,26 @@
 #include 
 #include 
 
-static int __patch_instruction(unsigned int *addr, unsigned int instr)
+static int __patch_instruction(unsigned int *exec_addr, unsigned int instr,
+  unsigned int *patch_addr)
 {
int err;
 
-   __put_user_size(instr, addr, 4, err);
+   __put_user_size(instr, patch_addr, 4, err);
if (err)
return err;
 
-   asm ("dcbst 0, %0; sync; icbi 0,%0; sync; isync" :: "r" (addr));
+   asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r" (patch_addr),
+   "r" (exec_addr));
 
return 0;
 }
 
+static int raw_patch_instruction(unsigned int *addr, unsigned int instr)
+{
+   return __patch_instruction(addr, instr, addr);
+}
+
 #ifdef CONFIG_STRICT_KERNEL_RWX
 static DEFINE_PER_CPU(struct vm_struct *, text_poke_area);
 
@@ -138,7 +145,7 @@ static inline int unmap_patch_area(unsigned long addr)
 int patch_instruction(unsigned int *addr, unsigned int instr)
 {
int err;
-   unsigned int *dest = NULL;
+   unsigned int *patch_addr = NULL;
unsigned long flags;
unsigned long text_poke_addr;
unsigned long kaddr = (unsigned long)addr;
@@ -149,7 +156,7 @@ int patch_instruction(unsigned int *addr, unsigned int 
instr)
 * to allow patching. We just do the plain old patching
 */
if (!this_cpu_read(*PTRRELOC(_poke_area)))
-   return __patch_instruction(addr, instr);
+   return raw_patch_instruction(addr, instr);
 
local_irq_save(flags);
 
@@ -159,17 +166,10 @@ int patch_instruction(unsigned int *addr, unsigned int 
instr)
goto out;
}
 
-   dest = (unsigned int *)(text_poke_addr) +
+   patch_addr = (unsigned int *)(text_poke_addr) +
((kaddr & ~PAGE_MASK) / sizeof(unsigned int));
 
-   /*
-* We use __put_user_size so that we can handle faults while
-* writing to dest and return err to handle faults gracefully
-*/
-   __put_user_size(instr, dest, 4, err);
-   if (!err)
-   asm ("dcbst 0, %0; sync; icbi 0,%0; icbi 0,%1; sync; isync"
-   ::"r" (dest), "r"(addr));
+   __patch_instruction(addr, instr, patch_addr);
 
err = unmap_patch_area(text_poke_addr);
if (err)
@@ -184,7 +184,7 @@ int patch_instruction(unsigned int *addr, unsigned int 
instr)
 
 int patch_instruction(unsigned int *addr, unsigned int instr)
 {
-   return __patch_instruction(addr, instr);
+   return raw_patch_instruction(addr, instr);
 }
 
 #endif /* CONFIG_STRICT_KERNEL_RWX */
-- 
2.13.3

[PATCH 2/2] powerpc/lib/feature-fixups: use raw_patch_instruction()

[Christophe Leroy]

feature fixups need to use patch_instruction() early in the boot,
even before the code is relocated to its final address, requiring
patch_instruction() to use PTRRELOC() in order to address data.

But feature fixups applies on code before it is set to read only,
even for modules. Therefore, feature fixups can use
raw_patch_instruction() instead.

Signed-off-by: Christophe Leroy 
---
 arch/powerpc/include/asm/code-patching.h | 1 +
 arch/powerpc/lib/code-patching.c | 4 ++--
 arch/powerpc/lib/feature-fixups.c| 8 
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/code-patching.h 
b/arch/powerpc/include/asm/code-patching.h
index abef812de7f8..1090024e8519 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -31,6 +31,7 @@ unsigned int create_cond_branch(const unsigned int *addr,
unsigned long target, int flags);
 int patch_branch(unsigned int *addr, unsigned long target, int flags);
 int patch_instruction(unsigned int *addr, unsigned int instr);
+int raw_patch_instruction(unsigned int *addr, unsigned int instr);
 
 int instr_is_relative_branch(unsigned int instr);
 int instr_is_branch_to_addr(const unsigned int *instr, unsigned long addr);
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 80954c910b66..b7c555df5cd6 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -38,7 +38,7 @@ static int __patch_instruction(unsigned int *exec_addr, 
unsigned int instr,
return 0;
 }
 
-static int raw_patch_instruction(unsigned int *addr, unsigned int instr)
+int raw_patch_instruction(unsigned int *addr, unsigned int instr)
 {
return __patch_instruction(addr, instr, addr);
 }
@@ -155,7 +155,7 @@ int patch_instruction(unsigned int *addr, unsigned int 
instr)
 * when text_poke_area is not ready, but we still need
 * to allow patching. We just do the plain old patching
 */
-   if (!this_cpu_read(*PTRRELOC(_poke_area)))
+   if (!this_cpu_read(text_poke_area))
return raw_patch_instruction(addr, instr);
 
local_irq_save(flags);
diff --git a/arch/powerpc/lib/feature-fixups.c 
b/arch/powerpc/lib/feature-fixups.c
index 41cf5ae273cf..0872d60ede10 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -62,7 +62,7 @@ static int patch_alt_instruction(unsigned int *src, unsigned 
int *dest,
}
}
 
-   patch_instruction(dest, instr);
+   raw_patch_instruction(dest, instr);
 
return 0;
 }
@@ -91,7 +91,7 @@ static int patch_feature_section(unsigned long value, struct 
fixup_entry *fcur)
}
 
for (; dest < end; dest++)
-   patch_instruction(dest, PPC_INST_NOP);
+   raw_patch_instruction(dest, PPC_INST_NOP);
 
return 0;
 }
@@ -129,7 +129,7 @@ void do_lwsync_fixups(unsigned long value, void 
*fixup_start, void *fixup_end)
 
for (; start < end; start++) {
dest = (void *)start + *start;
-   patch_instruction(dest, PPC_INST_LWSYNC);
+   raw_patch_instruction(dest, PPC_INST_LWSYNC);
}
 }
 
@@ -147,7 +147,7 @@ static void do_final_fixups(void)
length = (__end_interrupts - _stext) / sizeof(int);
 
while (length--) {
-   patch_instruction(dest, *src);
+   raw_patch_instruction(dest, *src);
src++;
dest++;
}
-- 
2.13.3

[PATCH 2/2] powerpc/lib/feature-fixups: use raw_patch_instruction()

[Christophe Leroy]

feature fixups need to use patch_instruction() early in the boot,
even before the code is relocated to its final address, requiring
patch_instruction() to use PTRRELOC() in order to address data.

But feature fixups applies on code before it is set to read only,
even for modules. Therefore, feature fixups can use
raw_patch_instruction() instead.

Signed-off-by: Christophe Leroy 
---
 arch/powerpc/include/asm/code-patching.h | 1 +
 arch/powerpc/lib/code-patching.c | 4 ++--
 arch/powerpc/lib/feature-fixups.c| 8 
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/powerpc/include/asm/code-patching.h 
b/arch/powerpc/include/asm/code-patching.h
index abef812de7f8..1090024e8519 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -31,6 +31,7 @@ unsigned int create_cond_branch(const unsigned int *addr,
unsigned long target, int flags);
 int patch_branch(unsigned int *addr, unsigned long target, int flags);
 int patch_instruction(unsigned int *addr, unsigned int instr);
+int raw_patch_instruction(unsigned int *addr, unsigned int instr);
 
 int instr_is_relative_branch(unsigned int instr);
 int instr_is_branch_to_addr(const unsigned int *instr, unsigned long addr);
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index 80954c910b66..b7c555df5cd6 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -38,7 +38,7 @@ static int __patch_instruction(unsigned int *exec_addr, 
unsigned int instr,
return 0;
 }
 
-static int raw_patch_instruction(unsigned int *addr, unsigned int instr)
+int raw_patch_instruction(unsigned int *addr, unsigned int instr)
 {
return __patch_instruction(addr, instr, addr);
 }
@@ -155,7 +155,7 @@ int patch_instruction(unsigned int *addr, unsigned int 
instr)
 * when text_poke_area is not ready, but we still need
 * to allow patching. We just do the plain old patching
 */
-   if (!this_cpu_read(*PTRRELOC(_poke_area)))
+   if (!this_cpu_read(text_poke_area))
return raw_patch_instruction(addr, instr);
 
local_irq_save(flags);
diff --git a/arch/powerpc/lib/feature-fixups.c 
b/arch/powerpc/lib/feature-fixups.c
index 41cf5ae273cf..0872d60ede10 100644
--- a/arch/powerpc/lib/feature-fixups.c
+++ b/arch/powerpc/lib/feature-fixups.c
@@ -62,7 +62,7 @@ static int patch_alt_instruction(unsigned int *src, unsigned 
int *dest,
}
}
 
-   patch_instruction(dest, instr);
+   raw_patch_instruction(dest, instr);
 
return 0;
 }
@@ -91,7 +91,7 @@ static int patch_feature_section(unsigned long value, struct 
fixup_entry *fcur)
}
 
for (; dest < end; dest++)
-   patch_instruction(dest, PPC_INST_NOP);
+   raw_patch_instruction(dest, PPC_INST_NOP);
 
return 0;
 }
@@ -129,7 +129,7 @@ void do_lwsync_fixups(unsigned long value, void 
*fixup_start, void *fixup_end)
 
for (; start < end; start++) {
dest = (void *)start + *start;
-   patch_instruction(dest, PPC_INST_LWSYNC);
+   raw_patch_instruction(dest, PPC_INST_LWSYNC);
}
 }
 
@@ -147,7 +147,7 @@ static void do_final_fixups(void)
length = (__end_interrupts - _stext) / sizeof(int);
 
while (length--) {
-   patch_instruction(dest, *src);
+   raw_patch_instruction(dest, *src);
src++;
dest++;
}
-- 
2.13.3

Re: [PATCH v2] gpio: davinci: Assign first bank regs for unbanked case

[Keerthy]

On Friday 10 November 2017 04:43 PM, Keerthy wrote:
> As per the re-design assign the first bank regs for unbanked
> irq case. This was missed out in the original patch.

Linus,

A gentle ping.

- Keerthy

> 
> Signed-off-by: Keerthy 
> Fixes: b5cf3fd827d2e1 ("gpio: davinci: Redesign driver to accommodate ngpios 
> in one gpio chip")
> ---
> 
> Changes in v2:
> 
>   * Fixed $Author
> 
>  drivers/gpio/gpio-davinci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
> index f75d844..e4b3d7d 100644
> --- a/drivers/gpio/gpio-davinci.c
> +++ b/drivers/gpio/gpio-davinci.c
> @@ -383,7 +383,7 @@ static int gpio_irq_type_unbanked(struct irq_data *data, 
> unsigned trigger)
>   u32 mask;
>  
>   d = (struct davinci_gpio_controller 
> *)irq_data_get_irq_handler_data(data);
> - g = (struct davinci_gpio_regs __iomem *)d->regs;
> + g = (struct davinci_gpio_regs __iomem *)d->regs[0];
>   mask = __gpio_mask(data->irq - d->base_irq);
>  
>   if (trigger & ~(IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
> 

Re: [PATCH v2] gpio: davinci: Assign first bank regs for unbanked case

[Keerthy]

On Friday 10 November 2017 04:43 PM, Keerthy wrote:
> As per the re-design assign the first bank regs for unbanked
> irq case. This was missed out in the original patch.

Linus,

A gentle ping.

- Keerthy

> 
> Signed-off-by: Keerthy 
> Fixes: b5cf3fd827d2e1 ("gpio: davinci: Redesign driver to accommodate ngpios 
> in one gpio chip")
> ---
> 
> Changes in v2:
> 
>   * Fixed $Author
> 
>  drivers/gpio/gpio-davinci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-davinci.c b/drivers/gpio/gpio-davinci.c
> index f75d844..e4b3d7d 100644
> --- a/drivers/gpio/gpio-davinci.c
> +++ b/drivers/gpio/gpio-davinci.c
> @@ -383,7 +383,7 @@ static int gpio_irq_type_unbanked(struct irq_data *data, 
> unsigned trigger)
>   u32 mask;
>  
>   d = (struct davinci_gpio_controller 
> *)irq_data_get_irq_handler_data(data);
> - g = (struct davinci_gpio_regs __iomem *)d->regs;
> + g = (struct davinci_gpio_regs __iomem *)d->regs[0];
>   mask = __gpio_mask(data->irq - d->base_irq);
>  
>   if (trigger & ~(IRQ_TYPE_EDGE_FALLING | IRQ_TYPE_EDGE_RISING))
> 

Re: [PATCH 2/2] scripts: leaking_addresses: help screen updates

[Kaiwan N Billimoria]

On Fri, Nov 24, 2017 at 11:29 AM, Tobin C. Harding  wrote:

> Neither of these patches applies to my tree. Are you editing the diff's
> by hand? I noticed the patches don't end with the version signature, like 
> this:
>
> 
> 2.7.4

I cloned your tree from here: https://github.com/tcharding/linux/tree/leaks
is that right?

One thing i can think of: i have to copy across the script to a
cloud-based 32-bit system, work on it there, copy it back to your tree
on my laptop manually, then i do the 'git diff -r' and basically
copy-paste that. Is this causing issues?

thanks, Kaiwan.

> thanks,
> Tobin.

Re: [PATCH 2/2] scripts: leaking_addresses: help screen updates

[Kaiwan N Billimoria]

On Fri, Nov 24, 2017 at 11:29 AM, Tobin C. Harding  wrote:

> Neither of these patches applies to my tree. Are you editing the diff's
> by hand? I noticed the patches don't end with the version signature, like 
> this:
>
> 
> 2.7.4

I cloned your tree from here: https://github.com/tcharding/linux/tree/leaks
is that right?

One thing i can think of: i have to copy across the script to a
cloud-based 32-bit system, work on it there, copy it back to your tree
on my laptop manually, then i do the 'git diff -r' and basically
copy-paste that. Is this causing issues?

thanks, Kaiwan.

> thanks,
> Tobin.

Re: [PATCH 1/1] Input: ims-pcu - fix typo in an error log

[Joe Perches]

On Fri, 2017-11-24 at 14:59 +0800, Zhen Lei wrote:
> Tiny typo fixed in an error log.
> 
> I found this when I backported the CVE-2017-16645 patch:
> ea04efee7635 ("Input: ims-psu - check if CDC union descriptor is sane")
> 
> Signed-off-by: Zhen Lei 
> ---
>  drivers/input/misc/ims-pcu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
[]
> @@ -1651,7 +1651,7 @@ static void ims_pcu_buffers_free(struct ims_pcu *pcu)
>   return union_desc;
> 
>   dev_err(>dev,
> - "Union descriptor to short (%d vs %zd\n)",
> + "Union descriptor too short (%d vs %zd\n)",

And this format is incorrect too.  It should be:

+   "Union descriptor too short (%d vs %zd)\n",

with the close parenthesis before the newline, not after.

Re: [PATCH 1/1] Input: ims-pcu - fix typo in an error log

[Joe Perches]

On Fri, 2017-11-24 at 14:59 +0800, Zhen Lei wrote:
> Tiny typo fixed in an error log.
> 
> I found this when I backported the CVE-2017-16645 patch:
> ea04efee7635 ("Input: ims-psu - check if CDC union descriptor is sane")
> 
> Signed-off-by: Zhen Lei 
> ---
>  drivers/input/misc/ims-pcu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
[]
> @@ -1651,7 +1651,7 @@ static void ims_pcu_buffers_free(struct ims_pcu *pcu)
>   return union_desc;
> 
>   dev_err(>dev,
> - "Union descriptor to short (%d vs %zd\n)",
> + "Union descriptor too short (%d vs %zd\n)",

And this format is incorrect too.  It should be:

+   "Union descriptor too short (%d vs %zd)\n",

with the close parenthesis before the newline, not after.

[PATCH 1/1] Input: ims-pcu - fix typo in an error log

[Zhen Lei]

Tiny typo fixed in an error log.

I found this when I backported the CVE-2017-16645 patch:
ea04efee7635 ("Input: ims-psu - check if CDC union descriptor is sane")

Signed-off-by: Zhen Lei 
---
 drivers/input/misc/ims-pcu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
index ae47312..253ae8e 100644
--- a/drivers/input/misc/ims-pcu.c
+++ b/drivers/input/misc/ims-pcu.c
@@ -1651,7 +1651,7 @@ static void ims_pcu_buffers_free(struct ims_pcu *pcu)
return union_desc;

dev_err(>dev,
-   "Union descriptor to short (%d vs %zd\n)",
+   "Union descriptor too short (%d vs %zd\n)",
union_desc->bLength, sizeof(*union_desc));
return NULL;
}
--
1.8.3

[PATCH 1/1] Input: ims-pcu - fix typo in an error log

[Zhen Lei]

Tiny typo fixed in an error log.

I found this when I backported the CVE-2017-16645 patch:
ea04efee7635 ("Input: ims-psu - check if CDC union descriptor is sane")

Signed-off-by: Zhen Lei 
---
 drivers/input/misc/ims-pcu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/input/misc/ims-pcu.c b/drivers/input/misc/ims-pcu.c
index ae47312..253ae8e 100644
--- a/drivers/input/misc/ims-pcu.c
+++ b/drivers/input/misc/ims-pcu.c
@@ -1651,7 +1651,7 @@ static void ims_pcu_buffers_free(struct ims_pcu *pcu)
return union_desc;

dev_err(>dev,
-   "Union descriptor to short (%d vs %zd\n)",
+   "Union descriptor too short (%d vs %zd\n)",
union_desc->bLength, sizeof(*union_desc));
return NULL;
}
--
1.8.3

[PATCH 2/6] clk: lpc32xx: pr_err() strings should end with newlines

[Arvind Yadav]

pr_err() messages should end with a new-line to avoid other messages
being concatenated.

Signed-off-by: Arvind Yadav 
---
 drivers/clk/nxp/clk-lpc32xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/nxp/clk-lpc32xx.c b/drivers/clk/nxp/clk-lpc32xx.c
index 7b359af..b669a5c 100644
--- a/drivers/clk/nxp/clk-lpc32xx.c
+++ b/drivers/clk/nxp/clk-lpc32xx.c
@@ -526,7 +526,7 @@ static unsigned long clk_pll_recalc_rate(struct clk_hw *hw,
!(pll_is_valid(parent_rate, 1, 100, 2000)
  && pll_is_valid(cco_rate, 1, 15600, 32000)
  && pll_is_valid(ref_rate, 1, 100, 2700)))
-   pr_err("%s: PLL clocks are not in valid ranges: %lu/%lu/%lu",
+   pr_err("%s: PLL clocks are not in valid ranges: %lu/%lu/%lu\n",
   clk_hw_get_name(hw),
   parent_rate, cco_rate, ref_rate);
 
@@ -1505,7 +1505,7 @@ static void __init lpc32xx_clk_init(struct device_node 
*np)
return;
}
if (clk_get_rate(clk_32k) != 32768) {
-   pr_err("invalid clock rate of external 32KHz oscillator");
+   pr_err("invalid clock rate of external 32KHz oscillator\n");
return;
}
 
-- 
1.9.1

[PATCH 2/6] clk: lpc32xx: pr_err() strings should end with newlines

[Arvind Yadav]

pr_err() messages should end with a new-line to avoid other messages
being concatenated.

Signed-off-by: Arvind Yadav 
---
 drivers/clk/nxp/clk-lpc32xx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/nxp/clk-lpc32xx.c b/drivers/clk/nxp/clk-lpc32xx.c
index 7b359af..b669a5c 100644
--- a/drivers/clk/nxp/clk-lpc32xx.c
+++ b/drivers/clk/nxp/clk-lpc32xx.c
@@ -526,7 +526,7 @@ static unsigned long clk_pll_recalc_rate(struct clk_hw *hw,
!(pll_is_valid(parent_rate, 1, 100, 2000)
  && pll_is_valid(cco_rate, 1, 15600, 32000)
  && pll_is_valid(ref_rate, 1, 100, 2700)))
-   pr_err("%s: PLL clocks are not in valid ranges: %lu/%lu/%lu",
+   pr_err("%s: PLL clocks are not in valid ranges: %lu/%lu/%lu\n",
   clk_hw_get_name(hw),
   parent_rate, cco_rate, ref_rate);
 
@@ -1505,7 +1505,7 @@ static void __init lpc32xx_clk_init(struct device_node 
*np)
return;
}
if (clk_get_rate(clk_32k) != 32768) {
-   pr_err("invalid clock rate of external 32KHz oscillator");
+   pr_err("invalid clock rate of external 32KHz oscillator\n");
return;
}
 
-- 
1.9.1

[PATCH 6/6] clk: h8300: pr_err() strings should end with newlines

[Arvind Yadav]

pr_err() messages should end with a new-line to avoid other messages
being concatenated.

Signed-off-by: Arvind Yadav 
---
 drivers/clk/h8300/clk-div.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/h8300/clk-div.c b/drivers/clk/h8300/clk-div.c
index 4ae6244..d413ade 100644
--- a/drivers/clk/h8300/clk-div.c
+++ b/drivers/clk/h8300/clk-div.c
@@ -24,13 +24,13 @@ static void __init h8300_div_clk_setup(struct device_node 
*node)
 
num_parents = of_clk_get_parent_count(node);
if (!num_parents) {
-   pr_err("%s: no parent found", clk_name);
+   pr_err("%s: no parent found\n", clk_name);
return;
}
 
divcr = of_iomap(node, 0);
if (divcr == NULL) {
-   pr_err("%s: failed to map divide register", clk_name);
+   pr_err("%s: failed to map divide register\n", clk_name);
goto error;
}
offset = (unsigned long)divcr & 3;
-- 
1.9.1

Re: [PATCH] arm64: dts: Hi3660: Fix state id for 'CPU_NAP' state

[Leo Yan]

Hi Sudeep,

On Thu, Nov 23, 2017 at 02:03:51PM +, Sudeep Holla wrote:
> Hi Daniel,
> 
> Thanks a lot for pointing me to this and having some useful discussion
> in private. That helped to dig a bit further on this.
> 
> On 23/11/17 05:40, Leo Yan wrote:
> > Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
> > idle state. From ftrace log we can observe CA73 CPUs can be easily waken
> > up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle anything
> > and sleep again; so there have tons of trace events for CA73 CPUs
> > entering and exiting idle state.
> > 
> > On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
> > set its psci parameter as '0x001' and from this parameter it can
> > calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
> > takes 1 as a invalid value for state id, so the CPU cannot enter idle
> > state and directly bail out to kernel.
> > 
> > This commit changes psci parameter to '0x' for state id = 0;
> > this id is accepted by ARM trusted firmware and finally CPU can stay
> > properly in 'CPU_NAP' state.
> > 
> 
> I would like to conditionally NACK this patch. If we can't update the
> ARM TF at all then, I will agree with this change reluctantly.

Thanks for reviewing. Just like Daniel said, we need to figure out the
right method for this. So suggestions are very welcome!

> This looks like an artifact of copy paste in ARM TF port for this
> platform. If you look as PSCI specification, CPU suspend parameter has
> some recommendations and it's good to follow then unless you have strong
> reasons not to.
> 
> As Daniel pointed to me, this patch is required to satisfy TF
> particularly [1]. Now that looks like copy pasted from Juno or FVP port
> and if you look deeper, it's clearly under !ARM_RECOM_STATE_ID_ENC [2]
> which was not copied IIUC :).

Thanks for sharing pointers. It's shame that the copying is not
correct for Hikey960 :)

Come back to recommended state id, I reviewed Juno board defintion and
I found it's not align with PSCI spec defintion, in ARM-TF Juno code
defines state as below [1]:

#define ARM_LOCAL_STATE_RUN 0
#define ARM_LOCAL_STATE_RET 1
#define ARM_LOCAL_STATE_OFF 2

In PSCI spec chapter "6.5 Recommended StateID Encoding" recommends power
state id as below:

0: Run
1: Standby
2: Retention
3: Powerdown

So could you confirm on Hikey960 we should follow PSCI definition for
state id definition?

> Juno's implementation is legacy as these recommendations were added
> later in the specification while Juno is 3 year old platform now.
> 
> Though strictly speaking it's not violation of the PSCI specification,
> but I would rather get this fixed not before it's too late and copied to
> the next generation of platforms. Since the firmware can be easily
> upgraded that shouldn't be that difficult.

If completely compliant with PSCI recommended state id, we need change
both for ARM-TF and kernel for this. In ARM-TF, I have sent PR [2].

For the kernel patch, we should change state id as below. Please let me
know if you have suggestion for this.

diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi 
b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index 12544c3..812437a 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -179,7 +179,7 @@
 
CPU_NAP: cpu-nap {
compatible = "arm,idle-state";
-   arm,psci-suspend-param = <0x001>;
+   arm,psci-suspend-param = <0x002>;
entry-latency-us = <7>;
exit-latency-us = <2>;
min-residency-us = <15>;
@@ -188,7 +188,7 @@
CPU_SLEEP: cpu-sleep {
compatible = "arm,idle-state";
local-timer-stop;
-   arm,psci-suspend-param = <0x001>;
+   arm,psci-suspend-param = <0x0010003>;
entry-latency-us = <40>;
exit-latency-us = <70>;
min-residency-us = <3000>;
@@ -197,7 +197,7 @@
CLUSTER_SLEEP_0: cluster-sleep-0 {
compatible = "arm,idle-state";
local-timer-stop;
-   arm,psci-suspend-param = <0x101>;
+   arm,psci-suspend-param = <0x1010033>;
entry-latency-us = <500>;
exit-latency-us = <5000>;
min-residency-us = <2>;
@@ -206,7 +206,7 @@
CLUSTER_SLEEP_1: cluster-sleep-1 {
compatible = "arm,idle-state";
local-timer-stop;
-

[PATCH 1/6] clk: stm32f4: pr_err() strings should end with newlines

[Arvind Yadav]

pr_err() messages should end with a new-line to avoid other messages
being concatenated.

Signed-off-by: Arvind Yadav 
---
 drivers/clk/clk-stm32f4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
index 96c6b6b..da44f8d 100644
--- a/drivers/clk/clk-stm32f4.c
+++ b/drivers/clk/clk-stm32f4.c
@@ -1424,7 +1424,7 @@ static void __init stm32f4_rcc_init(struct device_node 
*np)
 
base = of_iomap(np, 0);
if (!base) {
-   pr_err("%s: unable to map resource", np->name);
+   pr_err("%s: unable to map resource\n", np->name);
return;
}
 
-- 
1.9.1

[PATCH 6/6] clk: h8300: pr_err() strings should end with newlines

[Arvind Yadav]

pr_err() messages should end with a new-line to avoid other messages
being concatenated.

Signed-off-by: Arvind Yadav 
---
 drivers/clk/h8300/clk-div.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/h8300/clk-div.c b/drivers/clk/h8300/clk-div.c
index 4ae6244..d413ade 100644
--- a/drivers/clk/h8300/clk-div.c
+++ b/drivers/clk/h8300/clk-div.c
@@ -24,13 +24,13 @@ static void __init h8300_div_clk_setup(struct device_node 
*node)
 
num_parents = of_clk_get_parent_count(node);
if (!num_parents) {
-   pr_err("%s: no parent found", clk_name);
+   pr_err("%s: no parent found\n", clk_name);
return;
}
 
divcr = of_iomap(node, 0);
if (divcr == NULL) {
-   pr_err("%s: failed to map divide register", clk_name);
+   pr_err("%s: failed to map divide register\n", clk_name);
goto error;
}
offset = (unsigned long)divcr & 3;
-- 
1.9.1

Re: [PATCH] arm64: dts: Hi3660: Fix state id for 'CPU_NAP' state

[Leo Yan]

Hi Sudeep,

On Thu, Nov 23, 2017 at 02:03:51PM +, Sudeep Holla wrote:
> Hi Daniel,
> 
> Thanks a lot for pointing me to this and having some useful discussion
> in private. That helped to dig a bit further on this.
> 
> On 23/11/17 05:40, Leo Yan wrote:
> > Thanks a lot for Vincent Guittot careful work to find bug for 'CPU_NAP'
> > idle state. From ftrace log we can observe CA73 CPUs can be easily waken
> > up from 'CPU_NAP' state but the 'waken up' CPUs doesn't handle anything
> > and sleep again; so there have tons of trace events for CA73 CPUs
> > entering and exiting idle state.
> > 
> > On Hi3660 CA73 has retention state 'CPU_NAP' for CPU idle, this state we
> > set its psci parameter as '0x001' and from this parameter it can
> > calculate state id is 1. Unfortunately ARM trusted firmware (ARM-TF)
> > takes 1 as a invalid value for state id, so the CPU cannot enter idle
> > state and directly bail out to kernel.
> > 
> > This commit changes psci parameter to '0x' for state id = 0;
> > this id is accepted by ARM trusted firmware and finally CPU can stay
> > properly in 'CPU_NAP' state.
> > 
> 
> I would like to conditionally NACK this patch. If we can't update the
> ARM TF at all then, I will agree with this change reluctantly.

Thanks for reviewing. Just like Daniel said, we need to figure out the
right method for this. So suggestions are very welcome!

> This looks like an artifact of copy paste in ARM TF port for this
> platform. If you look as PSCI specification, CPU suspend parameter has
> some recommendations and it's good to follow then unless you have strong
> reasons not to.
> 
> As Daniel pointed to me, this patch is required to satisfy TF
> particularly [1]. Now that looks like copy pasted from Juno or FVP port
> and if you look deeper, it's clearly under !ARM_RECOM_STATE_ID_ENC [2]
> which was not copied IIUC :).

Thanks for sharing pointers. It's shame that the copying is not
correct for Hikey960 :)

Come back to recommended state id, I reviewed Juno board defintion and
I found it's not align with PSCI spec defintion, in ARM-TF Juno code
defines state as below [1]:

#define ARM_LOCAL_STATE_RUN 0
#define ARM_LOCAL_STATE_RET 1
#define ARM_LOCAL_STATE_OFF 2

In PSCI spec chapter "6.5 Recommended StateID Encoding" recommends power
state id as below:

0: Run
1: Standby
2: Retention
3: Powerdown

So could you confirm on Hikey960 we should follow PSCI definition for
state id definition?

> Juno's implementation is legacy as these recommendations were added
> later in the specification while Juno is 3 year old platform now.
> 
> Though strictly speaking it's not violation of the PSCI specification,
> but I would rather get this fixed not before it's too late and copied to
> the next generation of platforms. Since the firmware can be easily
> upgraded that shouldn't be that difficult.

If completely compliant with PSCI recommended state id, we need change
both for ARM-TF and kernel for this. In ARM-TF, I have sent PR [2].

For the kernel patch, we should change state id as below. Please let me
know if you have suggestion for this.

diff --git a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi 
b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
index 12544c3..812437a 100644
--- a/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
+++ b/arch/arm64/boot/dts/hisilicon/hi3660.dtsi
@@ -179,7 +179,7 @@
 
CPU_NAP: cpu-nap {
compatible = "arm,idle-state";
-   arm,psci-suspend-param = <0x001>;
+   arm,psci-suspend-param = <0x002>;
entry-latency-us = <7>;
exit-latency-us = <2>;
min-residency-us = <15>;
@@ -188,7 +188,7 @@
CPU_SLEEP: cpu-sleep {
compatible = "arm,idle-state";
local-timer-stop;
-   arm,psci-suspend-param = <0x001>;
+   arm,psci-suspend-param = <0x0010003>;
entry-latency-us = <40>;
exit-latency-us = <70>;
min-residency-us = <3000>;
@@ -197,7 +197,7 @@
CLUSTER_SLEEP_0: cluster-sleep-0 {
compatible = "arm,idle-state";
local-timer-stop;
-   arm,psci-suspend-param = <0x101>;
+   arm,psci-suspend-param = <0x1010033>;
entry-latency-us = <500>;
exit-latency-us = <5000>;
min-residency-us = <2>;
@@ -206,7 +206,7 @@
CLUSTER_SLEEP_1: cluster-sleep-1 {
compatible = "arm,idle-state";
local-timer-stop;
-

[PATCH 1/6] clk: stm32f4: pr_err() strings should end with newlines

[Arvind Yadav]

pr_err() messages should end with a new-line to avoid other messages
being concatenated.

Signed-off-by: Arvind Yadav 
---
 drivers/clk/clk-stm32f4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/clk-stm32f4.c b/drivers/clk/clk-stm32f4.c
index 96c6b6b..da44f8d 100644
--- a/drivers/clk/clk-stm32f4.c
+++ b/drivers/clk/clk-stm32f4.c
@@ -1424,7 +1424,7 @@ static void __init stm32f4_rcc_init(struct device_node 
*np)
 
base = of_iomap(np, 0);
if (!base) {
-   pr_err("%s: unable to map resource", np->name);
+   pr_err("%s: unable to map resource\n", np->name);
return;
}
 
-- 
1.9.1

[PATCH 3/6] clk: SPEAr: pr_err() strings should end with newlines

[Arvind Yadav]

pr_err() messages should end with a new-line to avoid other messages
being concatenated.

Signed-off-by: Arvind Yadav 
---
 drivers/clk/spear/clk-frac-synth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/spear/clk-frac-synth.c 
b/drivers/clk/spear/clk-frac-synth.c
index 58d678b..cbdf43a 100644
--- a/drivers/clk/spear/clk-frac-synth.c
+++ b/drivers/clk/spear/clk-frac-synth.c
@@ -131,7 +131,7 @@ struct clk *clk_register_frac(const char *name, const char 
*parent_name,
struct clk *clk;
 
if (!name || !parent_name || !reg || !rtbl || !rtbl_cnt) {
-   pr_err("Invalid arguments passed");
+   pr_err("Invalid arguments passed\n");
return ERR_PTR(-EINVAL);
}
 
-- 
1.9.1

[PATCH 5/6] clk: h8s2678: pr_err() strings should end with newlines

[Arvind Yadav]

pr_err() messages should end with a new-line to avoid other messages
being concatenated.

Signed-off-by: Arvind Yadav 
---
 drivers/clk/h8300/clk-h8s2678.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/h8300/clk-h8s2678.c b/drivers/clk/h8300/clk-h8s2678.c
index fc24b0b..b68045d 100644
--- a/drivers/clk/h8300/clk-h8s2678.c
+++ b/drivers/clk/h8300/clk-h8s2678.c
@@ -93,7 +93,7 @@ static void __init h8s2678_pll_clk_setup(struct device_node 
*node)
 
num_parents = of_clk_get_parent_count(node);
if (!num_parents) {
-   pr_err("%s: no parent found", clk_name);
+   pr_err("%s: no parent found\n", clk_name);
return;
}
 
@@ -104,13 +104,13 @@ static void __init h8s2678_pll_clk_setup(struct 
device_node *node)
 
pll_clock->sckcr = of_iomap(node, 0);
if (pll_clock->sckcr == NULL) {
-   pr_err("%s: failed to map divide register", clk_name);
+   pr_err("%s: failed to map divide register\n", clk_name);
goto free_clock;
}
 
pll_clock->pllcr = of_iomap(node, 1);
if (pll_clock->pllcr == NULL) {
-   pr_err("%s: failed to map multiply register", clk_name);
+   pr_err("%s: failed to map multiply register\n", clk_name);
goto unmap_sckcr;
}
 
-- 
1.9.1

[PATCH 3/6] clk: SPEAr: pr_err() strings should end with newlines

[Arvind Yadav]

pr_err() messages should end with a new-line to avoid other messages
being concatenated.

Signed-off-by: Arvind Yadav 
---
 drivers/clk/spear/clk-frac-synth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/spear/clk-frac-synth.c 
b/drivers/clk/spear/clk-frac-synth.c
index 58d678b..cbdf43a 100644
--- a/drivers/clk/spear/clk-frac-synth.c
+++ b/drivers/clk/spear/clk-frac-synth.c
@@ -131,7 +131,7 @@ struct clk *clk_register_frac(const char *name, const char 
*parent_name,
struct clk *clk;
 
if (!name || !parent_name || !reg || !rtbl || !rtbl_cnt) {
-   pr_err("Invalid arguments passed");
+   pr_err("Invalid arguments passed\n");
return ERR_PTR(-EINVAL);
}
 
-- 
1.9.1

[PATCH 5/6] clk: h8s2678: pr_err() strings should end with newlines

[Arvind Yadav]

pr_err() messages should end with a new-line to avoid other messages
being concatenated.

Signed-off-by: Arvind Yadav 
---
 drivers/clk/h8300/clk-h8s2678.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/clk/h8300/clk-h8s2678.c b/drivers/clk/h8300/clk-h8s2678.c
index fc24b0b..b68045d 100644
--- a/drivers/clk/h8300/clk-h8s2678.c
+++ b/drivers/clk/h8300/clk-h8s2678.c
@@ -93,7 +93,7 @@ static void __init h8s2678_pll_clk_setup(struct device_node 
*node)
 
num_parents = of_clk_get_parent_count(node);
if (!num_parents) {
-   pr_err("%s: no parent found", clk_name);
+   pr_err("%s: no parent found\n", clk_name);
return;
}
 
@@ -104,13 +104,13 @@ static void __init h8s2678_pll_clk_setup(struct 
device_node *node)
 
pll_clock->sckcr = of_iomap(node, 0);
if (pll_clock->sckcr == NULL) {
-   pr_err("%s: failed to map divide register", clk_name);
+   pr_err("%s: failed to map divide register\n", clk_name);
goto free_clock;
}
 
pll_clock->pllcr = of_iomap(node, 1);
if (pll_clock->pllcr == NULL) {
-   pr_err("%s: failed to map multiply register", clk_name);
+   pr_err("%s: failed to map multiply register\n", clk_name);
goto unmap_sckcr;
}
 
-- 
1.9.1

[PATCH 4/6] SPEAr: clk: pr_err() strings should end with newlines

[Arvind Yadav]

pr_err() messages should end with a new-line to avoid other messages
being concatenated.

Signed-off-by: Arvind Yadav 
---
 drivers/clk/spear/clk-gpt-synth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/spear/clk-gpt-synth.c 
b/drivers/clk/spear/clk-gpt-synth.c
index 1a722e9..1cf219a6 100644
--- a/drivers/clk/spear/clk-gpt-synth.c
+++ b/drivers/clk/spear/clk-gpt-synth.c
@@ -120,7 +120,7 @@ struct clk *clk_register_gpt(const char *name, const char 
*parent_name, unsigned
struct clk *clk;
 
if (!name || !parent_name || !reg || !rtbl || !rtbl_cnt) {
-   pr_err("Invalid arguments passed");
+   pr_err("Invalid arguments passed\n");
return ERR_PTR(-EINVAL);
}
 
-- 
1.9.1

[PATCH 4/6] SPEAr: clk: pr_err() strings should end with newlines

[Arvind Yadav]

pr_err() messages should end with a new-line to avoid other messages
being concatenated.

Signed-off-by: Arvind Yadav 
---
 drivers/clk/spear/clk-gpt-synth.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/spear/clk-gpt-synth.c 
b/drivers/clk/spear/clk-gpt-synth.c
index 1a722e9..1cf219a6 100644
--- a/drivers/clk/spear/clk-gpt-synth.c
+++ b/drivers/clk/spear/clk-gpt-synth.c
@@ -120,7 +120,7 @@ struct clk *clk_register_gpt(const char *name, const char 
*parent_name, unsigned
struct clk *clk;
 
if (!name || !parent_name || !reg || !rtbl || !rtbl_cnt) {
-   pr_err("Invalid arguments passed");
+   pr_err("Invalid arguments passed\n");
return ERR_PTR(-EINVAL);
}
 
-- 
1.9.1

[PATCH 0/6] clk: pr_err() strings should end with newlines

[Arvind Yadav]

pr_err() messages should end with a new-line to avoid other messages
being concatenated.

Arvind Yadav (6):
  [PATCH 1/6] clk: stm32f4: pr_err() strings should end with newlines
  [PATCH 2/6] clk: lpc32xx: pr_err() strings should end with newlines
  [PATCH 3/6] clk: SPEAr: pr_err() strings should end with newlines
  [PATCH 4/6] SPEAr: clk: pr_err() strings should end with newlines
  [PATCH 5/6] clk: h8s2678: pr_err() strings should end with newlines
  [PATCH 6/6] clk: h8300: pr_err() strings should end with newlines

 drivers/clk/clk-stm32f4.c  | 2 +-
 drivers/clk/h8300/clk-div.c| 4 ++--
 drivers/clk/h8300/clk-h8s2678.c| 6 +++---
 drivers/clk/nxp/clk-lpc32xx.c  | 4 ++--
 drivers/clk/spear/clk-frac-synth.c | 2 +-
 drivers/clk/spear/clk-gpt-synth.c  | 2 +-
 6 files changed, 10 insertions(+), 10 deletions(-)

-- 
1.9.1

[PATCH 0/6] clk: pr_err() strings should end with newlines

[Arvind Yadav]

pr_err() messages should end with a new-line to avoid other messages
being concatenated.

Arvind Yadav (6):
  [PATCH 1/6] clk: stm32f4: pr_err() strings should end with newlines
  [PATCH 2/6] clk: lpc32xx: pr_err() strings should end with newlines
  [PATCH 3/6] clk: SPEAr: pr_err() strings should end with newlines
  [PATCH 4/6] SPEAr: clk: pr_err() strings should end with newlines
  [PATCH 5/6] clk: h8s2678: pr_err() strings should end with newlines
  [PATCH 6/6] clk: h8300: pr_err() strings should end with newlines

 drivers/clk/clk-stm32f4.c  | 2 +-
 drivers/clk/h8300/clk-div.c| 4 ++--
 drivers/clk/h8300/clk-h8s2678.c| 6 +++---
 drivers/clk/nxp/clk-lpc32xx.c  | 4 ++--
 drivers/clk/spear/clk-frac-synth.c | 2 +-
 drivers/clk/spear/clk-gpt-synth.c  | 2 +-
 6 files changed, 10 insertions(+), 10 deletions(-)

-- 
1.9.1

$27M USD

[Sgt. Britta Lopez]

Apologies! I am a military woman ,seeking your kind assistance.

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

$27M USD

[Sgt. Britta Lopez]

Apologies! I am a military woman ,seeking your kind assistance.

---
This email has been checked for viruses by Avast antivirus software.
https://www.avast.com/antivirus

Re: [PATCH] fat: Fix sb_rdonly() change

[Joe Perches]

On Fri, 2017-11-24 at 15:07 +0900, OGAWA Hirofumi wrote:
> Joe Perches  writes:
> 
> > On Thu, 2017-11-23 at 15:29 +0900, OGAWA Hirofumi wrote:
> > > Ouch forgot to add stable@
> > > 
> > > -- 
> > > commit bc98a42c1f7d0f886c0c1b75a92a004976a46d9f introduced bug.
> > 
> > I think your commit message needs a bit more information.
> > 
> > It'd be useful to describe that the introduction of
> > sb_rdonly converted the bitwise & to a boolean and so
> > this conversion and comparison was made defective.
> > 
> > Are there any other instances of defective comparisons?
> 
> Please ask to that patch author.

The patch author, David Howells, is on the cc list.

btw:

It seems all the the other uses use a (bool) cast of the
(*flags & MS_RDONLY) and a comparison of sb_rdonly(sb).

It would make sense to change the argument type of the
ext[24]_setup_super int read_only arg to bool to match
the sb_rdonly() type. 

Re: [PATCH] fat: Fix sb_rdonly() change

[Joe Perches]

On Fri, 2017-11-24 at 15:07 +0900, OGAWA Hirofumi wrote:
> Joe Perches  writes:
> 
> > On Thu, 2017-11-23 at 15:29 +0900, OGAWA Hirofumi wrote:
> > > Ouch forgot to add stable@
> > > 
> > > -- 
> > > commit bc98a42c1f7d0f886c0c1b75a92a004976a46d9f introduced bug.
> > 
> > I think your commit message needs a bit more information.
> > 
> > It'd be useful to describe that the introduction of
> > sb_rdonly converted the bitwise & to a boolean and so
> > this conversion and comparison was made defective.
> > 
> > Are there any other instances of defective comparisons?
> 
> Please ask to that patch author.

The patch author, David Howells, is on the cc list.

btw:

It seems all the the other uses use a (bool) cast of the
(*flags & MS_RDONLY) and a comparison of sb_rdonly(sb).

It would make sense to change the argument type of the
ext[24]_setup_super int read_only arg to bool to match
the sb_rdonly() type. 

Re: Review of "[PATCH v2 0/3] virt: Add vboxguest driver for Virtual Box Guest integration"

[Greg Kroah-Hartman]

On Thu, Nov 23, 2017 at 07:37:48PM +0100, Hans de Goede wrote:
> Hi Arnd, Greg,
> 
> It seems that since there are no obvious glaring issues
> with v2 of my vboxguest driver series it is now stuck
> waiting for review.

It's also the merge window and I can't do anything then...

> Larry Finger (in the Cc) is willing to review this series,
> would Larry's Reviewed-by (once he is happy with the
> series) be enough to get this merged under drivers/misc
> (or drivers/virt) ?

It can't hurt, the fact that no one seems to want to review it,
including the original developers of the code, does not seem good, don't
make it all up to me please.

thanks,

greg k-h

Re: Review of "[PATCH v2 0/3] virt: Add vboxguest driver for Virtual Box Guest integration"

[Greg Kroah-Hartman]

On Thu, Nov 23, 2017 at 07:37:48PM +0100, Hans de Goede wrote:
> Hi Arnd, Greg,
> 
> It seems that since there are no obvious glaring issues
> with v2 of my vboxguest driver series it is now stuck
> waiting for review.

It's also the merge window and I can't do anything then...

> Larry Finger (in the Cc) is willing to review this series,
> would Larry's Reviewed-by (once he is happy with the
> series) be enough to get this merged under drivers/misc
> (or drivers/virt) ?

It can't hurt, the fact that no one seems to want to review it,
including the original developers of the code, does not seem good, don't
make it all up to me please.

thanks,

greg k-h

Re: [PATCH 00/23] [v4] KAISER: unmap most of the kernel from userspace page tables

[Dave Hansen]

On 11/23/2017 10:35 PM, Ingo Molnar wrote:
> So the pteval_t changes break the build on most non-x86 architectures (alpha, 
> arm, 
> arm64, etc.), because most of them don't have an asm/pgtable_types.h file.
> 
> pteval_t is an x86-ism.
> 
> So I left out the changes below.

There was a warning on the non-PAE 32-bit builds saying that there was a
shift larger than the type.  I assumed this was because of a reference
to _PAGE_NX, and thus we needed a change to pteval_t.

But, now that I think about it more, that doesn't make sense since
_PAGE_NX should be #defined down to a 0 on those configs unless
something is wrong.

Re: [PATCH 00/23] [v4] KAISER: unmap most of the kernel from userspace page tables

[Dave Hansen]

On 11/23/2017 10:35 PM, Ingo Molnar wrote:
> So the pteval_t changes break the build on most non-x86 architectures (alpha, 
> arm, 
> arm64, etc.), because most of them don't have an asm/pgtable_types.h file.
> 
> pteval_t is an x86-ism.
> 
> So I left out the changes below.

There was a warning on the non-PAE 32-bit builds saying that there was a
shift larger than the type.  I assumed this was because of a reference
to _PAGE_NX, and thus we needed a change to pteval_t.

But, now that I think about it more, that doesn't make sense since
_PAGE_NX should be #defined down to a 0 on those configs unless
something is wrong.

Re: [PATCH 00/23] [v4] KAISER: unmap most of the kernel from userspace page tables

[Ingo Molnar]

* Dave Hansen <dave.han...@linux.intel.com> wrote:

> I've updated these a bit since yesterday with some minor fixes:
>  * Fixed KASLR compile bug
>  * Fixed ds.c compile problem
>  * Changed ulong to pteval_t to fix 32-bit compile problem
>  * Stop mapping cpu_current_top_of_stack (never used until after CR3 switch)
> 
> Rather than re-spamming everyone, the resulting branch is here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/daveh/x86-kaiser.git/log/?h=kaiser-414-tipwip-20171123
> 
> If anyone wants to be re-spammed, just say the word.

So the pteval_t changes break the build on most non-x86 architectures (alpha, 
arm, 
arm64, etc.), because most of them don't have an asm/pgtable_types.h file.

pteval_t is an x86-ism.

So I left out the changes below.

Thanks,

Ingo

diff --git a/arch/x86/include/asm/kaiser.h b/arch/x86/include/asm/kaiser.h
index 35f12a8a7071..2198855f7de9 100644
--- a/arch/x86/include/asm/kaiser.h
+++ b/arch/x86/include/asm/kaiser.h
@@ -18,6 +18,8 @@
 #ifndef __ASSEMBLY__
 
 #ifdef CONFIG_KAISER
+#include 
+
 /**
  *  kaiser_add_mapping - map a kernel range into the user page tables
  *  @addr: the start address of the range
@@ -31,7 +33,7 @@
  *  table.
  */
 extern int kaiser_add_mapping(unsigned long addr, unsigned long size,
- unsigned long flags);
+ pteval_t flags);
 
 /**
  *  kaiser_add_mapping_cpu_entry - map the cpu entry area
diff --git a/arch/x86/mm/kaiser.c b/arch/x86/mm/kaiser.c
index 1eb27b410556..58cae2924724 100644
--- a/arch/x86/mm/kaiser.c
+++ b/arch/x86/mm/kaiser.c
@@ -431,7 +431,7 @@ void __init kaiser_init(void)
 }
 
 int kaiser_add_mapping(unsigned long addr, unsigned long size,
-  unsigned long flags)
+  pteval_t flags)
 {
return kaiser_add_user_map((const void *)addr, size, flags);
 }
diff --git a/include/linux/kaiser.h b/include/linux/kaiser.h
index 83d465599646..f662013515a1 100644
--- a/include/linux/kaiser.h
+++ b/include/linux/kaiser.h
@@ -4,7 +4,11 @@
 #ifdef CONFIG_KAISER
 #include 
 #else
+
 #ifndef __ASSEMBLY__
+
+#include 
+
 /*
  * These stubs are used whenever CONFIG_KAISER is off, which
  * includes architectures that support KAISER, but have it
@@ -20,7 +24,7 @@ static inline void kaiser_remove_mapping(unsigned long start, 
unsigned long size
 }
 
 static inline int kaiser_add_mapping(unsigned long addr, unsigned long size,
-unsigned long flags)
+pteval_t flags)
 {
return 0;
 }

Re: [PATCH 00/23] [v4] KAISER: unmap most of the kernel from userspace page tables

[Ingo Molnar]

* Dave Hansen  wrote:

> I've updated these a bit since yesterday with some minor fixes:
>  * Fixed KASLR compile bug
>  * Fixed ds.c compile problem
>  * Changed ulong to pteval_t to fix 32-bit compile problem
>  * Stop mapping cpu_current_top_of_stack (never used until after CR3 switch)
> 
> Rather than re-spamming everyone, the resulting branch is here:
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/daveh/x86-kaiser.git/log/?h=kaiser-414-tipwip-20171123
> 
> If anyone wants to be re-spammed, just say the word.

So the pteval_t changes break the build on most non-x86 architectures (alpha, 
arm, 
arm64, etc.), because most of them don't have an asm/pgtable_types.h file.

pteval_t is an x86-ism.

So I left out the changes below.

Thanks,

Ingo

diff --git a/arch/x86/include/asm/kaiser.h b/arch/x86/include/asm/kaiser.h
index 35f12a8a7071..2198855f7de9 100644
--- a/arch/x86/include/asm/kaiser.h
+++ b/arch/x86/include/asm/kaiser.h
@@ -18,6 +18,8 @@
 #ifndef __ASSEMBLY__
 
 #ifdef CONFIG_KAISER
+#include 
+
 /**
  *  kaiser_add_mapping - map a kernel range into the user page tables
  *  @addr: the start address of the range
@@ -31,7 +33,7 @@
  *  table.
  */
 extern int kaiser_add_mapping(unsigned long addr, unsigned long size,
- unsigned long flags);
+ pteval_t flags);
 
 /**
  *  kaiser_add_mapping_cpu_entry - map the cpu entry area
diff --git a/arch/x86/mm/kaiser.c b/arch/x86/mm/kaiser.c
index 1eb27b410556..58cae2924724 100644
--- a/arch/x86/mm/kaiser.c
+++ b/arch/x86/mm/kaiser.c
@@ -431,7 +431,7 @@ void __init kaiser_init(void)
 }
 
 int kaiser_add_mapping(unsigned long addr, unsigned long size,
-  unsigned long flags)
+  pteval_t flags)
 {
return kaiser_add_user_map((const void *)addr, size, flags);
 }
diff --git a/include/linux/kaiser.h b/include/linux/kaiser.h
index 83d465599646..f662013515a1 100644
--- a/include/linux/kaiser.h
+++ b/include/linux/kaiser.h
@@ -4,7 +4,11 @@
 #ifdef CONFIG_KAISER
 #include 
 #else
+
 #ifndef __ASSEMBLY__
+
+#include 
+
 /*
  * These stubs are used whenever CONFIG_KAISER is off, which
  * includes architectures that support KAISER, but have it
@@ -20,7 +24,7 @@ static inline void kaiser_remove_mapping(unsigned long start, 
unsigned long size
 }
 
 static inline int kaiser_add_mapping(unsigned long addr, unsigned long size,
-unsigned long flags)
+pteval_t flags)
 {
return 0;
 }

Re: [PATCH 1/6] perf: Add new type PERF_TYPE_PROBE

[Alexei Starovoitov]

On 11/23/17 2:02 AM, Peter Zijlstra wrote:

On Wed, Nov 15, 2017 at 09:23:33AM -0800, Song Liu wrote:


Note: We use type __u64 for pointer probe_desc instead of __aligned_u64.
The reason here is to avoid changing the size of struct perf_event_attr,
and breaking new-kernel-old-utility scenario. To avoid alignment problem
with the pointer, we will (in the following patches) copy probe_desc to
__aligned_u64 before using it as pointer.


ISTR there are only relatively few architectures where __u64 and
__aligned_u64 are not the same thing.

The comment that goes with it seems to suggest i386 has short alignment
for u64 but my compiler says differently:

printf("%d, %d\n", sizeof(unsigned long long), __alignof__(unsigned 
long long));

$ gcc -m32 -o align align.c && ./align
8, 8


unfortunately 32-bit is more screwed than it seems:

$ cat align.c
#include 

struct S {
  unsigned long long a;
} s;

struct U {
  unsigned long long a;
} u;

int main()
{
printf("%d, %d\n", sizeof(unsigned long long),
   __alignof__(unsigned long long));
printf("%d, %d\n", sizeof(s), __alignof__(s));
printf("%d, %d\n", sizeof(u), __alignof__(u));
}
$ gcc -m32 align.c
$ ./a.out
8, 8
8, 4
8, 4

so we have to use __aligned_u64 in uapi.

Otherwise, yes, we could have used config1 and config2 to pass pointers
to the kernel, but since they're defined as __u64 already we cannot
change them and have to do this ugly dance around 'config' field.
If you prefer we can do the same around 'config1', but it's not
any prettier.
We considered adding __aligned_u64 to the end of
'struct perf_event_attr', but it's a waste for most users, so reusing
the space of 'config' field like this seems the least evil.

Re: [PATCH 1/6] perf: Add new type PERF_TYPE_PROBE

[Alexei Starovoitov]

On 11/23/17 2:02 AM, Peter Zijlstra wrote:

On Wed, Nov 15, 2017 at 09:23:33AM -0800, Song Liu wrote:


Note: We use type __u64 for pointer probe_desc instead of __aligned_u64.
The reason here is to avoid changing the size of struct perf_event_attr,
and breaking new-kernel-old-utility scenario. To avoid alignment problem
with the pointer, we will (in the following patches) copy probe_desc to
__aligned_u64 before using it as pointer.


ISTR there are only relatively few architectures where __u64 and
__aligned_u64 are not the same thing.

The comment that goes with it seems to suggest i386 has short alignment
for u64 but my compiler says differently:

printf("%d, %d\n", sizeof(unsigned long long), __alignof__(unsigned 
long long));

$ gcc -m32 -o align align.c && ./align
8, 8


unfortunately 32-bit is more screwed than it seems:

$ cat align.c
#include 

struct S {
  unsigned long long a;
} s;

struct U {
  unsigned long long a;
} u;

int main()
{
printf("%d, %d\n", sizeof(unsigned long long),
   __alignof__(unsigned long long));
printf("%d, %d\n", sizeof(s), __alignof__(s));
printf("%d, %d\n", sizeof(u), __alignof__(u));
}
$ gcc -m32 align.c
$ ./a.out
8, 8
8, 4
8, 4

so we have to use __aligned_u64 in uapi.

Otherwise, yes, we could have used config1 and config2 to pass pointers
to the kernel, but since they're defined as __u64 already we cannot
change them and have to do this ugly dance around 'config' field.
If you prefer we can do the same around 'config1', but it's not
any prettier.
We considered adding __aligned_u64 to the end of
'struct perf_event_attr', but it's a waste for most users, so reusing
the space of 'config' field like this seems the least evil.

Re: [PATCH RT 03/10] random: avoid preempt_disable()ed section

[Alex Shi]

Hi Steve,

I just build the patches, a build error found here:

drivers/char/random.c: In function ‘get_random_int’:
drivers/char/random.c:1816:7: error: assignment from incompatible
pointer type [-Werror=incompatible-pointer-types]
  hash = _locked_var(hash_entropy_int_lock, get_random_int_hash);
   ^
drivers/char/random.c: In function ‘get_random_long’:
drivers/char/random.c:1838:7: error: assignment from incompatible
pointer type [-Werror=incompatible-pointer-types]
  hash = _locked_var(hash_entropy_int_lock, get_random_int_hash);
   ^

> - hash = get_cpu_var(get_random_int_hash);
> + hash = _locked_var(hash_entropy_int_lock, get_random_int_hash);
   ^
Is this a extra '&' which need to remove?

>  
>   hash[0] += current->pid + jiffies + random_get_entropy();
>   md5_transform(hash, random_int_secret);
>   ret = hash[0];
> - put_cpu_var(get_random_int_hash);
> + put_locked_var(hash_entropy_int_lock, get_random_int_hash);
>  
>   return ret;
>  }
> @@ -1833,12 +1835,12 @@ unsigned long get_random_long(void)
>   if (arch_get_random_long())
>   return ret;
>  
> - hash = get_cpu_var(get_random_int_hash);
> + hash = _locked_var(hash_entropy_int_lock, get_random_int_hash);
   ^
Ditto

Regards
Alex

Re: [PATCH RT 03/10] random: avoid preempt_disable()ed section

[Alex Shi]

Hi Steve,

I just build the patches, a build error found here:

drivers/char/random.c: In function ‘get_random_int’:
drivers/char/random.c:1816:7: error: assignment from incompatible
pointer type [-Werror=incompatible-pointer-types]
  hash = _locked_var(hash_entropy_int_lock, get_random_int_hash);
   ^
drivers/char/random.c: In function ‘get_random_long’:
drivers/char/random.c:1838:7: error: assignment from incompatible
pointer type [-Werror=incompatible-pointer-types]
  hash = _locked_var(hash_entropy_int_lock, get_random_int_hash);
   ^

> - hash = get_cpu_var(get_random_int_hash);
> + hash = _locked_var(hash_entropy_int_lock, get_random_int_hash);
   ^
Is this a extra '&' which need to remove?

>  
>   hash[0] += current->pid + jiffies + random_get_entropy();
>   md5_transform(hash, random_int_secret);
>   ret = hash[0];
> - put_cpu_var(get_random_int_hash);
> + put_locked_var(hash_entropy_int_lock, get_random_int_hash);
>  
>   return ret;
>  }
> @@ -1833,12 +1835,12 @@ unsigned long get_random_long(void)
>   if (arch_get_random_long())
>   return ret;
>  
> - hash = get_cpu_var(get_random_int_hash);
> + hash = _locked_var(hash_entropy_int_lock, get_random_int_hash);
   ^
Ditto

Regards
Alex

Re: [PATCH v2 00/11] Rockchip ISP1 Driver

[Jacob Chen]

HI all,

2017-11-24 10:36 GMT+08:00 Jacob Chen :
> This patch series add a ISP(Camera) v4l2 driver for rockchip rk3288/rk3399 
> SoC.
>
> Kernel Branch:
> https://github.com/wzyy2/linux/tree/rkisp1/drivers/media/platform/rockchip/isp1
>
> Below are some infomations about driver/hardware:
>
> Rockchip ISP1 have many Hardware Blocks(simplied):
>
>   MIPI  --> ISP --> DCrop(Mainpath) --> RSZ(Mainpath) --> DMA(Mainpath)
>   DMA-Input --> --> DCrop(Selfpath) --> RSZ(Selfpath) --> DMA(Selfpath);)
>
> (Acutally the TRM(rk3288, isp) could be found online.. which contains a 
> more detailed block diagrams ;-P)
>
> The funcitons of each hardware block:
>
>   Mainpath : up to 4k resolution, support raw/yuv format
>   Selfpath : up tp 1080p, support rotate, support rgb/yuv format
>   RSZ: scaling
>   DCrop: crop
>   ISP: 3A, Color processing, Crop
>   MIPI: MIPI Camera interface
>
> Media pipelines:
>
>   Mainpath, Selfpath <-- ISP subdev <-- MIPI  <-- Sensor
>   3A stats   <--<-- 3A parms
>
> Code struct:
>
>   capture.c : Mainpath, Selfpath, RSZ, DCROP : capture device.
>   rkisp1.c : ISP : v4l2 sub-device.
>   isp_params.c : 3A parms : output device.
>   isp_stats.c : 3A stats : capture device.
>   mipi_dphy_sy.c : MIPI : sperated v4l2 sub-device.
>
> Usage:
>   ChromiumOS:
> use below v4l2-ctl command to capture frames.
>
>   v4l2-ctl --verbose -d /dev/video4 --stream-mmap=2
>   --stream-to=/tmp/stream.out --stream-count=60 --stream-poll
>
> use below command to playback the video on your PC.
>
>   mplayer /tmp/stream.out -loop 0 --demuxer=rawvideo
>   --rawvideo=w=800:h=600:size=$((800*600*2)):format=yuy2
> or
>   mplayer ./stream.out -loop 0 -demuxer rawvideo -rawvideo
>   w=800:h=600:size=$((800*600*2)):format=yuy2
>
>   Linux:
> use rkcamsrc gstreamer plugin(just a modified v4l2src) to preview.
>
>   gst-launch-1.0 rkcamsrc device=/dev/video0 io-mode=4 disable-3A=true
>   videoconvert ! video/x-raw,format=NV12,width=640,height=480 ! kmssink
>
> Jacob Chen (7):
>   media: rkisp1: add rockchip isp1 driver
>   media: rkisp1: add Rockchip MIPI Synopsys DPHY driver
>   dt-bindings: Document the Rockchip ISP1 bindings
>   dt-bindings: Document the Rockchip MIPI RX D-PHY bindings
>   ARM: dts: rockchip: add isp node for rk3288
>   ARM: dts: rockchip: add rx0 mipi-phy for rk3288
>   MAINTAINERS: add entry for Rockchip ISP1 driver
>
> Jeffy Chen (1):
>   media: rkisp1: Add user space ABI definitions
>
> Shunqian Zheng (3):
>   media: videodev2.h, v4l2-ioctl: add rkisp1 meta buffer format
>   arm64: dts: rockchip: add isp0 node for rk3399
>   arm64: dts: rockchip: add rx0 mipi-phy for rk3399
>
>  .../devicetree/bindings/media/rockchip-isp1.txt|   61 +
>  .../bindings/media/rockchip-mipi-dphy.txt  |   77 +
>  MAINTAINERS|   10 +
>  arch/arm/boot/dts/rk3288.dtsi  |   24 +
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi   |   26 +
>  drivers/media/platform/Kconfig |   10 +
>  drivers/media/platform/Makefile|1 +
>  drivers/media/platform/rockchip/isp1/Makefile  |8 +
>  drivers/media/platform/rockchip/isp1/capture.c | 1691 
> 
>  drivers/media/platform/rockchip/isp1/capture.h |   46 +
>  drivers/media/platform/rockchip/isp1/common.h  |  330 
>  drivers/media/platform/rockchip/isp1/dev.c |  632 
>  drivers/media/platform/rockchip/isp1/isp_params.c  | 1556 ++
>  drivers/media/platform/rockchip/isp1/isp_params.h  |   81 +
>  drivers/media/platform/rockchip/isp1/isp_stats.c   |  537 +++
>  drivers/media/platform/rockchip/isp1/isp_stats.h   |   81 +
>  .../media/platform/rockchip/isp1/mipi_dphy_sy.c|  805 ++
>  drivers/media/platform/rockchip/isp1/regs.c|  251 +++
>  drivers/media/platform/rockchip/isp1/regs.h| 1578 ++
>  drivers/media/platform/rockchip/isp1/rkisp1.c  | 1230 ++
>  drivers/media/platform/rockchip/isp1/rkisp1.h  |  130 ++
>  drivers/media/v4l2-core/v4l2-ioctl.c   |2 +
>  include/uapi/linux/rkisp1-config.h |  554 +++
>  include/uapi/linux/videodev2.h |4 +
>  24 files changed, 9725 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/rockchip-isp1.txt
>  create mode 100644 
> Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt
>  create mode 100644 drivers/media/platform/rockchip/isp1/Makefile
>  create mode 100644 drivers/media/platform/rockchip/isp1/capture.c
>  create mode 100644 drivers/media/platform/rockchip/isp1/capture.h
>  create mode 100644 drivers/media/platform/rockchip/isp1/common.h
>  create mode 100644 drivers/media/platform/rockchip/isp1/dev.c
>  create mode 100644 drivers/media/platform/rockchip/isp1/isp_params.c
>  create mode 100644 

Re: [PATCH v2 00/11] Rockchip ISP1 Driver

[Jacob Chen]

HI all,

2017-11-24 10:36 GMT+08:00 Jacob Chen :
> This patch series add a ISP(Camera) v4l2 driver for rockchip rk3288/rk3399 
> SoC.
>
> Kernel Branch:
> https://github.com/wzyy2/linux/tree/rkisp1/drivers/media/platform/rockchip/isp1
>
> Below are some infomations about driver/hardware:
>
> Rockchip ISP1 have many Hardware Blocks(simplied):
>
>   MIPI  --> ISP --> DCrop(Mainpath) --> RSZ(Mainpath) --> DMA(Mainpath)
>   DMA-Input --> --> DCrop(Selfpath) --> RSZ(Selfpath) --> DMA(Selfpath);)
>
> (Acutally the TRM(rk3288, isp) could be found online.. which contains a 
> more detailed block diagrams ;-P)
>
> The funcitons of each hardware block:
>
>   Mainpath : up to 4k resolution, support raw/yuv format
>   Selfpath : up tp 1080p, support rotate, support rgb/yuv format
>   RSZ: scaling
>   DCrop: crop
>   ISP: 3A, Color processing, Crop
>   MIPI: MIPI Camera interface
>
> Media pipelines:
>
>   Mainpath, Selfpath <-- ISP subdev <-- MIPI  <-- Sensor
>   3A stats   <--<-- 3A parms
>
> Code struct:
>
>   capture.c : Mainpath, Selfpath, RSZ, DCROP : capture device.
>   rkisp1.c : ISP : v4l2 sub-device.
>   isp_params.c : 3A parms : output device.
>   isp_stats.c : 3A stats : capture device.
>   mipi_dphy_sy.c : MIPI : sperated v4l2 sub-device.
>
> Usage:
>   ChromiumOS:
> use below v4l2-ctl command to capture frames.
>
>   v4l2-ctl --verbose -d /dev/video4 --stream-mmap=2
>   --stream-to=/tmp/stream.out --stream-count=60 --stream-poll
>
> use below command to playback the video on your PC.
>
>   mplayer /tmp/stream.out -loop 0 --demuxer=rawvideo
>   --rawvideo=w=800:h=600:size=$((800*600*2)):format=yuy2
> or
>   mplayer ./stream.out -loop 0 -demuxer rawvideo -rawvideo
>   w=800:h=600:size=$((800*600*2)):format=yuy2
>
>   Linux:
> use rkcamsrc gstreamer plugin(just a modified v4l2src) to preview.
>
>   gst-launch-1.0 rkcamsrc device=/dev/video0 io-mode=4 disable-3A=true
>   videoconvert ! video/x-raw,format=NV12,width=640,height=480 ! kmssink
>
> Jacob Chen (7):
>   media: rkisp1: add rockchip isp1 driver
>   media: rkisp1: add Rockchip MIPI Synopsys DPHY driver
>   dt-bindings: Document the Rockchip ISP1 bindings
>   dt-bindings: Document the Rockchip MIPI RX D-PHY bindings
>   ARM: dts: rockchip: add isp node for rk3288
>   ARM: dts: rockchip: add rx0 mipi-phy for rk3288
>   MAINTAINERS: add entry for Rockchip ISP1 driver
>
> Jeffy Chen (1):
>   media: rkisp1: Add user space ABI definitions
>
> Shunqian Zheng (3):
>   media: videodev2.h, v4l2-ioctl: add rkisp1 meta buffer format
>   arm64: dts: rockchip: add isp0 node for rk3399
>   arm64: dts: rockchip: add rx0 mipi-phy for rk3399
>
>  .../devicetree/bindings/media/rockchip-isp1.txt|   61 +
>  .../bindings/media/rockchip-mipi-dphy.txt  |   77 +
>  MAINTAINERS|   10 +
>  arch/arm/boot/dts/rk3288.dtsi  |   24 +
>  arch/arm64/boot/dts/rockchip/rk3399.dtsi   |   26 +
>  drivers/media/platform/Kconfig |   10 +
>  drivers/media/platform/Makefile|1 +
>  drivers/media/platform/rockchip/isp1/Makefile  |8 +
>  drivers/media/platform/rockchip/isp1/capture.c | 1691 
> 
>  drivers/media/platform/rockchip/isp1/capture.h |   46 +
>  drivers/media/platform/rockchip/isp1/common.h  |  330 
>  drivers/media/platform/rockchip/isp1/dev.c |  632 
>  drivers/media/platform/rockchip/isp1/isp_params.c  | 1556 ++
>  drivers/media/platform/rockchip/isp1/isp_params.h  |   81 +
>  drivers/media/platform/rockchip/isp1/isp_stats.c   |  537 +++
>  drivers/media/platform/rockchip/isp1/isp_stats.h   |   81 +
>  .../media/platform/rockchip/isp1/mipi_dphy_sy.c|  805 ++
>  drivers/media/platform/rockchip/isp1/regs.c|  251 +++
>  drivers/media/platform/rockchip/isp1/regs.h| 1578 ++
>  drivers/media/platform/rockchip/isp1/rkisp1.c  | 1230 ++
>  drivers/media/platform/rockchip/isp1/rkisp1.h  |  130 ++
>  drivers/media/v4l2-core/v4l2-ioctl.c   |2 +
>  include/uapi/linux/rkisp1-config.h |  554 +++
>  include/uapi/linux/videodev2.h |4 +
>  24 files changed, 9725 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/rockchip-isp1.txt
>  create mode 100644 
> Documentation/devicetree/bindings/media/rockchip-mipi-dphy.txt
>  create mode 100644 drivers/media/platform/rockchip/isp1/Makefile
>  create mode 100644 drivers/media/platform/rockchip/isp1/capture.c
>  create mode 100644 drivers/media/platform/rockchip/isp1/capture.h
>  create mode 100644 drivers/media/platform/rockchip/isp1/common.h
>  create mode 100644 drivers/media/platform/rockchip/isp1/dev.c
>  create mode 100644 drivers/media/platform/rockchip/isp1/isp_params.c
>  create mode 100644 

Re: regression: 4.13 cannot follow symlinks on some ext3 fs

[Andreas Dilger]

On Nov 23, 2017, at 7:04 PM, Andi Kleen  wrote:
> 
>> As a workaround, you could delete and recreate the symlink with the new
> 
> I revert the patch for now. Everything seems to work.
> 
>> kernel to create a proper fast symlink.  It would be useful to scan
>> the image to see if there are other similar symlinks present:
>> 
>>find /myth/tmp -type l -size -60 -ls | awk '$2 != 0 { print }'
> 
> Doesn't find anything. Your recipe must be wrong.

I see that I should have used "-60c" to properly limit the listing to
short symlinks, but this doesn't appear to be the core problem.  It
looks like there is a bug in find (at least version 4.4.2 that I'm
testing with) that it doesn't print the blocks count properly.

According to find(1) the "-ls" argument should list the file the same
as "ls -dils" format (blocks is $2), but as shown below "find -ls"
prints "0" for blocks when it should be "4" (for a long symlink using
"+60c" in my example, I couldn't find any short+external symlinks on a
couple of 8 year old root filesystems):

$ find /etc/alternatives/rmid -type l -size +60c -ls
327877 0 lrwxrwxrwx 1 root root 73 Jan  4  2017 /etc/alternatives/rmid -> 
/usr/lib/jvm/java-1.8.0-openjdk-1.8.0.111-0.b15.el6_8.x86_64/jre/bin/rmid

$ ls -dils /etc/alternatives/rmid
327877 4 lrwxrwxrwx 1 root root 73 Jan  4  2017 /etc/alternatives/rmid -> 
/usr/lib/jvm/java-1.8.0-openjdk-1.8.0.111-0.b15.el6_8.x86_64/jre/bin/rmid*


Try the following command instead:

find / -type l -size -60c -print0 | xargs -0r ls -dils | awk '$2 != 0 { print }'


>> This is probably something that e2fsck should check for and fix.
> 
> Nah the kernel should just support it like it always did.

The reason we changed this code in the first place was because the
old check would repeatedly break when some new reason for storing
blocks on a symlink appeared.  It broke when xattrs were allowed
on symlinks for SELinux.  It broke when bigalloc blocks were added.
It broke when inline_data was added, and it would have broken (and
been really hard to fix efficiently) when large xattrs were added.

We checked old kernels, and old e2fsprogs, and didn't see any cases
where fast (<= 60 chars) symlinks were created using external blocks.
It seems that _something_ did create them, and it would be good to
figure that out so we can determine if it is a widespread problem.

I think e2fsck can fix this quite easily, and there really isn't
an easy way to revert to the old method if the large xattr feature
is enabled.  If you are willing to run a new kernel, you should also
be willing to run a new e2fsck.

We could probably add a fallback to the old mechanism (and print
a one-time warning to upgrade to a newer e2fsck) if an external fast
symlink is found and the large xattr  feature is not enabled, which
would give more time to fix this (hopefully rare in the wild) case.

Cheers, Andreas







signature.asc
Description: Message signed with OpenPGP

Re: regression: 4.13 cannot follow symlinks on some ext3 fs

[Andreas Dilger]

On Nov 23, 2017, at 7:04 PM, Andi Kleen  wrote:
> 
>> As a workaround, you could delete and recreate the symlink with the new
> 
> I revert the patch for now. Everything seems to work.
> 
>> kernel to create a proper fast symlink.  It would be useful to scan
>> the image to see if there are other similar symlinks present:
>> 
>>find /myth/tmp -type l -size -60 -ls | awk '$2 != 0 { print }'
> 
> Doesn't find anything. Your recipe must be wrong.

I see that I should have used "-60c" to properly limit the listing to
short symlinks, but this doesn't appear to be the core problem.  It
looks like there is a bug in find (at least version 4.4.2 that I'm
testing with) that it doesn't print the blocks count properly.

According to find(1) the "-ls" argument should list the file the same
as "ls -dils" format (blocks is $2), but as shown below "find -ls"
prints "0" for blocks when it should be "4" (for a long symlink using
"+60c" in my example, I couldn't find any short+external symlinks on a
couple of 8 year old root filesystems):

$ find /etc/alternatives/rmid -type l -size +60c -ls
327877 0 lrwxrwxrwx 1 root root 73 Jan  4  2017 /etc/alternatives/rmid -> 
/usr/lib/jvm/java-1.8.0-openjdk-1.8.0.111-0.b15.el6_8.x86_64/jre/bin/rmid

$ ls -dils /etc/alternatives/rmid
327877 4 lrwxrwxrwx 1 root root 73 Jan  4  2017 /etc/alternatives/rmid -> 
/usr/lib/jvm/java-1.8.0-openjdk-1.8.0.111-0.b15.el6_8.x86_64/jre/bin/rmid*


Try the following command instead:

find / -type l -size -60c -print0 | xargs -0r ls -dils | awk '$2 != 0 { print }'


>> This is probably something that e2fsck should check for and fix.
> 
> Nah the kernel should just support it like it always did.

The reason we changed this code in the first place was because the
old check would repeatedly break when some new reason for storing
blocks on a symlink appeared.  It broke when xattrs were allowed
on symlinks for SELinux.  It broke when bigalloc blocks were added.
It broke when inline_data was added, and it would have broken (and
been really hard to fix efficiently) when large xattrs were added.

We checked old kernels, and old e2fsprogs, and didn't see any cases
where fast (<= 60 chars) symlinks were created using external blocks.
It seems that _something_ did create them, and it would be good to
figure that out so we can determine if it is a widespread problem.

I think e2fsck can fix this quite easily, and there really isn't
an easy way to revert to the old method if the large xattr feature
is enabled.  If you are willing to run a new kernel, you should also
be willing to run a new e2fsck.

We could probably add a fallback to the old mechanism (and print
a one-time warning to upgrade to a newer e2fsck) if an external fast
symlink is found and the large xattr  feature is not enabled, which
would give more time to fix this (hopefully rare in the wild) case.

Cheers, Andreas







signature.asc
Description: Message signed with OpenPGP

Re: [PATCH] fat: Fix sb_rdonly() change

[OGAWA Hirofumi]

Joe Perches  writes:

> On Thu, 2017-11-23 at 15:29 +0900, OGAWA Hirofumi wrote:
>> Ouch forgot to add stable@
>> 
>> -- 
>> commit bc98a42c1f7d0f886c0c1b75a92a004976a46d9f introduced bug.
>
> I think your commit message needs a bit more information.
>
> It'd be useful to describe that the introduction of
> sb_rdonly converted the bitwise & to a boolean and so
> this conversion and comparison was made defective.
>
> Are there any other instances of defective comparisons?

Please ask to that patch author.
-- 
OGAWA Hirofumi 

Re: [PATCH] fat: Fix sb_rdonly() change

[OGAWA Hirofumi]

Joe Perches  writes:

> On Thu, 2017-11-23 at 15:29 +0900, OGAWA Hirofumi wrote:
>> Ouch forgot to add stable@
>> 
>> -- 
>> commit bc98a42c1f7d0f886c0c1b75a92a004976a46d9f introduced bug.
>
> I think your commit message needs a bit more information.
>
> It'd be useful to describe that the introduction of
> sb_rdonly converted the bitwise & to a boolean and so
> this conversion and comparison was made defective.
>
> Are there any other instances of defective comparisons?

Please ask to that patch author.
-- 
OGAWA Hirofumi 

Re: [PATCH 2/2] scripts: leaking_addresses: help screen updates

[Tobin C. Harding]

On Thu, Nov 23, 2017 at 10:45:31AM +0530, kaiwan.billimo...@gmail.com wrote:
> The current leaking_addresses.pl script only supports showing "leaked"
> 64-bit kernel virtual addresses. This patch modifies the "help" screen in the
> following manner:
> - the '--raw', '--suppress-dmesg', '--squash-by-path' and 
> '--squash-by-filename'
>   option switches are only meaningful when the '--input-raw=' option switch is
>   used. So, indent the 'Help' screen lines to reflect the fact.
> - an additional example demonstrating usage of the new '--page-offset'
>   parameter.
> 
> 
> Feedback welcome..
> 
> 
> Signed-off-by: Kaiwan N Billimoria 
> ---
> diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> index 7ca218221486..3832abb743d7 100755
> --- a/scripts/leaking_addresses.pl
> +++ b/scripts/leaking_addresses.pl
> @@ -105,10 +105,10 @@ Options:
>  
>   -o, --output-raw=  Save results for future processing.
>   -i, --input-raw=   Read results from file instead of scanning.
> - --rawShow raw results (default).
> - --suppress-dmesg Do not show dmesg results.
> - --squash-by-path Show one result per unique path.
> - --squash-by-filename Show one result per unique filename.
> +   --rawShow raw results (default).
> +   --suppress-dmesg Do not show dmesg results.
> +   --squash-by-path Show one result per unique path.
> +   --squash-by-filename Show one result per unique filename.
>   --page-offset=  PAGE_OFFSET value (for 32-bit kernels).
>   -d, --debug  Display debugging output.
>   -h, --help, --versionDisplay this help and exit.
> @@ -124,6 +124,10 @@ Examples:
>   # View summary report.
>   $0 --input-raw scan.out --squash-by-filename
>  
> + # (On a 32-bit system with a 2GB:2GB VMSPLIT), pass PAGE_OFFSET value
> + # as a parameter
> + $0 --page-offset=0x8000

This should be in the first patch since that is the patch that added it.

> +
>  Scans the running (32 or 64 bit) kernel for potential leaking addresses.
>  
>  EOM

Neither of these patches applies to my tree. Are you editing the diff's
by hand? I noticed the patches don't end with the version signature, like this:


2.7.4

thanks,
Tobin.

Re: [PATCH 2/2] scripts: leaking_addresses: help screen updates

[Tobin C. Harding]

On Thu, Nov 23, 2017 at 10:45:31AM +0530, kaiwan.billimo...@gmail.com wrote:
> The current leaking_addresses.pl script only supports showing "leaked"
> 64-bit kernel virtual addresses. This patch modifies the "help" screen in the
> following manner:
> - the '--raw', '--suppress-dmesg', '--squash-by-path' and 
> '--squash-by-filename'
>   option switches are only meaningful when the '--input-raw=' option switch is
>   used. So, indent the 'Help' screen lines to reflect the fact.
> - an additional example demonstrating usage of the new '--page-offset'
>   parameter.
> 
> 
> Feedback welcome..
> 
> 
> Signed-off-by: Kaiwan N Billimoria 
> ---
> diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> index 7ca218221486..3832abb743d7 100755
> --- a/scripts/leaking_addresses.pl
> +++ b/scripts/leaking_addresses.pl
> @@ -105,10 +105,10 @@ Options:
>  
>   -o, --output-raw=  Save results for future processing.
>   -i, --input-raw=   Read results from file instead of scanning.
> - --rawShow raw results (default).
> - --suppress-dmesg Do not show dmesg results.
> - --squash-by-path Show one result per unique path.
> - --squash-by-filename Show one result per unique filename.
> +   --rawShow raw results (default).
> +   --suppress-dmesg Do not show dmesg results.
> +   --squash-by-path Show one result per unique path.
> +   --squash-by-filename Show one result per unique filename.
>   --page-offset=  PAGE_OFFSET value (for 32-bit kernels).
>   -d, --debug  Display debugging output.
>   -h, --help, --versionDisplay this help and exit.
> @@ -124,6 +124,10 @@ Examples:
>   # View summary report.
>   $0 --input-raw scan.out --squash-by-filename
>  
> + # (On a 32-bit system with a 2GB:2GB VMSPLIT), pass PAGE_OFFSET value
> + # as a parameter
> + $0 --page-offset=0x8000

This should be in the first patch since that is the patch that added it.

> +
>  Scans the running (32 or 64 bit) kernel for potential leaking addresses.
>  
>  EOM

Neither of these patches applies to my tree. Are you editing the diff's
by hand? I noticed the patches don't end with the version signature, like this:


2.7.4

thanks,
Tobin.

[RFC v2] dma-coherent: introduce no-align to avoid allocation failure and save memory

[Jaewon Kim]

dma-coherent uses bitmap APIs which internally consider align based on the
requested size. If most of allocations are small size like KBs, using
alignment scheme seems to be good for anti-fragmentation. But if large
allocation are commonly used, then an allocation could be failed because
of the alignment. To avoid the allocation failure, we had to increase total
size.

This is a example, total size is 30MB, only few memory at front is being
used, and 9MB is being requsted. Then 9MB will be aligned to 16MB. The
first try on offset 0MB will be failed because others already are using
them. The second try on offset 16MB will be failed because of ouf of bound.

So if the alignment is not necessary on a specific dma-coherent memory
region, we can set no-align property. Then dma-coherent will ignore the
alignment only for the memory region.

patch changelog:

v2: use no-align property rather than forcely using no-align

Signed-off-by: Jaewon Kim 
---
 .../bindings/reserved-memory/reserved-memory.txt   |  6 +++
 arch/arm/mm/dma-mapping-nommu.c|  3 +-
 drivers/base/dma-coherent.c| 49 --
 include/linux/dma-mapping.h| 12 +++---
 4 files changed, 50 insertions(+), 20 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt 
b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
index 16291f2a4688..b279e111a7ca 100644
--- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
@@ -63,6 +63,12 @@ reusable (optional) - empty property
   able to reclaim it back. Typically that means that the operating
   system can use that region to store volatile or cached data that
   can be otherwise regenerated or migrated elsewhere.
+no-align (optional) - empty property
+- Depending on a device or its usage pattern, tring to do aligning is not
+  useful. Because of aligning, allocation can be failed and that leads to
+  increasing total memory size to avoid the allocation failure. This
+  property indicates allocator will not try to do aligning on size nor
+  offset.
 
 Linux implementation note:
 - If a "linux,cma-default" property is present, then Linux will use the
diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
index 6db5fc26d154..6512dae5d19b 100644
--- a/arch/arm/mm/dma-mapping-nommu.c
+++ b/arch/arm/mm/dma-mapping-nommu.c
@@ -75,8 +75,7 @@ static void arm_nommu_dma_free(struct device *dev, size_t 
size,
if (attrs & DMA_ATTR_NON_CONSISTENT) {
ops->free(dev, size, cpu_addr, dma_addr, attrs);
} else {
-   int ret = dma_release_from_global_coherent(get_order(size),
-  cpu_addr);
+   int ret = dma_release_from_global_coherent(size, cpu_addr);
 
WARN_ON_ONCE(ret == 0);
}
diff --git a/drivers/base/dma-coherent.c b/drivers/base/dma-coherent.c
index 1e6396bb807b..95d96bd764d9 100644
--- a/drivers/base/dma-coherent.c
+++ b/drivers/base/dma-coherent.c
@@ -17,6 +17,7 @@ struct dma_coherent_mem {
int flags;
unsigned long   *bitmap;
spinlock_t  spinlock;
+   boolno_align;
booluse_dev_dma_pfn_offset;
 };
 
@@ -163,19 +164,35 @@ EXPORT_SYMBOL(dma_mark_declared_memory_occupied);
 static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
ssize_t size, dma_addr_t *dma_handle)
 {
-   int order = get_order(size);
unsigned long flags;
int pageno;
void *ret;
 
spin_lock_irqsave(>spinlock, flags);
 
-   if (unlikely(size > (mem->size << PAGE_SHIFT)))
+   if (unlikely(size > (mem->size << PAGE_SHIFT))) {
+   WARN_ONCE(1, "%s too big size, req-size: %zu total-size: %d\n",
+ __func__, size, (mem->size << PAGE_SHIFT));
goto err;
+   }
 
-   pageno = bitmap_find_free_region(mem->bitmap, mem->size, order);
-   if (unlikely(pageno < 0))
-   goto err;
+   if (mem->no_align) {
+   int nr_page = PAGE_ALIGN(size) >> PAGE_SHIFT;
+
+   pageno = bitmap_find_next_zero_area(mem->bitmap, mem->size, 0,
+   nr_page, 0);
+   if (unlikely(pageno >= mem->size)) {
+   pr_err("%s: alloc failed, req-size: %u pages\n", 
__func__, nr_page);
+   goto err;
+   }
+   bitmap_set(mem->bitmap, pageno, nr_page);
+   } else {
+   int order = get_order(size);
+
+   pageno = bitmap_find_free_region(mem->bitmap, mem->size, order);
+   if (unlikely(pageno < 0))
+   goto err;
+   }
 
/*
 

[RFC v2] dma-coherent: introduce no-align to avoid allocation failure and save memory

[Jaewon Kim]

dma-coherent uses bitmap APIs which internally consider align based on the
requested size. If most of allocations are small size like KBs, using
alignment scheme seems to be good for anti-fragmentation. But if large
allocation are commonly used, then an allocation could be failed because
of the alignment. To avoid the allocation failure, we had to increase total
size.

This is a example, total size is 30MB, only few memory at front is being
used, and 9MB is being requsted. Then 9MB will be aligned to 16MB. The
first try on offset 0MB will be failed because others already are using
them. The second try on offset 16MB will be failed because of ouf of bound.

So if the alignment is not necessary on a specific dma-coherent memory
region, we can set no-align property. Then dma-coherent will ignore the
alignment only for the memory region.

patch changelog:

v2: use no-align property rather than forcely using no-align

Signed-off-by: Jaewon Kim 
---
 .../bindings/reserved-memory/reserved-memory.txt   |  6 +++
 arch/arm/mm/dma-mapping-nommu.c|  3 +-
 drivers/base/dma-coherent.c| 49 --
 include/linux/dma-mapping.h| 12 +++---
 4 files changed, 50 insertions(+), 20 deletions(-)

diff --git 
a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt 
b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
index 16291f2a4688..b279e111a7ca 100644
--- a/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
+++ b/Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt
@@ -63,6 +63,12 @@ reusable (optional) - empty property
   able to reclaim it back. Typically that means that the operating
   system can use that region to store volatile or cached data that
   can be otherwise regenerated or migrated elsewhere.
+no-align (optional) - empty property
+- Depending on a device or its usage pattern, tring to do aligning is not
+  useful. Because of aligning, allocation can be failed and that leads to
+  increasing total memory size to avoid the allocation failure. This
+  property indicates allocator will not try to do aligning on size nor
+  offset.
 
 Linux implementation note:
 - If a "linux,cma-default" property is present, then Linux will use the
diff --git a/arch/arm/mm/dma-mapping-nommu.c b/arch/arm/mm/dma-mapping-nommu.c
index 6db5fc26d154..6512dae5d19b 100644
--- a/arch/arm/mm/dma-mapping-nommu.c
+++ b/arch/arm/mm/dma-mapping-nommu.c
@@ -75,8 +75,7 @@ static void arm_nommu_dma_free(struct device *dev, size_t 
size,
if (attrs & DMA_ATTR_NON_CONSISTENT) {
ops->free(dev, size, cpu_addr, dma_addr, attrs);
} else {
-   int ret = dma_release_from_global_coherent(get_order(size),
-  cpu_addr);
+   int ret = dma_release_from_global_coherent(size, cpu_addr);
 
WARN_ON_ONCE(ret == 0);
}
diff --git a/drivers/base/dma-coherent.c b/drivers/base/dma-coherent.c
index 1e6396bb807b..95d96bd764d9 100644
--- a/drivers/base/dma-coherent.c
+++ b/drivers/base/dma-coherent.c
@@ -17,6 +17,7 @@ struct dma_coherent_mem {
int flags;
unsigned long   *bitmap;
spinlock_t  spinlock;
+   boolno_align;
booluse_dev_dma_pfn_offset;
 };
 
@@ -163,19 +164,35 @@ EXPORT_SYMBOL(dma_mark_declared_memory_occupied);
 static void *__dma_alloc_from_coherent(struct dma_coherent_mem *mem,
ssize_t size, dma_addr_t *dma_handle)
 {
-   int order = get_order(size);
unsigned long flags;
int pageno;
void *ret;
 
spin_lock_irqsave(>spinlock, flags);
 
-   if (unlikely(size > (mem->size << PAGE_SHIFT)))
+   if (unlikely(size > (mem->size << PAGE_SHIFT))) {
+   WARN_ONCE(1, "%s too big size, req-size: %zu total-size: %d\n",
+ __func__, size, (mem->size << PAGE_SHIFT));
goto err;
+   }
 
-   pageno = bitmap_find_free_region(mem->bitmap, mem->size, order);
-   if (unlikely(pageno < 0))
-   goto err;
+   if (mem->no_align) {
+   int nr_page = PAGE_ALIGN(size) >> PAGE_SHIFT;
+
+   pageno = bitmap_find_next_zero_area(mem->bitmap, mem->size, 0,
+   nr_page, 0);
+   if (unlikely(pageno >= mem->size)) {
+   pr_err("%s: alloc failed, req-size: %u pages\n", 
__func__, nr_page);
+   goto err;
+   }
+   bitmap_set(mem->bitmap, pageno, nr_page);
+   } else {
+   int order = get_order(size);
+
+   pageno = bitmap_find_free_region(mem->bitmap, mem->size, order);
+   if (unlikely(pageno < 0))
+   goto err;
+   }
 
/*
 * Memory was found in the 

Re: [PATCH 1/2] scripts: leaking_addresses: add support for 32-bit kernel addresses

[Tobin C. Harding]

Hi Kaiwan,

thanks for the patches!

On Thu, Nov 23, 2017 at 10:44:00AM +0530, kaiwan.billimo...@gmail.com wrote:
> The current leaking_addresses.pl script only supports showing "leaked"
> 64-bit kernel virtual addresses. This patch adds support for showing
> "leaked" 32-bit kernel virtual addresses. It also takes into account Tobin's
> feedback on the previous iteration. (Note: this patch is meant to apply on
> the 'leaks' branch of Tobin's tree).

Please don't mention me in the commit log. Usually this sort of comment
would go below the - so it doesn't get into the kernel tree.

Perhaps

Currently leaking_addresses.pl only supports scanning 64 bit kernels. We
can scan 32 bit kernels also by ... (describe PAGE_OFFSET stuff)

> Briefly, the way it works- once it detects we're running on an i'x'86 
> platform,
> (where x=3|4|5|6), it takes this arch into account for checking. The 
> essential rationale:
>  if virt-addr >= PAGE_OFFSET => it's a kernel virtual address.
> 
> This version programatically queries and sets PAGE_OFFSET based on the
> /boot/config-$(uname -r) content. If, for any reason, this file cannot be
> used, we fallback to requesting the user to pass PAGE_OFFSET as a parameter.

This is a good start. What if we were to check a few places in order?

/boot/config
/boot/config-$(uname -r)
/proc/config.gz 

And fall back to 0xc000 with a warning printed to stderr if we can't
find it?

I'd suggest creating a sub routine get_page_offset() that returns
it. You could cache the result locally in the subroutine to make it
faster, here is a stack overflow post on how to do that


https://stackoverflow.com/questions/10841076/static-local-variables-in-perl

Or you could save it to a global and check this each time the you enter
the subroutine, which ever you fancy.

> Pending/TODO:
> - support for ARM-32

We don't need this in the git log either :)

> Feedback welcome..

Or this. Once it is in the tree no feed back will be possible.

> Signed-off-by: Kaiwan N Billimoria 
> ---
> diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> index 865c07649dff..0566f8055ec5 100755
> --- a/scripts/leaking_addresses.pl
> +++ b/scripts/leaking_addresses.pl
> @@ -2,10 +2,10 @@
>  #
>  # (c) 2017 Tobin C. Harding 
>  # (c) 2017 Kaiwan N Billimoria  (ix86 support)
> - 
> +
>  # Licensed under the terms of the GNU GPL License version 2
>  #
> -# leaking_addresses.pl: Scan 64 bit kernel for potential leaking addresses.
> +# leaking_addresses.pl: Scan 32/64 bit kernel for potential leaking 
> addresses.
>  #  - Scans dmesg output.
>  #  - Walks directory tree and parses each file (for each directory in @DIRS).
>  #
> @@ -14,7 +14,7 @@
>  #
>  # You may like to set kptr_restrict=2 before running script
>  # (see Documentation/sysctl/kernel.txt).
> -
> +#
>  use warnings;
>  use strict;
>  use POSIX;
> @@ -37,7 +37,7 @@ my $TIMEOUT = 10;
>  # Script can only grep for kernel addresses on the following architectures. 
> If
>  # your architecture is not listed here and has a grep'able kernel address 
> please
>  # consider submitting a patch.
> -my @SUPPORTED_ARCHITECTURES = ('x86_64', 'ppc64');
> +my @SUPPORTED_ARCHITECTURES = ('x86_64', 'ppc64', 'i[3456]86');
>  
>  # Command line options.
>  my $help = 0;
> @@ -49,6 +49,12 @@ my $input_raw = "";# Read raw results from file 
> instead of scanning.
>  my $suppress_dmesg = 0;  # Don't show dmesg in output.
>  my $squash_by_path = 0;  # Summary report grouped by absolute 
> path.
>  my $squash_by_filename = 0;  # Summary report grouped by filename.
> +my $page_offset_param = 0;  # 32-bit: overrides value of 
> PAGE_OFFSET_32BIT

I don't like the _param here, it's not in style with the rest of the
code. I do like the global name $PAGE_OFFSET_32BIT though. You don't
_really_ need both since the command line option _is_ a global. I also
struggled with the Perl variable nomenclature between capitals for
globals but not for command line options. (For the record I attempted to
imitate checkpatch.pl.)

 +my $page_offset = 0; # ...

> +
> +my $bit_size = 64;  # Check 64-bit kernel addresses by default

I thought we said we didn't need this?

> +my $kconfig_file = '/boot/config-'.`uname -r`;
> +$kconfig_file =~ s/\R*//g;
> +my $PAGE_OFFSET_32BIT = 0xc000;

So, the page_offset stuff still needs a bit of work. We want it as
simple as possible and also we don't want the 32 bit stuff cluttering up
the 64 bit stuff (i.e with lots of globals). For this reason I think it
would be nice to confine all this to a subroutine then we can do

if (is_ix86_32()) {
$page_offset = get_page_offset();
...
if ($addr < $page_offset)
...
}

>  # Do not parse these files (absolute path).
>  my @skip_parse_files_abs = ('/proc/kmsg',
> @@ -99,10 +105,11 @@ Options:
>  
>   -o, 

Re: [PATCH 1/2] scripts: leaking_addresses: add support for 32-bit kernel addresses

[Tobin C. Harding]

Hi Kaiwan,

thanks for the patches!

On Thu, Nov 23, 2017 at 10:44:00AM +0530, kaiwan.billimo...@gmail.com wrote:
> The current leaking_addresses.pl script only supports showing "leaked"
> 64-bit kernel virtual addresses. This patch adds support for showing
> "leaked" 32-bit kernel virtual addresses. It also takes into account Tobin's
> feedback on the previous iteration. (Note: this patch is meant to apply on
> the 'leaks' branch of Tobin's tree).

Please don't mention me in the commit log. Usually this sort of comment
would go below the - so it doesn't get into the kernel tree.

Perhaps

Currently leaking_addresses.pl only supports scanning 64 bit kernels. We
can scan 32 bit kernels also by ... (describe PAGE_OFFSET stuff)

> Briefly, the way it works- once it detects we're running on an i'x'86 
> platform,
> (where x=3|4|5|6), it takes this arch into account for checking. The 
> essential rationale:
>  if virt-addr >= PAGE_OFFSET => it's a kernel virtual address.
> 
> This version programatically queries and sets PAGE_OFFSET based on the
> /boot/config-$(uname -r) content. If, for any reason, this file cannot be
> used, we fallback to requesting the user to pass PAGE_OFFSET as a parameter.

This is a good start. What if we were to check a few places in order?

/boot/config
/boot/config-$(uname -r)
/proc/config.gz 

And fall back to 0xc000 with a warning printed to stderr if we can't
find it?

I'd suggest creating a sub routine get_page_offset() that returns
it. You could cache the result locally in the subroutine to make it
faster, here is a stack overflow post on how to do that


https://stackoverflow.com/questions/10841076/static-local-variables-in-perl

Or you could save it to a global and check this each time the you enter
the subroutine, which ever you fancy.

> Pending/TODO:
> - support for ARM-32

We don't need this in the git log either :)

> Feedback welcome..

Or this. Once it is in the tree no feed back will be possible.

> Signed-off-by: Kaiwan N Billimoria 
> ---
> diff --git a/scripts/leaking_addresses.pl b/scripts/leaking_addresses.pl
> index 865c07649dff..0566f8055ec5 100755
> --- a/scripts/leaking_addresses.pl
> +++ b/scripts/leaking_addresses.pl
> @@ -2,10 +2,10 @@
>  #
>  # (c) 2017 Tobin C. Harding 
>  # (c) 2017 Kaiwan N Billimoria  (ix86 support)
> - 
> +
>  # Licensed under the terms of the GNU GPL License version 2
>  #
> -# leaking_addresses.pl: Scan 64 bit kernel for potential leaking addresses.
> +# leaking_addresses.pl: Scan 32/64 bit kernel for potential leaking 
> addresses.
>  #  - Scans dmesg output.
>  #  - Walks directory tree and parses each file (for each directory in @DIRS).
>  #
> @@ -14,7 +14,7 @@
>  #
>  # You may like to set kptr_restrict=2 before running script
>  # (see Documentation/sysctl/kernel.txt).
> -
> +#
>  use warnings;
>  use strict;
>  use POSIX;
> @@ -37,7 +37,7 @@ my $TIMEOUT = 10;
>  # Script can only grep for kernel addresses on the following architectures. 
> If
>  # your architecture is not listed here and has a grep'able kernel address 
> please
>  # consider submitting a patch.
> -my @SUPPORTED_ARCHITECTURES = ('x86_64', 'ppc64');
> +my @SUPPORTED_ARCHITECTURES = ('x86_64', 'ppc64', 'i[3456]86');
>  
>  # Command line options.
>  my $help = 0;
> @@ -49,6 +49,12 @@ my $input_raw = "";# Read raw results from file 
> instead of scanning.
>  my $suppress_dmesg = 0;  # Don't show dmesg in output.
>  my $squash_by_path = 0;  # Summary report grouped by absolute 
> path.
>  my $squash_by_filename = 0;  # Summary report grouped by filename.
> +my $page_offset_param = 0;  # 32-bit: overrides value of 
> PAGE_OFFSET_32BIT

I don't like the _param here, it's not in style with the rest of the
code. I do like the global name $PAGE_OFFSET_32BIT though. You don't
_really_ need both since the command line option _is_ a global. I also
struggled with the Perl variable nomenclature between capitals for
globals but not for command line options. (For the record I attempted to
imitate checkpatch.pl.)

 +my $page_offset = 0; # ...

> +
> +my $bit_size = 64;  # Check 64-bit kernel addresses by default

I thought we said we didn't need this?

> +my $kconfig_file = '/boot/config-'.`uname -r`;
> +$kconfig_file =~ s/\R*//g;
> +my $PAGE_OFFSET_32BIT = 0xc000;

So, the page_offset stuff still needs a bit of work. We want it as
simple as possible and also we don't want the 32 bit stuff cluttering up
the 64 bit stuff (i.e with lots of globals). For this reason I think it
would be nice to confine all this to a subroutine then we can do

if (is_ix86_32()) {
$page_offset = get_page_offset();
...
if ($addr < $page_offset)
...
}

>  # Do not parse these files (absolute path).
>  my @skip_parse_files_abs = ('/proc/kmsg',
> @@ -99,10 +105,11 @@ Options:
>  
>   -o, --output-raw=  Save results for future processing.
>   -i, --input-raw=   Read 

Re: [PATCH v2 1/5] mm: memory_hotplug: Memory hotplug (add) support for arm64

[Arun KS]

On Thu, Nov 23, 2017 at 4:43 PM, Maciej Bielski
 wrote:
> Introduces memory hotplug functionality (hot-add) for arm64.
>
> Changes v1->v2:
> - swapper pgtable updated in place on hot add, avoiding unnecessary copy:
>   all changes are additive and non destructive.
>
> - stop_machine used to updated swapper on hot add, avoiding races
>
> - checking if pagealloc is under debug to stay coherent with mem_map
>
> Signed-off-by: Maciej Bielski 
> Signed-off-by: Andrea Reale 
> ---
>  arch/arm64/Kconfig   | 12 ++
>  arch/arm64/configs/defconfig |  1 +
>  arch/arm64/include/asm/mmu.h |  3 ++
>  arch/arm64/mm/init.c | 87 
> 
>  arch/arm64/mm/mmu.c  | 39 
>  5 files changed, 142 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 0df64a6..c736bba 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -641,6 +641,14 @@ config HOTPLUG_CPU
>   Say Y here to experiment with turning CPUs off and on.  CPUs
>   can be controlled through /sys/devices/system/cpu.
>
> +config ARCH_HAS_ADD_PAGES
> +   def_bool y
> +   depends on ARCH_ENABLE_MEMORY_HOTPLUG
> +
> +config ARCH_ENABLE_MEMORY_HOTPLUG
> +   def_bool y
> +depends on !NUMA
> +
>  # Common NUMA Features
>  config NUMA
> bool "Numa Memory Allocation and Scheduler Support"
> @@ -715,6 +723,10 @@ config ARCH_HAS_CACHE_LINE_SIZE
>
>  source "mm/Kconfig"
>
> +config ARCH_MEMORY_PROBE
> +   def_bool y
> +   depends on MEMORY_HOTPLUG
> +
>  config SECCOMP
> bool "Enable seccomp to safely compute untrusted bytecode"
> ---help---
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 34480e9..5fc5656 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -80,6 +80,7 @@ CONFIG_ARM64_VA_BITS_48=y
>  CONFIG_SCHED_MC=y
>  CONFIG_NUMA=y
>  CONFIG_PREEMPT=y
> +CONFIG_MEMORY_HOTPLUG=y
>  CONFIG_KSM=y
>  CONFIG_TRANSPARENT_HUGEPAGE=y
>  CONFIG_CMA=y
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index 0d34bf0..2b3fa4d 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -40,5 +40,8 @@ extern void create_pgd_mapping(struct mm_struct *mm, 
> phys_addr_t phys,
>pgprot_t prot, bool page_mappings_only);
>  extern void *fixmap_remap_fdt(phys_addr_t dt_phys);
>  extern void mark_linear_text_alias_ro(void);
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +extern void hotplug_paging(phys_addr_t start, phys_addr_t size);
> +#endif
>
>  #endif
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 5960bef..e96e7d3 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -722,3 +722,90 @@ static int __init register_mem_limit_dumper(void)
> return 0;
>  }
>  __initcall(register_mem_limit_dumper);
> +
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +int add_pages(int nid, unsigned long start_pfn,
> +   unsigned long nr_pages, bool want_memblock)
> +{
> +   int ret;
> +   u64 start_addr = start_pfn << PAGE_SHIFT;
> +   /*
> +* Mark the first page in the range as unusable. This is needed
> +* because __add_section (within __add_pages) wants pfn_valid
> +* of it to be false, and in arm64 pfn falid is implemented by
> +* just checking at the nomap flag for existing blocks.
> +*
> +* A small trick here is that __add_section() requires only
> +* phys_start_pfn (that is the first pfn of a section) to be
> +* invalid. Regardless of whether it was assumed (by the function
> +* author) that all pfns within a section are either all valid
> +* or all invalid, it allows to avoid looping twice (once here,
> +* second when memblock_clear_nomap() is called) through all
> +* pfns of the section and modify only one pfn. Thanks to that,
> +* further, in __add_zone() only this very first pfn is skipped
> +* and corresponding page is not flagged reserved. Therefore it
> +* is enough to correct this setup only for it.
> +*
> +* When arch_add_memory() returns the walk_memory_range() function
> +* is called and passed with online_memory_block() callback,
> +* which execution finally reaches the memory_block_action()
> +* function, where also only the first pfn of a memory block is
> +* checked to be reserved. Above, it was first pfn of a section,
> +* here it is a block but
> +* (drivers/base/memory.c):
> +* sections_per_block = block_sz / MIN_MEMORY_BLOCK_SIZE;
> +* (include/linux/memory.h):
> +* #define MIN_MEMORY_BLOCK_SIZE (1UL << SECTION_SIZE_BITS)
> +* so we can consider block and section equivalently
> +*/
> +   

Re: [PATCH v2 1/5] mm: memory_hotplug: Memory hotplug (add) support for arm64

[Arun KS]

On Thu, Nov 23, 2017 at 4:43 PM, Maciej Bielski
 wrote:
> Introduces memory hotplug functionality (hot-add) for arm64.
>
> Changes v1->v2:
> - swapper pgtable updated in place on hot add, avoiding unnecessary copy:
>   all changes are additive and non destructive.
>
> - stop_machine used to updated swapper on hot add, avoiding races
>
> - checking if pagealloc is under debug to stay coherent with mem_map
>
> Signed-off-by: Maciej Bielski 
> Signed-off-by: Andrea Reale 
> ---
>  arch/arm64/Kconfig   | 12 ++
>  arch/arm64/configs/defconfig |  1 +
>  arch/arm64/include/asm/mmu.h |  3 ++
>  arch/arm64/mm/init.c | 87 
> 
>  arch/arm64/mm/mmu.c  | 39 
>  5 files changed, 142 insertions(+)
>
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index 0df64a6..c736bba 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -641,6 +641,14 @@ config HOTPLUG_CPU
>   Say Y here to experiment with turning CPUs off and on.  CPUs
>   can be controlled through /sys/devices/system/cpu.
>
> +config ARCH_HAS_ADD_PAGES
> +   def_bool y
> +   depends on ARCH_ENABLE_MEMORY_HOTPLUG
> +
> +config ARCH_ENABLE_MEMORY_HOTPLUG
> +   def_bool y
> +depends on !NUMA
> +
>  # Common NUMA Features
>  config NUMA
> bool "Numa Memory Allocation and Scheduler Support"
> @@ -715,6 +723,10 @@ config ARCH_HAS_CACHE_LINE_SIZE
>
>  source "mm/Kconfig"
>
> +config ARCH_MEMORY_PROBE
> +   def_bool y
> +   depends on MEMORY_HOTPLUG
> +
>  config SECCOMP
> bool "Enable seccomp to safely compute untrusted bytecode"
> ---help---
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index 34480e9..5fc5656 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -80,6 +80,7 @@ CONFIG_ARM64_VA_BITS_48=y
>  CONFIG_SCHED_MC=y
>  CONFIG_NUMA=y
>  CONFIG_PREEMPT=y
> +CONFIG_MEMORY_HOTPLUG=y
>  CONFIG_KSM=y
>  CONFIG_TRANSPARENT_HUGEPAGE=y
>  CONFIG_CMA=y
> diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
> index 0d34bf0..2b3fa4d 100644
> --- a/arch/arm64/include/asm/mmu.h
> +++ b/arch/arm64/include/asm/mmu.h
> @@ -40,5 +40,8 @@ extern void create_pgd_mapping(struct mm_struct *mm, 
> phys_addr_t phys,
>pgprot_t prot, bool page_mappings_only);
>  extern void *fixmap_remap_fdt(phys_addr_t dt_phys);
>  extern void mark_linear_text_alias_ro(void);
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +extern void hotplug_paging(phys_addr_t start, phys_addr_t size);
> +#endif
>
>  #endif
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 5960bef..e96e7d3 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -722,3 +722,90 @@ static int __init register_mem_limit_dumper(void)
> return 0;
>  }
>  __initcall(register_mem_limit_dumper);
> +
> +#ifdef CONFIG_MEMORY_HOTPLUG
> +int add_pages(int nid, unsigned long start_pfn,
> +   unsigned long nr_pages, bool want_memblock)
> +{
> +   int ret;
> +   u64 start_addr = start_pfn << PAGE_SHIFT;
> +   /*
> +* Mark the first page in the range as unusable. This is needed
> +* because __add_section (within __add_pages) wants pfn_valid
> +* of it to be false, and in arm64 pfn falid is implemented by
> +* just checking at the nomap flag for existing blocks.
> +*
> +* A small trick here is that __add_section() requires only
> +* phys_start_pfn (that is the first pfn of a section) to be
> +* invalid. Regardless of whether it was assumed (by the function
> +* author) that all pfns within a section are either all valid
> +* or all invalid, it allows to avoid looping twice (once here,
> +* second when memblock_clear_nomap() is called) through all
> +* pfns of the section and modify only one pfn. Thanks to that,
> +* further, in __add_zone() only this very first pfn is skipped
> +* and corresponding page is not flagged reserved. Therefore it
> +* is enough to correct this setup only for it.
> +*
> +* When arch_add_memory() returns the walk_memory_range() function
> +* is called and passed with online_memory_block() callback,
> +* which execution finally reaches the memory_block_action()
> +* function, where also only the first pfn of a memory block is
> +* checked to be reserved. Above, it was first pfn of a section,
> +* here it is a block but
> +* (drivers/base/memory.c):
> +* sections_per_block = block_sz / MIN_MEMORY_BLOCK_SIZE;
> +* (include/linux/memory.h):
> +* #define MIN_MEMORY_BLOCK_SIZE (1UL << SECTION_SIZE_BITS)
> +* so we can consider block and section equivalently
> +*/
> +   memblock_mark_nomap(start_addr, 1< +   ret = __add_pages(nid, start_pfn, nr_pages, 

Re: [PATCH] crypto: arm64/aes - do not call crypto_unregister_skcipher twice on error

[LABBE Corentin]

On Wed, Nov 22, 2017 at 08:55:14AM +, Ard Biesheuvel wrote:
> Hello Corentin,
> 
> On 22 November 2017 at 08:08, Corentin Labbe  wrote:
> > When a cipher fail
> 
> fails
> 
> > to register in aes_init(), the error path go thought
> 
> goes through
> 
> > aes_exit() then crypto_unregister_skciphers().
> > Since aes_exit calls also crypto_unregister_skcipher, this trigger a
> 
> triggers
> 
> > refcount_t: underflow; use-after-free.
> >
> > Signed-off-by: Corentin Labbe 
> > ---
> >  arch/arm64/crypto/aes-glue.c | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/crypto/aes-glue.c b/arch/arm64/crypto/aes-glue.c
> > index 998ba519a026..9e42ec96243e 100644
> > --- a/arch/arm64/crypto/aes-glue.c
> > +++ b/arch/arm64/crypto/aes-glue.c
> > @@ -664,7 +664,10 @@ static int __init aes_init(void)
> > return 0;
> >
> >  unregister_simds:
> > -   aes_exit();
> > +   for (i = 0; i < ARRAY_SIZE(aes_simd_algs); i++)
> > +   if (aes_simd_algs[i])
> > +   simd_skcipher_free(aes_simd_algs[i]);
> > +   crypto_unregister_shashes(mac_algs, ARRAY_SIZE(mac_algs));
> >  unregister_ciphers:
> > crypto_unregister_skciphers(aes_algs, ARRAY_SIZE(aes_algs));
> > return err;
> > --
> > 2.13.6
> >
> >
> 
> 
> Would this also fix it?
> 
> diff --git a/arch/arm64/crypto/aes-glue.c b/arch/arm64/crypto/aes-glue.c
> index 998ba519a026..2fa850e86aa8 100644
> --- a/arch/arm64/crypto/aes-glue.c
> +++ b/arch/arm64/crypto/aes-glue.c
> @@ -665,6 +665,7 @@ static int __init aes_init(void)
> 
>  unregister_simds:
> aes_exit();
> +   return err;
>  unregister_ciphers:
> crypto_unregister_skciphers(aes_algs, ARRAY_SIZE(aes_algs));
> return err;

Yes it is better.

I will send a v2 today.

Regards

Re: [PATCH] crypto: arm64/aes - do not call crypto_unregister_skcipher twice on error

[LABBE Corentin]

On Wed, Nov 22, 2017 at 08:55:14AM +, Ard Biesheuvel wrote:
> Hello Corentin,
> 
> On 22 November 2017 at 08:08, Corentin Labbe  wrote:
> > When a cipher fail
> 
> fails
> 
> > to register in aes_init(), the error path go thought
> 
> goes through
> 
> > aes_exit() then crypto_unregister_skciphers().
> > Since aes_exit calls also crypto_unregister_skcipher, this trigger a
> 
> triggers
> 
> > refcount_t: underflow; use-after-free.
> >
> > Signed-off-by: Corentin Labbe 
> > ---
> >  arch/arm64/crypto/aes-glue.c | 5 -
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/crypto/aes-glue.c b/arch/arm64/crypto/aes-glue.c
> > index 998ba519a026..9e42ec96243e 100644
> > --- a/arch/arm64/crypto/aes-glue.c
> > +++ b/arch/arm64/crypto/aes-glue.c
> > @@ -664,7 +664,10 @@ static int __init aes_init(void)
> > return 0;
> >
> >  unregister_simds:
> > -   aes_exit();
> > +   for (i = 0; i < ARRAY_SIZE(aes_simd_algs); i++)
> > +   if (aes_simd_algs[i])
> > +   simd_skcipher_free(aes_simd_algs[i]);
> > +   crypto_unregister_shashes(mac_algs, ARRAY_SIZE(mac_algs));
> >  unregister_ciphers:
> > crypto_unregister_skciphers(aes_algs, ARRAY_SIZE(aes_algs));
> > return err;
> > --
> > 2.13.6
> >
> >
> 
> 
> Would this also fix it?
> 
> diff --git a/arch/arm64/crypto/aes-glue.c b/arch/arm64/crypto/aes-glue.c
> index 998ba519a026..2fa850e86aa8 100644
> --- a/arch/arm64/crypto/aes-glue.c
> +++ b/arch/arm64/crypto/aes-glue.c
> @@ -665,6 +665,7 @@ static int __init aes_init(void)
> 
>  unregister_simds:
> aes_exit();
> +   return err;
>  unregister_ciphers:
> crypto_unregister_skciphers(aes_algs, ARRAY_SIZE(aes_algs));
> return err;

Yes it is better.

I will send a v2 today.

Regards

[PATCH 1/2] dt-bindings: clocksource: Add Spreadtrum SC9860 timer

[Baolin Wang]

This patch adds documentation of device tree bindings for the timers
found on Spreadtrum SC9860 platform.

Signed-off-by: Baolin Wang 
---
 .../bindings/timer/spreadtrum,sprd-timer.txt   |   20 
 1 file changed, 20 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/timer/spreadtrum,sprd-timer.txt

diff --git a/Documentation/devicetree/bindings/timer/spreadtrum,sprd-timer.txt 
b/Documentation/devicetree/bindings/timer/spreadtrum,sprd-timer.txt
new file mode 100644
index 000..f9d5eb9
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/spreadtrum,sprd-timer.txt
@@ -0,0 +1,20 @@
+Spreadtrum timers
+
+The Spreadtrum SC9860 platform provides 3 general-purpose timers.
+These timers can support 32bit or 64bit counter, as well as supporting
+period mode or one-shot mode, and they are can be wakeup source
+during deep sleep.
+
+Required properties:
+- compatible: should be "sprd,sc9860-timer" for SC9860 platform.
+- reg: The register address of the timer device.
+- interrupts: Should contain the interrupt for the timer device.
+- clock-frequency: The frequency of the clock that drives the counter, in Hz.
+
+Example:
+   timer@4005 {
+   compatible = "sprd,sc9860-timer";
+   reg = <0 0x4005 0 0x20>;
+   interrupts = ;
+   clock-frequency = <32768>;
+   };
-- 
1.7.9.5

[PATCH 1/2] dt-bindings: clocksource: Add Spreadtrum SC9860 timer

[Baolin Wang]

This patch adds documentation of device tree bindings for the timers
found on Spreadtrum SC9860 platform.

Signed-off-by: Baolin Wang 
---
 .../bindings/timer/spreadtrum,sprd-timer.txt   |   20 
 1 file changed, 20 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/timer/spreadtrum,sprd-timer.txt

diff --git a/Documentation/devicetree/bindings/timer/spreadtrum,sprd-timer.txt 
b/Documentation/devicetree/bindings/timer/spreadtrum,sprd-timer.txt
new file mode 100644
index 000..f9d5eb9
--- /dev/null
+++ b/Documentation/devicetree/bindings/timer/spreadtrum,sprd-timer.txt
@@ -0,0 +1,20 @@
+Spreadtrum timers
+
+The Spreadtrum SC9860 platform provides 3 general-purpose timers.
+These timers can support 32bit or 64bit counter, as well as supporting
+period mode or one-shot mode, and they are can be wakeup source
+during deep sleep.
+
+Required properties:
+- compatible: should be "sprd,sc9860-timer" for SC9860 platform.
+- reg: The register address of the timer device.
+- interrupts: Should contain the interrupt for the timer device.
+- clock-frequency: The frequency of the clock that drives the counter, in Hz.
+
+Example:
+   timer@4005 {
+   compatible = "sprd,sc9860-timer";
+   reg = <0 0x4005 0 0x20>;
+   interrupts = ;
+   clock-frequency = <32768>;
+   };
-- 
1.7.9.5

[PATCH 2/2] clocksource: sprd: Add timer driver for Spreadtrum SC9860 platform

[Baolin Wang]

The Spreadtrum SC9860 platform will use the architected timers as local
clock events, but we also need a broadcast timer device to wakeup the
cpus when the cpus are in sleep mode.

Thus this patch registers the timer0 to be a broadcast timer supporting
periodic and oneshot events.

Signed-off-by: Baolin Wang 
---
 drivers/clocksource/Kconfig  |8 ++
 drivers/clocksource/Makefile |1 +
 drivers/clocksource/sprd-timer.c |  213 ++
 3 files changed, 222 insertions(+)
 create mode 100644 drivers/clocksource/sprd-timer.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index cc60620..aa05132 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -467,6 +467,14 @@ config MTK_TIMER
help
  Support for Mediatek timer driver.
 
+config SPRD_TIMER
+   bool "Spreadtrum timer driver"
+   depends on GENERIC_CLOCKEVENTS
+   depends on ARCH_SPRD || COMPILE_TEST
+   select TIMER_OF
+   help
+ Enables the support for the Spreadtrum timer driver.
+
 config SYS_SUPPORTS_SH_MTU2
 bool
 
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index dbc1ad1..c657d3d 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_CLKSRC_TI_32K)   += timer-ti-32k.o
 obj-$(CONFIG_CLKSRC_NPS)   += timer-nps.o
 obj-$(CONFIG_OXNAS_RPS_TIMER)  += timer-oxnas-rps.o
 obj-$(CONFIG_OWL_TIMER)+= owl-timer.o
+obj-$(CONFIG_SPRD_TIMER)   += sprd-timer.o
 
 obj-$(CONFIG_ARC_TIMERS)   += arc_timer.o
 obj-$(CONFIG_ARM_ARCH_TIMER)   += arm_arch_timer.o
diff --git a/drivers/clocksource/sprd-timer.c b/drivers/clocksource/sprd-timer.c
new file mode 100644
index 000..6fec0c5
--- /dev/null
+++ b/drivers/clocksource/sprd-timer.c
@@ -0,0 +1,213 @@
+/*
+ * Copyright (C) 2017 Spreadtrum Communications Inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define TIMER_NAME "sprd_timer"
+
+#define TIMER_LOAD_LO  0x0
+#define TIMER_LOAD_HI  0x4
+#define TIMER_VALUE_LO 0x8
+#define TIMER_VALUE_HI 0xc
+
+#define TIMER_CTL  0x10
+#define TIMER_CTL_PERIOD_MODE  BIT(0)
+#define TIMER_CTL_ENABLE   BIT(1)
+#define TIMER_CTL_64BIT_WIDTH  BIT(16)
+
+#define TIMER_INT  0x14
+#define TIMER_INT_EN   BIT(0)
+#define TIMER_INT_RAW_STS  BIT(1)
+#define TIMER_INT_MASK_STS BIT(2)
+#define TIMER_INT_CLR  BIT(3)
+
+#define TIMER_VALUE_SHDW_LO0x18
+#define TIMER_VALUE_SHDW_HI0x1c
+
+#define TIMER_VALUE_LO_MASKGENMASK(31, 0)
+#define TIMER_VALUE_HI_SHIFT   32
+
+struct sprd_timer_device {
+   struct clock_event_device ce;
+   void __iomem *base;
+   u32 freq;
+   int irq;
+};
+
+static inline struct sprd_timer_device *
+to_sprd_timer(struct clock_event_device *c)
+{
+   return container_of(c, struct sprd_timer_device, ce);
+}
+
+static void sprd_timer_enable(struct sprd_timer_device *timer, u32 flag)
+{
+   u32 val = readl_relaxed(timer->base + TIMER_CTL);
+
+   val |= TIMER_CTL_ENABLE;
+   if (flag & TIMER_CTL_64BIT_WIDTH)
+   val |= TIMER_CTL_64BIT_WIDTH;
+   else
+   val &= ~TIMER_CTL_64BIT_WIDTH;
+
+   if (flag & TIMER_CTL_PERIOD_MODE)
+   val |= TIMER_CTL_PERIOD_MODE;
+   else
+   val &= ~TIMER_CTL_PERIOD_MODE;
+
+   writel_relaxed(val, timer->base + TIMER_CTL);
+}
+
+static void sprd_timer_disable(struct sprd_timer_device *timer)
+{
+   u32 val = readl_relaxed(timer->base + TIMER_CTL);
+
+   val &= ~TIMER_CTL_ENABLE;
+   writel_relaxed(val, timer->base + TIMER_CTL);
+}
+
+static void sprd_timer_update_counter(struct sprd_timer_device *timer,
+ unsigned long cycles)
+{
+   writel_relaxed(cycles & TIMER_VALUE_LO_MASK,
+  timer->base + TIMER_LOAD_LO);
+   writel_relaxed(cycles >> TIMER_VALUE_HI_SHIFT,
+  timer->base + TIMER_LOAD_HI);
+}
+
+static void sprd_timer_enable_interrupt(struct sprd_timer_device *timer)
+{
+   writel_relaxed(TIMER_INT_EN, timer->base + TIMER_INT);
+}
+
+static void sprd_timer_clear_interrupt(struct sprd_timer_device *timer)
+{
+   u32 val = readl_relaxed(timer->base + TIMER_INT);
+
+   val |= TIMER_INT_CLR;
+   writel_relaxed(val, timer->base + TIMER_INT);
+}
+
+static int sprd_timer_set_next_event(unsigned long cycles,
+struct clock_event_device *ce)
+{
+   struct sprd_timer_device *timer = to_sprd_timer(ce);
+
+   sprd_timer_disable(timer);
+   sprd_timer_update_counter(timer, cycles);
+   sprd_timer_enable(timer, TIMER_CTL_64BIT_WIDTH);
+
+   return 0;
+}
+
+static int sprd_timer_set_periodic(struct 

[PATCH 2/2] clocksource: sprd: Add timer driver for Spreadtrum SC9860 platform

[Baolin Wang]

The Spreadtrum SC9860 platform will use the architected timers as local
clock events, but we also need a broadcast timer device to wakeup the
cpus when the cpus are in sleep mode.

Thus this patch registers the timer0 to be a broadcast timer supporting
periodic and oneshot events.

Signed-off-by: Baolin Wang 
---
 drivers/clocksource/Kconfig  |8 ++
 drivers/clocksource/Makefile |1 +
 drivers/clocksource/sprd-timer.c |  213 ++
 3 files changed, 222 insertions(+)
 create mode 100644 drivers/clocksource/sprd-timer.c

diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
index cc60620..aa05132 100644
--- a/drivers/clocksource/Kconfig
+++ b/drivers/clocksource/Kconfig
@@ -467,6 +467,14 @@ config MTK_TIMER
help
  Support for Mediatek timer driver.
 
+config SPRD_TIMER
+   bool "Spreadtrum timer driver"
+   depends on GENERIC_CLOCKEVENTS
+   depends on ARCH_SPRD || COMPILE_TEST
+   select TIMER_OF
+   help
+ Enables the support for the Spreadtrum timer driver.
+
 config SYS_SUPPORTS_SH_MTU2
 bool
 
diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
index dbc1ad1..c657d3d 100644
--- a/drivers/clocksource/Makefile
+++ b/drivers/clocksource/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_CLKSRC_TI_32K)   += timer-ti-32k.o
 obj-$(CONFIG_CLKSRC_NPS)   += timer-nps.o
 obj-$(CONFIG_OXNAS_RPS_TIMER)  += timer-oxnas-rps.o
 obj-$(CONFIG_OWL_TIMER)+= owl-timer.o
+obj-$(CONFIG_SPRD_TIMER)   += sprd-timer.o
 
 obj-$(CONFIG_ARC_TIMERS)   += arc_timer.o
 obj-$(CONFIG_ARM_ARCH_TIMER)   += arm_arch_timer.o
diff --git a/drivers/clocksource/sprd-timer.c b/drivers/clocksource/sprd-timer.c
new file mode 100644
index 000..6fec0c5
--- /dev/null
+++ b/drivers/clocksource/sprd-timer.c
@@ -0,0 +1,213 @@
+/*
+ * Copyright (C) 2017 Spreadtrum Communications Inc.
+ *
+ * SPDX-License-Identifier: GPL-2.0
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define TIMER_NAME "sprd_timer"
+
+#define TIMER_LOAD_LO  0x0
+#define TIMER_LOAD_HI  0x4
+#define TIMER_VALUE_LO 0x8
+#define TIMER_VALUE_HI 0xc
+
+#define TIMER_CTL  0x10
+#define TIMER_CTL_PERIOD_MODE  BIT(0)
+#define TIMER_CTL_ENABLE   BIT(1)
+#define TIMER_CTL_64BIT_WIDTH  BIT(16)
+
+#define TIMER_INT  0x14
+#define TIMER_INT_EN   BIT(0)
+#define TIMER_INT_RAW_STS  BIT(1)
+#define TIMER_INT_MASK_STS BIT(2)
+#define TIMER_INT_CLR  BIT(3)
+
+#define TIMER_VALUE_SHDW_LO0x18
+#define TIMER_VALUE_SHDW_HI0x1c
+
+#define TIMER_VALUE_LO_MASKGENMASK(31, 0)
+#define TIMER_VALUE_HI_SHIFT   32
+
+struct sprd_timer_device {
+   struct clock_event_device ce;
+   void __iomem *base;
+   u32 freq;
+   int irq;
+};
+
+static inline struct sprd_timer_device *
+to_sprd_timer(struct clock_event_device *c)
+{
+   return container_of(c, struct sprd_timer_device, ce);
+}
+
+static void sprd_timer_enable(struct sprd_timer_device *timer, u32 flag)
+{
+   u32 val = readl_relaxed(timer->base + TIMER_CTL);
+
+   val |= TIMER_CTL_ENABLE;
+   if (flag & TIMER_CTL_64BIT_WIDTH)
+   val |= TIMER_CTL_64BIT_WIDTH;
+   else
+   val &= ~TIMER_CTL_64BIT_WIDTH;
+
+   if (flag & TIMER_CTL_PERIOD_MODE)
+   val |= TIMER_CTL_PERIOD_MODE;
+   else
+   val &= ~TIMER_CTL_PERIOD_MODE;
+
+   writel_relaxed(val, timer->base + TIMER_CTL);
+}
+
+static void sprd_timer_disable(struct sprd_timer_device *timer)
+{
+   u32 val = readl_relaxed(timer->base + TIMER_CTL);
+
+   val &= ~TIMER_CTL_ENABLE;
+   writel_relaxed(val, timer->base + TIMER_CTL);
+}
+
+static void sprd_timer_update_counter(struct sprd_timer_device *timer,
+ unsigned long cycles)
+{
+   writel_relaxed(cycles & TIMER_VALUE_LO_MASK,
+  timer->base + TIMER_LOAD_LO);
+   writel_relaxed(cycles >> TIMER_VALUE_HI_SHIFT,
+  timer->base + TIMER_LOAD_HI);
+}
+
+static void sprd_timer_enable_interrupt(struct sprd_timer_device *timer)
+{
+   writel_relaxed(TIMER_INT_EN, timer->base + TIMER_INT);
+}
+
+static void sprd_timer_clear_interrupt(struct sprd_timer_device *timer)
+{
+   u32 val = readl_relaxed(timer->base + TIMER_INT);
+
+   val |= TIMER_INT_CLR;
+   writel_relaxed(val, timer->base + TIMER_INT);
+}
+
+static int sprd_timer_set_next_event(unsigned long cycles,
+struct clock_event_device *ce)
+{
+   struct sprd_timer_device *timer = to_sprd_timer(ce);
+
+   sprd_timer_disable(timer);
+   sprd_timer_update_counter(timer, cycles);
+   sprd_timer_enable(timer, TIMER_CTL_64BIT_WIDTH);
+
+   return 0;
+}
+
+static int sprd_timer_set_periodic(struct clock_event_device *ce)
+{
+  

Re: [PATCH] ASoC: amd: added error checks in dma driver

[Mukunda,Vijendar]

On Thursday 23 November 2017 10:59 PM, Mark Brown wrote:

On Thu, Nov 23, 2017 at 08:59:43AM -0800, Guenter Roeck wrote:

On Thu, Nov 23, 2017 at 8:30 AM, Vijendar Mukunda
 wrote:

added error checks in acp dma driver
Signed-off-by: Vijendar Mukunda 
Signed-off-by: Akshu Agrawal 
Signed-off-by: Guenter Roeck 

This is inappropriate.

Specifically: if Guenter wasn't involved in writing or forwarding the
patch he shouldn't have a signoff in there, and if you're the one
sending the mail you should be the last person in the chain of signoffs.
Please see SubmittingPatches for details of what a signoff means and why
they're important.


  This patch was implemented on top of changes implemented by Guenter.
  There is a separate thread - RE: [PATCH] ASoC: amd: Add error checking
  to probe function in which Guenter posted changes.

  Got it, apologies will post changes as v2 version.

Re: [PATCH] ASoC: amd: added error checks in dma driver

[Mukunda,Vijendar]

On Thursday 23 November 2017 10:59 PM, Mark Brown wrote:

On Thu, Nov 23, 2017 at 08:59:43AM -0800, Guenter Roeck wrote:

On Thu, Nov 23, 2017 at 8:30 AM, Vijendar Mukunda
 wrote:

added error checks in acp dma driver
Signed-off-by: Vijendar Mukunda 
Signed-off-by: Akshu Agrawal 
Signed-off-by: Guenter Roeck 

This is inappropriate.

Specifically: if Guenter wasn't involved in writing or forwarding the
patch he shouldn't have a signoff in there, and if you're the one
sending the mail you should be the last person in the chain of signoffs.
Please see SubmittingPatches for details of what a signoff means and why
they're important.


  This patch was implemented on top of changes implemented by Guenter.
  There is a separate thread - RE: [PATCH] ASoC: amd: Add error checking
  to probe function in which Guenter posted changes.

  Got it, apologies will post changes as v2 version.

Re: [PATCH v2 1/2] s390/virtio: remove the old KVM virtio headers

[Thomas Huth]

On 24.11.2017 06:21, Michael S. Tsirkin wrote:
> commit 7fb2b2d51 ("s390/virtio: remove the old KVM virtio transport")
> dropped the transport support. We don't need to keep the header around.
> 
> Cc: Thomas Huth 
> Cc: Cornelia Huck 
> Cc: Halil Pasic 
> Cc: Heiko Carstens 
> Cc: Martin Schwidefsky 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  arch/s390/include/uapi/asm/kvm_virtio.h | 65 
> -
>  1 file changed, 65 deletions(-)
>  delete mode 100644 arch/s390/include/uapi/asm/kvm_virtio.h
> 
> diff --git a/arch/s390/include/uapi/asm/kvm_virtio.h 
> b/arch/s390/include/uapi/asm/kvm_virtio.h
> deleted file mode 100644
> index 7328367..000
> --- a/arch/s390/include/uapi/asm/kvm_virtio.h
> +++ /dev/null

This seems to be already upstream? See:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a401917bc3e2d251ce521

 Thomas

Re: [PATCH v2 1/2] s390/virtio: remove the old KVM virtio headers

[Thomas Huth]

On 24.11.2017 06:21, Michael S. Tsirkin wrote:
> commit 7fb2b2d51 ("s390/virtio: remove the old KVM virtio transport")
> dropped the transport support. We don't need to keep the header around.
> 
> Cc: Thomas Huth 
> Cc: Cornelia Huck 
> Cc: Halil Pasic 
> Cc: Heiko Carstens 
> Cc: Martin Schwidefsky 
> Signed-off-by: Michael S. Tsirkin 
> ---
>  arch/s390/include/uapi/asm/kvm_virtio.h | 65 
> -
>  1 file changed, 65 deletions(-)
>  delete mode 100644 arch/s390/include/uapi/asm/kvm_virtio.h
> 
> diff --git a/arch/s390/include/uapi/asm/kvm_virtio.h 
> b/arch/s390/include/uapi/asm/kvm_virtio.h
> deleted file mode 100644
> index 7328367..000
> --- a/arch/s390/include/uapi/asm/kvm_virtio.h
> +++ /dev/null

This seems to be already upstream? See:

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a401917bc3e2d251ce521

 Thomas