Re: kernel BUG at crypto/asymmetric_keys/public_key.c:80
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
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
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
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
On Thu, Nov 23, 2017 at 2:42 PM, Alexander Potapenkowrote: > >> > 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
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
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
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
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
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
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
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
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
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()
From: Markus ElfringDate: 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()
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
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
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
From: Markus ElfringDate: 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
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
From: Markus ElfringDate: 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
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()
* Mikulas Patockawrote: > 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()
* 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()
From: Markus ElfringDate: 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()
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
> 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
> 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()
From: Markus ElfringDate: 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()
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
* Dave Hansenwrote: > 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
* 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
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
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()
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()
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()
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()
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
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
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
On Fri, Nov 24, 2017 at 11:29 AM, Tobin C. Hardingwrote: > 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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
On Fri, 2017-11-24 at 15:07 +0900, OGAWA Hirofumi wrote: > Joe Percheswrites: > > > 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
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"
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"
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
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
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 <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
* 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
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
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
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
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
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
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
On Nov 23, 2017, at 7:04 PM, Andi Kleenwrote: > >> 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
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
Joe Percheswrites: > 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
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
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
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
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
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
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
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
On Thu, Nov 23, 2017 at 4:43 PM, Maciej Bielskiwrote: > 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
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
On Wed, Nov 22, 2017 at 08:55:14AM +, Ard Biesheuvel wrote: > Hello Corentin, > > On 22 November 2017 at 08:08, Corentin Labbewrote: > > 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
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
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
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
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
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
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 Mukundawrote: 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
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
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
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