Re: [PATCH 4.19 000/205] 4.19.3-stable review
On Tue, Nov 20, 2018 at 12:44:31PM +0530, Harsh Shandilya wrote: > On 19 November 2018 9:55:07 PM IST, Greg Kroah-Hartman > wrote: > >This is the start of the stable review cycle for the 4.19.3 release. > >There are 205 patches in this series, all will be posted as a response > >to this one. If anyone has any issues with these being applied, please > >let me know. > > > >Responses should be made by Wed Nov 21 16:24:55 UTC 2018. > >Anything received after that time might be too late. > > > >The whole patch series can be found in one patch at: > > > > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.19.3-rc1.gz > >or in the git tree and branch at: > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > >linux-4.19.y > >and the diffstat can be found below. > > > >thanks, > > > >greg k-h > > > Compiled and booted on the Lenovo IdeaPad 330-15ARR, no dmesg regressions. Great! thanks for testing and letting me know. greg k-h
Re: [PATCH 4.19 000/205] 4.19.3-stable review
On Tue, Nov 20, 2018 at 12:44:31PM +0530, Harsh Shandilya wrote: > On 19 November 2018 9:55:07 PM IST, Greg Kroah-Hartman > wrote: > >This is the start of the stable review cycle for the 4.19.3 release. > >There are 205 patches in this series, all will be posted as a response > >to this one. If anyone has any issues with these being applied, please > >let me know. > > > >Responses should be made by Wed Nov 21 16:24:55 UTC 2018. > >Anything received after that time might be too late. > > > >The whole patch series can be found in one patch at: > > > > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.19.3-rc1.gz > >or in the git tree and branch at: > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > >linux-4.19.y > >and the diffstat can be found below. > > > >thanks, > > > >greg k-h > > > Compiled and booted on the Lenovo IdeaPad 330-15ARR, no dmesg regressions. Great! thanks for testing and letting me know. greg k-h
Re: [PATCH 3.18 00/90] 3.18.126-stable review
On Mon, Nov 19, 2018 at 05:09:38PM -0700, shuah wrote: > On 11/19/18 9:28 AM, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 3.18.126 release. > > There are 90 patches in this series, all will be posted as a response > > to this one. If anyone has any issues with these being applied, please > > let me know. > > > > Responses should be made by Wed Nov 21 16:25:28 UTC 2018. > > Anything received after that time might be too late. > > > > The whole patch series can be found in one patch at: > > > > https://www.kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.126-rc1.gz > > or in the git tree and branch at: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > > linux-3.18.y > > and the diffstat can be found below. > > > > thanks, > > > > greg k-h > > > > Compiled and booted ob my test system. No dmesg regressions. Thanks for testing all of these and letting me know. greg k-h
Re: [PATCH 3.18 00/90] 3.18.126-stable review
On Mon, Nov 19, 2018 at 05:09:38PM -0700, shuah wrote: > On 11/19/18 9:28 AM, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 3.18.126 release. > > There are 90 patches in this series, all will be posted as a response > > to this one. If anyone has any issues with these being applied, please > > let me know. > > > > Responses should be made by Wed Nov 21 16:25:28 UTC 2018. > > Anything received after that time might be too late. > > > > The whole patch series can be found in one patch at: > > > > https://www.kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.126-rc1.gz > > or in the git tree and branch at: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > > linux-3.18.y > > and the diffstat can be found below. > > > > thanks, > > > > greg k-h > > > > Compiled and booted ob my test system. No dmesg regressions. Thanks for testing all of these and letting me know. greg k-h
Re: [RFC PATCH] mm, proc: report PR_SET_THP_DISABLE in proc
On Mon 19-11-18 14:05:34, David Rientjes wrote: > On Thu, 15 Nov 2018, Michal Hocko wrote: > > > > The userspace had a single way to determine if thp had been disabled for > > > a > > > specific vma and that was broken with your commit. We have since fixed > > > it. Modifying our software stack to start looking for some field > > > somewhere else will not help anybody else that this has affected or will > > > affect. I'm interested in not breaking userspace, not trying a wait and > > > see approach to see if anybody else complains once we start looking for > > > some other field. The risk outweighs the reward, it already broke us, > > > and > > > I'd prefer not to even open the possibility of breaking anybody else. > > > > I very much agree on "do not break userspace" part but this is kind of > > gray area. VMA flags are a deep internal implementation detail and > > nobody should really depend on it for anything important. The original > > motivation for introducing it was CRIU where it is kind of > > understandable. I would argue they should find a different way but it is > > just too late for them. > > > > For this particular case there was no other bug report except for yours > > and if it is possible to fix it on your end then I would really love to > > make the a sensible user interface to query the status. If we are going > > to change the semantic of the exported flag again then we risk yet > > another breakage. > > > > Therefore I am asking whether changing your particular usecase to a new > > interface is possible because that would allow to have a longerm > > sensible user interface rather than another kludge which still doesn't > > cover all the usecases (e.g. there is no way to reliably query the > > madvise status after your patch). > > > > Providing another interface is great, I have no objection other than > emitting another line for every vma on the system for smaps is probably > overkill for something as rare as PR_SET_THP_DISABLE. Let me think about a full patch and see how it looks like. > > That said, I think the current handling of the "nh" flag being emitted in > smaps is logical and ensures no further userspace breakage. I have already expressed a concern that there is no way to query for MADV_NOHUGEPAGE if we overload the flag. So this is not a riskfree option. > If that is to > be removed, I consider it an unnecessary risk. That would raised in code > review. -- Michal Hocko SUSE Labs
Re: [RFC PATCH] mm, proc: report PR_SET_THP_DISABLE in proc
On Mon 19-11-18 14:05:34, David Rientjes wrote: > On Thu, 15 Nov 2018, Michal Hocko wrote: > > > > The userspace had a single way to determine if thp had been disabled for > > > a > > > specific vma and that was broken with your commit. We have since fixed > > > it. Modifying our software stack to start looking for some field > > > somewhere else will not help anybody else that this has affected or will > > > affect. I'm interested in not breaking userspace, not trying a wait and > > > see approach to see if anybody else complains once we start looking for > > > some other field. The risk outweighs the reward, it already broke us, > > > and > > > I'd prefer not to even open the possibility of breaking anybody else. > > > > I very much agree on "do not break userspace" part but this is kind of > > gray area. VMA flags are a deep internal implementation detail and > > nobody should really depend on it for anything important. The original > > motivation for introducing it was CRIU where it is kind of > > understandable. I would argue they should find a different way but it is > > just too late for them. > > > > For this particular case there was no other bug report except for yours > > and if it is possible to fix it on your end then I would really love to > > make the a sensible user interface to query the status. If we are going > > to change the semantic of the exported flag again then we risk yet > > another breakage. > > > > Therefore I am asking whether changing your particular usecase to a new > > interface is possible because that would allow to have a longerm > > sensible user interface rather than another kludge which still doesn't > > cover all the usecases (e.g. there is no way to reliably query the > > madvise status after your patch). > > > > Providing another interface is great, I have no objection other than > emitting another line for every vma on the system for smaps is probably > overkill for something as rare as PR_SET_THP_DISABLE. Let me think about a full patch and see how it looks like. > > That said, I think the current handling of the "nh" flag being emitted in > smaps is logical and ensures no further userspace breakage. I have already expressed a concern that there is no way to query for MADV_NOHUGEPAGE if we overload the flag. So this is not a riskfree option. > If that is to > be removed, I consider it an unnecessary risk. That would raised in code > review. -- Michal Hocko SUSE Labs
Re: [PATCH 4.4 131/160] mm: thp: relax __GFP_THISNODE for MADV_HUGEPAGE mappings
On Mon 19-11-18 14:16:24, David Rientjes wrote:
> On Mon, 19 Nov 2018, Greg Kroah-Hartman wrote:
>
> > 4.4-stable review patch. If anyone has any objections, please let me know.
> >
>
> As I noted when this patch was originally proposed and when I nacked it[*]
> because it causes a 13.9% increase in remote memory access latency and up
> to 40% increase in remote memory allocation latency on much of our
> software stack that uses MADV_HUGEPAGE after mremapping the text segment
> to memory backed by hugepages, I don't think this is stable material.
There was a wider consensus that this is the most minimal fix for users
who see a regression introduced by 5265047ac301 ("mm, thp: really
limit transparent hugepage allocation to local node"). As it has been
discussed extensively there is no universal win but we should always opt
for the safer side which this patch is accomplishing. The changelog goes
in length explaining them along with numbers. I am not happy that your
particular workload is suffering but this area certainly requires much
more changes to satisfy wider range of users.
> The 4.4 kernel is almost three years old and this changes the NUMA
> locality of any user of MADV_HUGEPAGE.
Yes and we have seen bug reports as we adopted this older kernel only
now.
--
Michal Hocko
SUSE Labs
Re: [PATCH 4.4 131/160] mm: thp: relax __GFP_THISNODE for MADV_HUGEPAGE mappings
On Mon 19-11-18 14:16:24, David Rientjes wrote:
> On Mon, 19 Nov 2018, Greg Kroah-Hartman wrote:
>
> > 4.4-stable review patch. If anyone has any objections, please let me know.
> >
>
> As I noted when this patch was originally proposed and when I nacked it[*]
> because it causes a 13.9% increase in remote memory access latency and up
> to 40% increase in remote memory allocation latency on much of our
> software stack that uses MADV_HUGEPAGE after mremapping the text segment
> to memory backed by hugepages, I don't think this is stable material.
There was a wider consensus that this is the most minimal fix for users
who see a regression introduced by 5265047ac301 ("mm, thp: really
limit transparent hugepage allocation to local node"). As it has been
discussed extensively there is no universal win but we should always opt
for the safer side which this patch is accomplishing. The changelog goes
in length explaining them along with numbers. I am not happy that your
particular workload is suffering but this area certainly requires much
more changes to satisfy wider range of users.
> The 4.4 kernel is almost three years old and this changes the NUMA
> locality of any user of MADV_HUGEPAGE.
Yes and we have seen bug reports as we adopted this older kernel only
now.
--
Michal Hocko
SUSE Labs
Re: [PATCH 02/13] x86/fault: Check user_mode(regs) when validating a stack extension
* Andy Lutomirski wrote:
> The fault handling code tries to validate that a page fault from
> user mode that would extend the stack is within a certain range of
> the user SP. regs->sp is only equal to the user SP if
> user_mode(regs). In the extremely unlikely event that that
> sw_error_code had the USER bit set but the faulting instruction was
> in the kernel (i.e. the faulting instruction was WRUSS), then the
> *kernel* stack pointer would have been checked, which would be an
> info leak.
>
> Note to -stable maintainers: don't backport this unless you backport
> CET. The bug it fixes is unobservable in current kernels.
>
> Signed-off-by: Andy Lutomirski
> ---
> arch/x86/mm/fault.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> index 91d4d2722f2e..eae7ee3ce89b 100644
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -1377,7 +1377,7 @@ void do_user_addr_fault(struct pt_regs *regs,
> bad_area(regs, sw_error_code, address);
> return;
> }
> - if (sw_error_code & X86_PF_USER) {
> + if (user_mode(regs)) {
> /*
>* Accessing the stack below %sp is always a bug.
>* The large cushion allows instructions like enter
Note that this check is gone now due to:
1d8ca3be86eb: x86/mm/fault: Allow stack access below %rsp
Thanks,
Ingo
Re: [PATCH 02/13] x86/fault: Check user_mode(regs) when validating a stack extension
* Andy Lutomirski wrote:
> The fault handling code tries to validate that a page fault from
> user mode that would extend the stack is within a certain range of
> the user SP. regs->sp is only equal to the user SP if
> user_mode(regs). In the extremely unlikely event that that
> sw_error_code had the USER bit set but the faulting instruction was
> in the kernel (i.e. the faulting instruction was WRUSS), then the
> *kernel* stack pointer would have been checked, which would be an
> info leak.
>
> Note to -stable maintainers: don't backport this unless you backport
> CET. The bug it fixes is unobservable in current kernels.
>
> Signed-off-by: Andy Lutomirski
> ---
> arch/x86/mm/fault.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
> index 91d4d2722f2e..eae7ee3ce89b 100644
> --- a/arch/x86/mm/fault.c
> +++ b/arch/x86/mm/fault.c
> @@ -1377,7 +1377,7 @@ void do_user_addr_fault(struct pt_regs *regs,
> bad_area(regs, sw_error_code, address);
> return;
> }
> - if (sw_error_code & X86_PF_USER) {
> + if (user_mode(regs)) {
> /*
>* Accessing the stack below %sp is always a bug.
>* The large cushion allows instructions like enter
Note that this check is gone now due to:
1d8ca3be86eb: x86/mm/fault: Allow stack access below %rsp
Thanks,
Ingo
Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size
On Mon, Nov 19, 2018 at 5:50 PM Robin Murphy wrote: > > On 19/11/2018 14:18, Ramon Fried wrote: > > On Tue, Oct 9, 2018 at 8:02 AM Benjamin Herrenschmidt > > wrote: > >> > >> On Wed, 2018-10-03 at 16:10 -0700, Alexander Duyck wrote: > -* Because 32-bit DMA masks are so common we expect every > architecture > -* to be able to satisfy them - either by not supporting more > physical > -* memory, or by providing a ZONE_DMA32. If neither is the > case, the > -* architecture needs to use an IOMMU instead of the direct > mapping. > -*/ > - if (mask < phys_to_dma(dev, DMA_BIT_MASK(32))) > + u64 min_mask; > + > + if (IS_ENABLED(CONFIG_ZONE_DMA)) > + min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS); > + else > + min_mask = DMA_BIT_MASK(32); > + > + min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT); > + > + if (mask >= phys_to_dma(dev, min_mask)) > return 0; > -#endif > return 1; > } > >>> > >>> So I believe I have run into the same issue that Guenter reported. On > >>> an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and > >>> all probe attempts for various devices were failing with -EIO errors. > >>> > >>> I believe the last mask check should be "if (mask < phys_to_dma(dev, > >>> min_mask))" not a ">=" check. > >> > >> Right, that test is backwards. I needed to change it here too (powermac > >> with the rest of the powerpc series). > >> > >> Cheers, > >> Ben. > >> > >> > > > > Hi, I'm working on a MIPS64 soc with PCIe root complex on it, and it > > appears that this series of patches are causing all PCI drivers that > > request 64bit mask to fail with -5. > > It's broken in 4.19. However, I just checked, it working on master. > > We may need to backport a couple of patches to 4.19. I'm not sure > > though which patches should be backported as there were at least 10 > > patches resolving this dma_direct area recently. > > Christoph, Robin. > > Can we ask Greg to backport all these changes ? What do you think ? > > As far as I'm aware, the only real issue in 4.19 was my subtle breakage > around setting bus_dma_mask - that's fixed by 6778be4e5209, which > according to my inbox got picked up by autosel for 4.19 stable last week. > > Robin. Yep, 6778be4e5209 fixes the issue. Thanks a lot ! Ramon.
Re: [PATCH 5/5] dma-direct: always allow dma mask <= physiscal memory size
On Mon, Nov 19, 2018 at 5:50 PM Robin Murphy wrote: > > On 19/11/2018 14:18, Ramon Fried wrote: > > On Tue, Oct 9, 2018 at 8:02 AM Benjamin Herrenschmidt > > wrote: > >> > >> On Wed, 2018-10-03 at 16:10 -0700, Alexander Duyck wrote: > -* Because 32-bit DMA masks are so common we expect every > architecture > -* to be able to satisfy them - either by not supporting more > physical > -* memory, or by providing a ZONE_DMA32. If neither is the > case, the > -* architecture needs to use an IOMMU instead of the direct > mapping. > -*/ > - if (mask < phys_to_dma(dev, DMA_BIT_MASK(32))) > + u64 min_mask; > + > + if (IS_ENABLED(CONFIG_ZONE_DMA)) > + min_mask = DMA_BIT_MASK(ARCH_ZONE_DMA_BITS); > + else > + min_mask = DMA_BIT_MASK(32); > + > + min_mask = min_t(u64, min_mask, (max_pfn - 1) << PAGE_SHIFT); > + > + if (mask >= phys_to_dma(dev, min_mask)) > return 0; > -#endif > return 1; > } > >>> > >>> So I believe I have run into the same issue that Guenter reported. On > >>> an x86_64 system w/ Intel IOMMU. I wasn't able to complete boot and > >>> all probe attempts for various devices were failing with -EIO errors. > >>> > >>> I believe the last mask check should be "if (mask < phys_to_dma(dev, > >>> min_mask))" not a ">=" check. > >> > >> Right, that test is backwards. I needed to change it here too (powermac > >> with the rest of the powerpc series). > >> > >> Cheers, > >> Ben. > >> > >> > > > > Hi, I'm working on a MIPS64 soc with PCIe root complex on it, and it > > appears that this series of patches are causing all PCI drivers that > > request 64bit mask to fail with -5. > > It's broken in 4.19. However, I just checked, it working on master. > > We may need to backport a couple of patches to 4.19. I'm not sure > > though which patches should be backported as there were at least 10 > > patches resolving this dma_direct area recently. > > Christoph, Robin. > > Can we ask Greg to backport all these changes ? What do you think ? > > As far as I'm aware, the only real issue in 4.19 was my subtle breakage > around setting bus_dma_mask - that's fixed by 6778be4e5209, which > according to my inbox got picked up by autosel for 4.19 stable last week. > > Robin. Yep, 6778be4e5209 fixes the issue. Thanks a lot ! Ramon.
Re: omap5 fixing palmas IRQ_TYPE_NONE warning leads to gpadc timeouts
On 19/11/2018 18.19, Tony Lindgren wrote: > * Peter Ujfalusi [181119 10:16]: >> On 2018-11-13 20:06, Tony Lindgren wrote: >>> Looks like the IRQ_TYPE_NONE issue still is there for omap5 and >>> should be fixed with IRQ_TYPE_HIGH. >>> >>> No idea about why palmas interrupts would stop working though, >>> Peter, do you have any ideas on this one? >> >> No, I don't. >> The INT polarity can be changed in Palmas. >> based on the pdata->irq_flags (queried via irqd_get_trigger_type()) >> the code configures it: >> >> if (pdata->irq_flags & IRQ_TYPE_LEVEL_HIGH) >> reg = PALMAS_POLARITY_CTRL_INT_POLARITY; >> else >> reg = 0; >> >> and we pass the same irq_flags to the regmap_add_irq_chip() >> IRQ_TYPE_LEVEL_HIGH == IRQF_TRIGGER_HIGH == 0x0004 >> >> A change in DT should be enough, no need to patch palmas.c, imho. > > But it's not. I'm now wondering if wakeupgen is inverting the > polarity for this interrupt? > > GIC docs say this about SPI interrupts: > > "SPI is triggered on a rising edge or is active-HIGH level-sensitive." > > So when setting IRQ_TYPE_LEVEL_HIGH in dts, we still must not > invert the polarity in palmas while tegra needs to. So either > tegra114 hardware is inverting the polarity, or omap5 wakeupgen > is. > > Does the palmas trm say which way PALMAS_POLARITY_CTRL > triggers if PALMAS_POLARITY_CTRL_INT_POLARITY is set? bit7 INT_POLARITY Select the polarity of the INT output line 0: Interrupt line (INT) is low when interrupt is pending (default) RW 1: Interrupt line (INT) is high when interrupt is pending > Also note that dra7 is using a gpio for palmas interrupt. > > Regards, > > Tony > - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: omap5 fixing palmas IRQ_TYPE_NONE warning leads to gpadc timeouts
On 19/11/2018 18.19, Tony Lindgren wrote: > * Peter Ujfalusi [181119 10:16]: >> On 2018-11-13 20:06, Tony Lindgren wrote: >>> Looks like the IRQ_TYPE_NONE issue still is there for omap5 and >>> should be fixed with IRQ_TYPE_HIGH. >>> >>> No idea about why palmas interrupts would stop working though, >>> Peter, do you have any ideas on this one? >> >> No, I don't. >> The INT polarity can be changed in Palmas. >> based on the pdata->irq_flags (queried via irqd_get_trigger_type()) >> the code configures it: >> >> if (pdata->irq_flags & IRQ_TYPE_LEVEL_HIGH) >> reg = PALMAS_POLARITY_CTRL_INT_POLARITY; >> else >> reg = 0; >> >> and we pass the same irq_flags to the regmap_add_irq_chip() >> IRQ_TYPE_LEVEL_HIGH == IRQF_TRIGGER_HIGH == 0x0004 >> >> A change in DT should be enough, no need to patch palmas.c, imho. > > But it's not. I'm now wondering if wakeupgen is inverting the > polarity for this interrupt? > > GIC docs say this about SPI interrupts: > > "SPI is triggered on a rising edge or is active-HIGH level-sensitive." > > So when setting IRQ_TYPE_LEVEL_HIGH in dts, we still must not > invert the polarity in palmas while tegra needs to. So either > tegra114 hardware is inverting the polarity, or omap5 wakeupgen > is. > > Does the palmas trm say which way PALMAS_POLARITY_CTRL > triggers if PALMAS_POLARITY_CTRL_INT_POLARITY is set? bit7 INT_POLARITY Select the polarity of the INT output line 0: Interrupt line (INT) is low when interrupt is pending (default) RW 1: Interrupt line (INT) is high when interrupt is pending > Also note that dra7 is using a gpio for palmas interrupt. > > Regards, > > Tony > - Péter Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: Cleaning up numbering for new x86 syscalls?
* Andy Lutomirski wrote: > Hi all- > > We currently have some giant turds in the way that syscalls are > numbered. We have the x86_32 table, which is totally sane other than > some legacy multiplexers. Then we have the x86_64 table, which is, > um, demented: > > - The numbers don't match x86_32. I have no idea why. > > - We use bit 30, which triggers in_x32_syscall(). It should have > been bit 31, bit I digress. > > - We have this weird set of extra x32 syscalls that start at 512. > Who wants to bet whether we have no bugs if someone does syscall with, > say, nr == 512 (i.e. not 512 | BIT(30)) or nr == (16 | BIT(30))? The > latter would be non-compat ioctl with in_x32_syscall() set and hence > in_compat_syscall() set. > > - Bloody restart_syscall() has a different number on x86_64 and > x64_32, which is a big mess. > > I propose we consider some subset of the following: > > 1. Introduce restart_syscall_2(). Make its number be 1024. Maybe > someday we could start using it instead of restart_syscall(). The > only issue I can see is programs that allow restart_syscall() using > seccomp but don't allow the new variant. > > 2. Introduce an outright ban on new syscalls with nr < 1024. Also let's make sure it results in a build error or boot panic if someone tries. > 3. Introduce an outright ban on the addition of new __x32_compat > syscalls. If new compat hacks are needed, they can use > in_compat_syscall(), thank you very much. Here too build-time and runtime enforcement would be nice. > 4. Modify the wrappers of the __x32_compat entries so that they will > return -ENOSYS if in_x32_syscall() returns false. > > 5. Adjust the scripts so that we only have to wire up new syscalls > once. They'll have a nr above 1024, and they'll have the same nr on > all x86 variants. > > Thoughts? Fully agreed: 6. Is x32 even used in practice? I still think it was a mistake to add it and some significant distributions like Fedora are not enabling it. Barring any sane way to phase out x32 support I'd suggest we implement all your suggestions. Thanks, Ingo
Re: Cleaning up numbering for new x86 syscalls?
* Andy Lutomirski wrote: > Hi all- > > We currently have some giant turds in the way that syscalls are > numbered. We have the x86_32 table, which is totally sane other than > some legacy multiplexers. Then we have the x86_64 table, which is, > um, demented: > > - The numbers don't match x86_32. I have no idea why. > > - We use bit 30, which triggers in_x32_syscall(). It should have > been bit 31, bit I digress. > > - We have this weird set of extra x32 syscalls that start at 512. > Who wants to bet whether we have no bugs if someone does syscall with, > say, nr == 512 (i.e. not 512 | BIT(30)) or nr == (16 | BIT(30))? The > latter would be non-compat ioctl with in_x32_syscall() set and hence > in_compat_syscall() set. > > - Bloody restart_syscall() has a different number on x86_64 and > x64_32, which is a big mess. > > I propose we consider some subset of the following: > > 1. Introduce restart_syscall_2(). Make its number be 1024. Maybe > someday we could start using it instead of restart_syscall(). The > only issue I can see is programs that allow restart_syscall() using > seccomp but don't allow the new variant. > > 2. Introduce an outright ban on new syscalls with nr < 1024. Also let's make sure it results in a build error or boot panic if someone tries. > 3. Introduce an outright ban on the addition of new __x32_compat > syscalls. If new compat hacks are needed, they can use > in_compat_syscall(), thank you very much. Here too build-time and runtime enforcement would be nice. > 4. Modify the wrappers of the __x32_compat entries so that they will > return -ENOSYS if in_x32_syscall() returns false. > > 5. Adjust the scripts so that we only have to wire up new syscalls > once. They'll have a nr above 1024, and they'll have the same nr on > all x86 variants. > > Thoughts? Fully agreed: 6. Is x32 even used in practice? I still think it was a mistake to add it and some significant distributions like Fedora are not enabling it. Barring any sane way to phase out x32 support I'd suggest we implement all your suggestions. Thanks, Ingo
Re: [PATCH] dmaengine: ti: omap-dma: Configure LCH_TYPE for OMAP1
Aaro,
On 19/11/2018 20.46, Aaro Koskinen wrote:
> Hi,
>
> On Mon, Nov 19, 2018 at 12:40:40PM +0200, Peter Ujfalusi wrote:
>> When the channel is configured for slave operation the LCH_TYPE needs to be
>> set to LCh-P. For memcpy channels the LCH_TYPE must be set to LCh-2D.
>>
>> Signed-off-by: Peter Ujfalusi
>
> I don't have the documentation, but based on what omap_udc driver (still
> using the legacy OMAP DMA API) does this seems to be correct.
They are hard to fine, true. From the omap1710 TRM:
Logical channel types (LCh types) supported are:
- LCh-2D for nonsynchronized transfers (memory transfers, 1D and 2D)
- LCh-P for synchronized transfers (mostly peripheral transfers)
- LCh-PD similar to LCh-P but runs on a dedicated physical channel
- LCh-G for graphical transfers/operations
- LCh-D for display transfers
Looking at other part it looks like hat LCH-2D channel mode can happily
service a peripheral. LCH-P supports the same features as LCH-2D, but it
lacks support for Single/Double-indexed addressing mode on the
peripheral port side.
So, this patch might not be needed at all. Can you test the omap_udc
with s/OMAP_DMA_LCH_P/OMAP_DMA_LCH_2D
If USB works, then we can just drop this patch.
Note: if we ever need the port_window support in OMAP1 then we need
double indexing on the peripheral side.
> I tested the patch on Nokia 770 with MMC and couldn't see any negative
> impact.
>
> Tested-by: Aaro Koskinen
>
> A.
>
>> ---
>> drivers/dma/ti/omap-dma.c | 11 +++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
>> index a4a931ddf6f6..a18cfd497f04 100644
>> --- a/drivers/dma/ti/omap-dma.c
>> +++ b/drivers/dma/ti/omap-dma.c
>> @@ -185,6 +185,10 @@ enum {
>>
>> CLNK_CTRL_ENABLE_LNK= BIT(15),
>>
>> +/* OMAP1 only */
>> +LCH_CTRL_LCH_2D = 0,
>> +LCH_CTRL_LCH_P = 2,
>> +
>> CDP_DST_VALID_INC = 0 << 0,
>> CDP_DST_VALID_RELOAD= 1 << 0,
>> CDP_DST_VALID_REUSE = 2 << 0,
>> @@ -529,6 +533,7 @@ static void omap_dma_start_sg(struct omap_chan *c,
>> struct omap_desc *d)
>>
>> static void omap_dma_start_desc(struct omap_chan *c)
>> {
>> +struct omap_dmadev *od = to_omap_dma_dev(c->vc.chan.device);
>> struct virt_dma_desc *vd = vchan_next_desc(>vc);
>> struct omap_desc *d;
>> unsigned cxsa, cxei, cxfi;
>> @@ -570,6 +575,12 @@ static void omap_dma_start_desc(struct omap_chan *c)
>> omap_dma_chan_write(c, CSDP, d->csdp);
>> omap_dma_chan_write(c, CLNK_CTRL, d->clnk_ctrl);
>>
>> +if (dma_omap1() && !__dma_omap15xx(od->plat->dma_attr)) {
>> +if (is_slave_direction(d->dir))
>> +omap_dma_chan_write(c, LCH_CTRL, LCH_CTRL_LCH_P);
>> +else
>> +omap_dma_chan_write(c, LCH_CTRL, LCH_CTRL_LCH_2D);
>> +}
>> omap_dma_start_sg(c, d);
>> }
>>
>> --
>> Peter
>>
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>>
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [Ksummit-discuss] [RFC PATCH 2/3] MAINTAINERS, Handbook: Subsystem Profile
On Fri, 16 Nov 2018, Joe Perches wrote: > On Fri, 2018-11-16 at 14:44 +0200, Jani Nikula wrote: >> I quickly cooked up this script to produce the top-5 commit prefixes for >> the given files over the arbitrary last 200 commits. It'll give you a >> pretty good idea if you're even close. >> >> --- >> #!/bin/sh >> # usage: subject-prefix FILE [...] >> # show top 5 subject prefixes for FILEs >> >> git log --format=%s -n 200 -- "$@" |\ >> grep -v "^Merge " |\ >> sed 's/\(.*\):.*/\1/' |\ >> sort | uniq -c | sort -nr | sed 's/ *[0-9]\+ //' |\ >> head -n 5 >> --- >> >> Someone who knows perl could turn that into a checkpatch check: See if >> the patch subject prefix is one of the top-5 for all files changed by >> the patch, and ask the user to double check if it isn't. Or some >> heuristics thereof. > > This won't work when a patch contains multiple files > from different paths, or even multiple files from a > single driver. *shrug* You can give it multiple files as argument, and it'll give you an approximation of what the prefix could be, whether you're way off or not. Close enough at least for the single driver case. Obviously not perfect, but hey, it took me all of five minutes to write that. ;) BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
Re: [PATCH] dmaengine: ti: omap-dma: Configure LCH_TYPE for OMAP1
Aaro,
On 19/11/2018 20.46, Aaro Koskinen wrote:
> Hi,
>
> On Mon, Nov 19, 2018 at 12:40:40PM +0200, Peter Ujfalusi wrote:
>> When the channel is configured for slave operation the LCH_TYPE needs to be
>> set to LCh-P. For memcpy channels the LCH_TYPE must be set to LCh-2D.
>>
>> Signed-off-by: Peter Ujfalusi
>
> I don't have the documentation, but based on what omap_udc driver (still
> using the legacy OMAP DMA API) does this seems to be correct.
They are hard to fine, true. From the omap1710 TRM:
Logical channel types (LCh types) supported are:
- LCh-2D for nonsynchronized transfers (memory transfers, 1D and 2D)
- LCh-P for synchronized transfers (mostly peripheral transfers)
- LCh-PD similar to LCh-P but runs on a dedicated physical channel
- LCh-G for graphical transfers/operations
- LCh-D for display transfers
Looking at other part it looks like hat LCH-2D channel mode can happily
service a peripheral. LCH-P supports the same features as LCH-2D, but it
lacks support for Single/Double-indexed addressing mode on the
peripheral port side.
So, this patch might not be needed at all. Can you test the omap_udc
with s/OMAP_DMA_LCH_P/OMAP_DMA_LCH_2D
If USB works, then we can just drop this patch.
Note: if we ever need the port_window support in OMAP1 then we need
double indexing on the peripheral side.
> I tested the patch on Nokia 770 with MMC and couldn't see any negative
> impact.
>
> Tested-by: Aaro Koskinen
>
> A.
>
>> ---
>> drivers/dma/ti/omap-dma.c | 11 +++
>> 1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/dma/ti/omap-dma.c b/drivers/dma/ti/omap-dma.c
>> index a4a931ddf6f6..a18cfd497f04 100644
>> --- a/drivers/dma/ti/omap-dma.c
>> +++ b/drivers/dma/ti/omap-dma.c
>> @@ -185,6 +185,10 @@ enum {
>>
>> CLNK_CTRL_ENABLE_LNK= BIT(15),
>>
>> +/* OMAP1 only */
>> +LCH_CTRL_LCH_2D = 0,
>> +LCH_CTRL_LCH_P = 2,
>> +
>> CDP_DST_VALID_INC = 0 << 0,
>> CDP_DST_VALID_RELOAD= 1 << 0,
>> CDP_DST_VALID_REUSE = 2 << 0,
>> @@ -529,6 +533,7 @@ static void omap_dma_start_sg(struct omap_chan *c,
>> struct omap_desc *d)
>>
>> static void omap_dma_start_desc(struct omap_chan *c)
>> {
>> +struct omap_dmadev *od = to_omap_dma_dev(c->vc.chan.device);
>> struct virt_dma_desc *vd = vchan_next_desc(>vc);
>> struct omap_desc *d;
>> unsigned cxsa, cxei, cxfi;
>> @@ -570,6 +575,12 @@ static void omap_dma_start_desc(struct omap_chan *c)
>> omap_dma_chan_write(c, CSDP, d->csdp);
>> omap_dma_chan_write(c, CLNK_CTRL, d->clnk_ctrl);
>>
>> +if (dma_omap1() && !__dma_omap15xx(od->plat->dma_attr)) {
>> +if (is_slave_direction(d->dir))
>> +omap_dma_chan_write(c, LCH_CTRL, LCH_CTRL_LCH_P);
>> +else
>> +omap_dma_chan_write(c, LCH_CTRL, LCH_CTRL_LCH_2D);
>> +}
>> omap_dma_start_sg(c, d);
>> }
>>
>> --
>> Peter
>>
>> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
>> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>>
- Péter
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
Re: [Ksummit-discuss] [RFC PATCH 2/3] MAINTAINERS, Handbook: Subsystem Profile
On Fri, 16 Nov 2018, Joe Perches wrote: > On Fri, 2018-11-16 at 14:44 +0200, Jani Nikula wrote: >> I quickly cooked up this script to produce the top-5 commit prefixes for >> the given files over the arbitrary last 200 commits. It'll give you a >> pretty good idea if you're even close. >> >> --- >> #!/bin/sh >> # usage: subject-prefix FILE [...] >> # show top 5 subject prefixes for FILEs >> >> git log --format=%s -n 200 -- "$@" |\ >> grep -v "^Merge " |\ >> sed 's/\(.*\):.*/\1/' |\ >> sort | uniq -c | sort -nr | sed 's/ *[0-9]\+ //' |\ >> head -n 5 >> --- >> >> Someone who knows perl could turn that into a checkpatch check: See if >> the patch subject prefix is one of the top-5 for all files changed by >> the patch, and ask the user to double check if it isn't. Or some >> heuristics thereof. > > This won't work when a patch contains multiple files > from different paths, or even multiple files from a > single driver. *shrug* You can give it multiple files as argument, and it'll give you an approximation of what the prefix could be, whether you're way off or not. Close enough at least for the single driver case. Obviously not perfect, but hey, it took me all of five minutes to write that. ;) BR, Jani. -- Jani Nikula, Intel Open Source Graphics Center
[PATCH 2/2] x86/acpi, x86/boot: Take RSDP address from boot params if available
In case the RSDP address in struct boot_params is specified don't try
to find the table by searching, but take the address directly as set
by the boot loader.
Signed-off-by: Juergen Gross
---
arch/x86/include/uapi/asm/bootparam.h | 3 ++-
arch/x86/kernel/acpi/boot.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/uapi/asm/bootparam.h
b/arch/x86/include/uapi/asm/bootparam.h
index a06cbf019744..60733f137e9a 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -155,7 +155,8 @@ struct boot_params {
__u8 _pad2[4]; /* 0x054 */
__u64 tboot_addr; /* 0x058 */
struct ist_info ist_info; /* 0x060 */
- __u8 _pad3[16];/* 0x070 */
+ __u64 acpi_rsdp_addr; /* 0x070 */
+ __u8 _pad3[8]; /* 0x078 */
__u8 hd0_info[16]; /* obsolete! */ /* 0x080 */
__u8 hd1_info[16]; /* obsolete! */ /* 0x090 */
struct sys_desc_table sys_desc_table; /* obsolete! */ /* 0x0a0 */
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index fb3b1f3a5aba..06635fbca81c 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1776,5 +1776,5 @@ void __init arch_reserve_mem_area(acpi_physical_address
addr, size_t size)
u64 x86_default_get_root_pointer(void)
{
- return 0;
+ return boot_params.acpi_rsdp_addr;
}
--
2.16.4
[PATCH 2/2] x86/acpi, x86/boot: Take RSDP address from boot params if available
In case the RSDP address in struct boot_params is specified don't try
to find the table by searching, but take the address directly as set
by the boot loader.
Signed-off-by: Juergen Gross
---
arch/x86/include/uapi/asm/bootparam.h | 3 ++-
arch/x86/kernel/acpi/boot.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/uapi/asm/bootparam.h
b/arch/x86/include/uapi/asm/bootparam.h
index a06cbf019744..60733f137e9a 100644
--- a/arch/x86/include/uapi/asm/bootparam.h
+++ b/arch/x86/include/uapi/asm/bootparam.h
@@ -155,7 +155,8 @@ struct boot_params {
__u8 _pad2[4]; /* 0x054 */
__u64 tboot_addr; /* 0x058 */
struct ist_info ist_info; /* 0x060 */
- __u8 _pad3[16];/* 0x070 */
+ __u64 acpi_rsdp_addr; /* 0x070 */
+ __u8 _pad3[8]; /* 0x078 */
__u8 hd0_info[16]; /* obsolete! */ /* 0x080 */
__u8 hd1_info[16]; /* obsolete! */ /* 0x090 */
struct sys_desc_table sys_desc_table; /* obsolete! */ /* 0x0a0 */
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index fb3b1f3a5aba..06635fbca81c 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -1776,5 +1776,5 @@ void __init arch_reserve_mem_area(acpi_physical_address
addr, size_t size)
u64 x86_default_get_root_pointer(void)
{
- return 0;
+ return boot_params.acpi_rsdp_addr;
}
--
2.16.4
Re: [PATCH 1/4] mmc: sdhci-omap: Fix DCRC error handling during tuning
Hi Uffe
On 19/11/18 6:48 PM, Ulf Hansson wrote:
> On 19 November 2018 at 12:16, Faiz Abbas wrote:
>> Commit 7d33c3581536 ("mmc: sdhci-omap: Workaround for Errata i802")
>> disabled DCRC interrupts during tuning. This write to the interrupt
>> enable register gets overwritten in sdhci_prepare_data() and the
>> interrupt is not in fact disabled. Fix this by disabling the interrupt
>> in the host->ier variable.
>>
>> Signed-off-by: Faiz Abbas
>
> Should we have fixes/stable tag?
>
Ok. Will add that.
Thanks,
Faiz
Re: [PATCH 1/4] mmc: sdhci-omap: Fix DCRC error handling during tuning
Hi Uffe
On 19/11/18 6:48 PM, Ulf Hansson wrote:
> On 19 November 2018 at 12:16, Faiz Abbas wrote:
>> Commit 7d33c3581536 ("mmc: sdhci-omap: Workaround for Errata i802")
>> disabled DCRC interrupts during tuning. This write to the interrupt
>> enable register gets overwritten in sdhci_prepare_data() and the
>> interrupt is not in fact disabled. Fix this by disabling the interrupt
>> in the host->ier variable.
>>
>> Signed-off-by: Faiz Abbas
>
> Should we have fixes/stable tag?
>
Ok. Will add that.
Thanks,
Faiz
Re: [PATCH v4 1/2] gpio: Add driver for PC Engines APU boards
Hello Linus Signed-off-by: Florian Eckert This is looking better and better! Thanks to everyone helping out and thanks for your perseverance Florian! I have to thanks for reviewing my driver. This is the way opensource works. Thanks for the feedback i will update the driver with your suggestions. - Florian
Re: [PATCH 4.19 000/205] 4.19.3-stable review
On 19 November 2018 9:55:07 PM IST, Greg Kroah-Hartman wrote: >This is the start of the stable review cycle for the 4.19.3 release. >There are 205 patches in this series, all will be posted as a response >to this one. If anyone has any issues with these being applied, please >let me know. > >Responses should be made by Wed Nov 21 16:24:55 UTC 2018. >Anything received after that time might be too late. > >The whole patch series can be found in one patch at: > > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.19.3-rc1.gz >or in the git tree and branch at: > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git >linux-4.19.y >and the diffstat can be found below. > >thanks, > >greg k-h > Compiled and booted on the Lenovo IdeaPad 330-15ARR, no dmesg regressions. -- Harsh Shandilya PRJKT Development LLC
Re: [PATCH v4 1/2] gpio: Add driver for PC Engines APU boards
Hello Linus Signed-off-by: Florian Eckert This is looking better and better! Thanks to everyone helping out and thanks for your perseverance Florian! I have to thanks for reviewing my driver. This is the way opensource works. Thanks for the feedback i will update the driver with your suggestions. - Florian
Re: [PATCH 4.19 000/205] 4.19.3-stable review
On 19 November 2018 9:55:07 PM IST, Greg Kroah-Hartman wrote: >This is the start of the stable review cycle for the 4.19.3 release. >There are 205 patches in this series, all will be posted as a response >to this one. If anyone has any issues with these being applied, please >let me know. > >Responses should be made by Wed Nov 21 16:24:55 UTC 2018. >Anything received after that time might be too late. > >The whole patch series can be found in one patch at: > > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.19.3-rc1.gz >or in the git tree and branch at: > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git >linux-4.19.y >and the diffstat can be found below. > >thanks, > >greg k-h > Compiled and booted on the Lenovo IdeaPad 330-15ARR, no dmesg regressions. -- Harsh Shandilya PRJKT Development LLC
Re: [PATCH v3 3/3] ALSA: hda: add support for Huawei WMI micmute LED
On Tue, 20 Nov 2018 00:57:13 +0100, Pavel Machek wrote: > > > +#if IS_ENABLED(CONFIG_HUAWEI_LAPTOP) > > +#include > > + > > +static int (*huawei_wmi_micmute_led_set_func)(bool); > > + > > So we should not be doing this. > > Thinkpad ACPI module exports its LEDs there, for example. Both thinkpad_acpi and dell_laptop provide the interface to HD-audio in the very same way like this. thanks, Takashi
Re: [PATCH v3 3/3] ALSA: hda: add support for Huawei WMI micmute LED
On Tue, 20 Nov 2018 00:57:13 +0100, Pavel Machek wrote: > > > +#if IS_ENABLED(CONFIG_HUAWEI_LAPTOP) > > +#include > > + > > +static int (*huawei_wmi_micmute_led_set_func)(bool); > > + > > So we should not be doing this. > > Thinkpad ACPI module exports its LEDs there, for example. Both thinkpad_acpi and dell_laptop provide the interface to HD-audio in the very same way like this. thanks, Takashi
Re: [PATCH v2 0/3] Fixes for Tegra soctherm
Hi all, Do you have any comments on this serial? Thanks. Wei. On 13/11/2018 6:06 PM, Wei Ni wrote: > This series fixed some issues for Tegra soctherm > > Main changes from v1: > 1. Acked by Thierry Reding for the patch > "thermal: tegra: fix memory allocation". > 2. Print out the sensor name when register failed. > 2. Remove patch "thermal: tegra: fix coverity defect" > > Wei Ni (3): > thermal: tegra: continue if sensor register fails > thermal: tegra: remove unnecessary warnings > thermal: tegra: fix memory allocation > > drivers/thermal/tegra/soctherm.c | 16 +--- > 1 file changed, 9 insertions(+), 7 deletions(-) >
Re: [PATCH v2 0/3] Fixes for Tegra soctherm
Hi all, Do you have any comments on this serial? Thanks. Wei. On 13/11/2018 6:06 PM, Wei Ni wrote: > This series fixed some issues for Tegra soctherm > > Main changes from v1: > 1. Acked by Thierry Reding for the patch > "thermal: tegra: fix memory allocation". > 2. Print out the sensor name when register failed. > 2. Remove patch "thermal: tegra: fix coverity defect" > > Wei Ni (3): > thermal: tegra: continue if sensor register fails > thermal: tegra: remove unnecessary warnings > thermal: tegra: fix memory allocation > > drivers/thermal/tegra/soctherm.c | 16 +--- > 1 file changed, 9 insertions(+), 7 deletions(-) >
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Monday, November 19, 2018 1:33 PM > To: Naga Sureshkumar Relli > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > computersforpe...@gmail.com; marek.va...@gmail.com; > linux-...@lists.infradead.org; linux- > ker...@vger.kernel.org; nagasures...@gmail.com; r...@kernel.org; Michal Simek > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan NAND > Flash Controller > > On Mon, 19 Nov 2018 06:20:28 + > Naga Sureshkumar Relli wrote: > > > H Boris, > > > > > -Original Message- > > > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > > > Sent: Monday, November 19, 2018 1:13 AM > > > To: Naga Sureshkumar Relli > > > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > > > computersforpe...@gmail.com; marek.va...@gmail.com; > > > linux-...@lists.infradead.org; linux- ker...@vger.kernel.org; > > > nagasures...@gmail.com; r...@kernel.org; Michal Simek > > > > > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support > > > for Arasan NAND Flash Controller > > > > > > On Thu, 15 Nov 2018 09:34:16 + > > > Naga Sureshkumar Relli wrote: > > > > > > > Hi Boris & Miquel, > > > > > > > > I am updating the driver by addressing your comments, and I have > > > > one concern, especially in anfc_read_page_hwecc(), there I am checking > > > > for erased pages > bit flips. > > > > Since Arasan NAND controller doesn't have multibit error detection > > > > beyond 24-bit( it can correct up to 24 bit), i.e. there is no > > > > indication from controller to detect > > > uncorrectable error beyond 24bit. > > > > > > Do you mean that you can't detect uncorrectable errors, or just that > > > it's not 100% sure to detect errors above max_strength? > > Yes, in Arasan NAND controller there is no way to detect uncorrectable > > errors beyond 24- > bit. > > So how do you detect uncorrectable errors when the strength is less than > 24bits? Below or equal to the level of ECC strength, controller will definitely correct. But beyond the level of ECC strength, it won't even detect the errors. > > > > > > > > So I took some error count as default value(MULTI_BIT_ERR_CNT 16, I > > > > put this based on the error count that I got while reading erased page > > > > on Micron device). > > > > And during a page read, will just read the error count register and > > > > compare this value with the default error count(16) and if it is more > > > > Than default then I > am > > > checking for erased page bit flips. > > > > > > Hm, that's wrong, especially if you set ecc_strength to something > 16. > > Ok > > > > > > > I am doubting that this will not work in all cases. > > > > > > It definitely doesn't. > > Ok > > > > > > > In my case it is just working because the error count that it got on an > > > > erased page is 16. > > > > Could you please suggest a way to do detect erased_page bit flips when > > > > reading a page > with > > > HW-ECC?. > > > > > > I'm a bit lost. Is the problem only about bitflips in erase pages, or is > > > it also impacting reads > of > > > written pages that lead to uncorrectable errors. > > Yes, it is for both. But in case of read errors that we can't detect beyond > > 24-bit, then the > answer from HW design team > > Is that the flash part is bad. > > Unfortunately till now we haven't ran into that situation(read errors of > > written pages beyond > 24-bit). > > Can you please run nandbiterrs (availaible in mtd-utils). I fear your > device won't pass the test. Yes, nandbiterror test is passing till 24bit, after that it is failing. > > > But we are hitting this because of erased page reading(needed in case of > > ubifs). > > > > > > > > Don't you have a bit (or several bits) reporting when the ECC engine was > > > not able to > correct > > > data? I you do, you should base the "detect bitflips in erase pages" > > > logic on this information. > > Bit reporting for several bit errors is there only for Hamming(1bit > > correction and 2bit > detection) but not in BCH. > > > > Then I tend to agree with Miquel: your ECC engine is broken, and I'm > not even sure how to deal with that yet. So as per the Miquel's suggestion, can I proceed to add the below one? "you should re-read the page in raw mode and check for the number of bitflips manually (thanks to the helpers in the core). Again, if the number of BF is above 16, we can assume the page is bad and increment ->ecc.failed accordingly." Thanks, Naga Sureshkumar Relli
RE: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for Arasan NAND Flash Controller
Hi Boris, > -Original Message- > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > Sent: Monday, November 19, 2018 1:33 PM > To: Naga Sureshkumar Relli > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > computersforpe...@gmail.com; marek.va...@gmail.com; > linux-...@lists.infradead.org; linux- > ker...@vger.kernel.org; nagasures...@gmail.com; r...@kernel.org; Michal Simek > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support for > Arasan NAND > Flash Controller > > On Mon, 19 Nov 2018 06:20:28 + > Naga Sureshkumar Relli wrote: > > > H Boris, > > > > > -Original Message- > > > From: Boris Brezillon [mailto:boris.brezil...@bootlin.com] > > > Sent: Monday, November 19, 2018 1:13 AM > > > To: Naga Sureshkumar Relli > > > Cc: miquel.ray...@bootlin.com; rich...@nod.at; dw...@infradead.org; > > > computersforpe...@gmail.com; marek.va...@gmail.com; > > > linux-...@lists.infradead.org; linux- ker...@vger.kernel.org; > > > nagasures...@gmail.com; r...@kernel.org; Michal Simek > > > > > > Subject: Re: [LINUX PATCH v12 3/3] mtd: rawnand: arasan: Add support > > > for Arasan NAND Flash Controller > > > > > > On Thu, 15 Nov 2018 09:34:16 + > > > Naga Sureshkumar Relli wrote: > > > > > > > Hi Boris & Miquel, > > > > > > > > I am updating the driver by addressing your comments, and I have > > > > one concern, especially in anfc_read_page_hwecc(), there I am checking > > > > for erased pages > bit flips. > > > > Since Arasan NAND controller doesn't have multibit error detection > > > > beyond 24-bit( it can correct up to 24 bit), i.e. there is no > > > > indication from controller to detect > > > uncorrectable error beyond 24bit. > > > > > > Do you mean that you can't detect uncorrectable errors, or just that > > > it's not 100% sure to detect errors above max_strength? > > Yes, in Arasan NAND controller there is no way to detect uncorrectable > > errors beyond 24- > bit. > > So how do you detect uncorrectable errors when the strength is less than > 24bits? Below or equal to the level of ECC strength, controller will definitely correct. But beyond the level of ECC strength, it won't even detect the errors. > > > > > > > > So I took some error count as default value(MULTI_BIT_ERR_CNT 16, I > > > > put this based on the error count that I got while reading erased page > > > > on Micron device). > > > > And during a page read, will just read the error count register and > > > > compare this value with the default error count(16) and if it is more > > > > Than default then I > am > > > checking for erased page bit flips. > > > > > > Hm, that's wrong, especially if you set ecc_strength to something > 16. > > Ok > > > > > > > I am doubting that this will not work in all cases. > > > > > > It definitely doesn't. > > Ok > > > > > > > In my case it is just working because the error count that it got on an > > > > erased page is 16. > > > > Could you please suggest a way to do detect erased_page bit flips when > > > > reading a page > with > > > HW-ECC?. > > > > > > I'm a bit lost. Is the problem only about bitflips in erase pages, or is > > > it also impacting reads > of > > > written pages that lead to uncorrectable errors. > > Yes, it is for both. But in case of read errors that we can't detect beyond > > 24-bit, then the > answer from HW design team > > Is that the flash part is bad. > > Unfortunately till now we haven't ran into that situation(read errors of > > written pages beyond > 24-bit). > > Can you please run nandbiterrs (availaible in mtd-utils). I fear your > device won't pass the test. Yes, nandbiterror test is passing till 24bit, after that it is failing. > > > But we are hitting this because of erased page reading(needed in case of > > ubifs). > > > > > > > > Don't you have a bit (or several bits) reporting when the ECC engine was > > > not able to > correct > > > data? I you do, you should base the "detect bitflips in erase pages" > > > logic on this information. > > Bit reporting for several bit errors is there only for Hamming(1bit > > correction and 2bit > detection) but not in BCH. > > > > Then I tend to agree with Miquel: your ECC engine is broken, and I'm > not even sure how to deal with that yet. So as per the Miquel's suggestion, can I proceed to add the below one? "you should re-read the page in raw mode and check for the number of bitflips manually (thanks to the helpers in the core). Again, if the number of BF is above 16, we can assume the page is bad and increment ->ecc.failed accordingly." Thanks, Naga Sureshkumar Relli
RE: [PATCH] thermal: imx: fix for dependency on cpu-freq
Gentle Ping...
Best Regards!
Anson Huang
> -Original Message-
> From: Anson Huang
> Sent: 2018年10月24日 14:40
> To: rui.zh...@intel.com; edubez...@gmail.com; linux...@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Cc: dl-linux-imx
> Subject: [PATCH] thermal: imx: fix for dependency on cpu-freq
>
> The thermal driver is a standalone driver for monitoring SoC temperature by
> enabling thermal sensor, so it can be enabled even when CONFIG_CPU_FREQ is
> NOT set. So remove the dependency with CPU_THERMAL.
>
> Add CONFIG_CPU_FREQ check for cpu-freq related operation in thermal driver
> to make thermal driver probe successfully when CONFIG_CPU_FREQ is NOT
> set.
>
> Signed-off-by: Anson Huang
> ---
> drivers/thermal/Kconfig | 2 +-
> drivers/thermal/imx_thermal.c | 4
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index
> 1775d44..c8a6352 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -212,7 +212,7 @@ config HISI_THERMAL
>
> config IMX_THERMAL
> tristate "Temperature sensor driver for Freescale i.MX SoCs"
> - depends on (ARCH_MXC && CPU_THERMAL) || COMPILE_TEST
> + depends on ARCH_MXC || COMPILE_TEST
> depends on NVMEM || !NVMEM
> depends on MFD_SYSCON
> depends on OF
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 1566154..87386d1 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -648,6 +648,7 @@ static const struct of_device_id
> of_imx_thermal_match[] = { }; MODULE_DEVICE_TABLE(of,
> of_imx_thermal_match);
>
> +#ifdef CONFIG_CPU_FREQ
> /*
> * Create cooling device in case no #cooling-cells property is available in
> * CPU node
> @@ -668,6 +669,7 @@ static int imx_thermal_register_legacy_cooling(struct
> imx_thermal_data *data)
>
> return 0;
> }
> +#endif
>
> static int imx_thermal_probe(struct platform_device *pdev) { @@ -743,6
> +745,7 @@ static int imx_thermal_probe(struct platform_device *pdev)
> regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
>data->socdata->power_down_mask);
>
> +#ifdef CONFIG_CPU_FREQ
> data->policy = cpufreq_cpu_get(0);
> if (!data->policy) {
> pr_debug("%s: CPUFreq policy not found\n", __func__); @@ -755,6
> +758,7 @@ static int imx_thermal_probe(struct platform_device *pdev)
> "failed to register cpufreq cooling device: %d\n", ret);
> return ret;
> }
> +#endif
>
> data->thermal_clk = devm_clk_get(>dev, NULL);
> if (IS_ERR(data->thermal_clk)) {
> --
> 2.7.4
RE: [PATCH] thermal: imx: fix for dependency on cpu-freq
Gentle Ping...
Best Regards!
Anson Huang
> -Original Message-
> From: Anson Huang
> Sent: 2018年10月24日 14:40
> To: rui.zh...@intel.com; edubez...@gmail.com; linux...@vger.kernel.org;
> linux-kernel@vger.kernel.org
> Cc: dl-linux-imx
> Subject: [PATCH] thermal: imx: fix for dependency on cpu-freq
>
> The thermal driver is a standalone driver for monitoring SoC temperature by
> enabling thermal sensor, so it can be enabled even when CONFIG_CPU_FREQ is
> NOT set. So remove the dependency with CPU_THERMAL.
>
> Add CONFIG_CPU_FREQ check for cpu-freq related operation in thermal driver
> to make thermal driver probe successfully when CONFIG_CPU_FREQ is NOT
> set.
>
> Signed-off-by: Anson Huang
> ---
> drivers/thermal/Kconfig | 2 +-
> drivers/thermal/imx_thermal.c | 4
> 2 files changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/thermal/Kconfig b/drivers/thermal/Kconfig index
> 1775d44..c8a6352 100644
> --- a/drivers/thermal/Kconfig
> +++ b/drivers/thermal/Kconfig
> @@ -212,7 +212,7 @@ config HISI_THERMAL
>
> config IMX_THERMAL
> tristate "Temperature sensor driver for Freescale i.MX SoCs"
> - depends on (ARCH_MXC && CPU_THERMAL) || COMPILE_TEST
> + depends on ARCH_MXC || COMPILE_TEST
> depends on NVMEM || !NVMEM
> depends on MFD_SYSCON
> depends on OF
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 1566154..87386d1 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
> @@ -648,6 +648,7 @@ static const struct of_device_id
> of_imx_thermal_match[] = { }; MODULE_DEVICE_TABLE(of,
> of_imx_thermal_match);
>
> +#ifdef CONFIG_CPU_FREQ
> /*
> * Create cooling device in case no #cooling-cells property is available in
> * CPU node
> @@ -668,6 +669,7 @@ static int imx_thermal_register_legacy_cooling(struct
> imx_thermal_data *data)
>
> return 0;
> }
> +#endif
>
> static int imx_thermal_probe(struct platform_device *pdev) { @@ -743,6
> +745,7 @@ static int imx_thermal_probe(struct platform_device *pdev)
> regmap_write(map, data->socdata->sensor_ctrl + REG_SET,
>data->socdata->power_down_mask);
>
> +#ifdef CONFIG_CPU_FREQ
> data->policy = cpufreq_cpu_get(0);
> if (!data->policy) {
> pr_debug("%s: CPUFreq policy not found\n", __func__); @@ -755,6
> +758,7 @@ static int imx_thermal_probe(struct platform_device *pdev)
> "failed to register cpufreq cooling device: %d\n", ret);
> return ret;
> }
> +#endif
>
> data->thermal_clk = devm_clk_get(>dev, NULL);
> if (IS_ERR(data->thermal_clk)) {
> --
> 2.7.4
WE NEED YOUR HELP FOR A BUSINESS INVESTMENT !!!
Hello, My client is seeking a US$100 Million private investment diversification in a viable business in your region with an Individual like you. This offer will require your technical and managerial partnership. Should you be inclined to pursue this offer, Contact me for detailed information. Regards, Khaled Salim Financial Consultant --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus
WE NEED YOUR HELP FOR A BUSINESS INVESTMENT !!!
Hello, My client is seeking a US$100 Million private investment diversification in a viable business in your region with an Individual like you. This offer will require your technical and managerial partnership. Should you be inclined to pursue this offer, Contact me for detailed information. Regards, Khaled Salim Financial Consultant --- This email has been checked for viruses by Avast antivirus software. https://www.avast.com/antivirus
Re: KASAN: use-after-free Read in locks_delete_block
On Sat, Nov 17, 2018 at 3:03 PM, Bruce Fields wrote: > On Sat, Nov 17, 2018 at 08:33:27AM -0500, Jeff Layton wrote: >> Thanks for the explanation, Dmitry. I've added the tag to the patch in >> my tree. It should show up in linux-next soon. >> >> I still find it a little misleading to say that syzbot reported a bug >> when it actually found a bug inside an earlier version of the patch, but >> I'll just learn to get over it. > > The usual tag for someone that found a bug in an earlier version of a > patch would be Reviewed-by:. Is there any reason we can't use that > here? The "syzbot+..." email should be enough on its own, I can't see a > reason why their scripts would need to require a particular tag. Or > maybe we could use Tested-by:, or some other tag made up for this case? > > I do worry that someone who sees "Reported-by:..." might for example > mistakenly assume that it would help to backport that patch if they see > a similar-looking oops. I see. It may also be picked by scripts that detects patches that need to be backported to stable because of the "Reported-by: syzbot" tag. This is somewhat unfortunate. There is no problem parsing another tag on syzbot side. Does Tested-by look good to you? If it found a bug in the patch and then it was fixed, Tested-by looks reasonable. And we also detect Reported-and-tested-by already because that's what syzbot suggests after it tested a proposed fix for a bug. I am somewhat concerned how to spread this information across all kernel developers. There is effectively no way to do this. We can't expect people to read docs, they generally don't. I guess I just document this at "See https://goo.gl/tpsmEJ for more information" and then we can point other people there if/when this concern pops up again.
Re: KASAN: use-after-free Read in locks_delete_block
On Sat, Nov 17, 2018 at 3:03 PM, Bruce Fields wrote: > On Sat, Nov 17, 2018 at 08:33:27AM -0500, Jeff Layton wrote: >> Thanks for the explanation, Dmitry. I've added the tag to the patch in >> my tree. It should show up in linux-next soon. >> >> I still find it a little misleading to say that syzbot reported a bug >> when it actually found a bug inside an earlier version of the patch, but >> I'll just learn to get over it. > > The usual tag for someone that found a bug in an earlier version of a > patch would be Reviewed-by:. Is there any reason we can't use that > here? The "syzbot+..." email should be enough on its own, I can't see a > reason why their scripts would need to require a particular tag. Or > maybe we could use Tested-by:, or some other tag made up for this case? > > I do worry that someone who sees "Reported-by:..." might for example > mistakenly assume that it would help to backport that patch if they see > a similar-looking oops. I see. It may also be picked by scripts that detects patches that need to be backported to stable because of the "Reported-by: syzbot" tag. This is somewhat unfortunate. There is no problem parsing another tag on syzbot side. Does Tested-by look good to you? If it found a bug in the patch and then it was fixed, Tested-by looks reasonable. And we also detect Reported-and-tested-by already because that's what syzbot suggests after it tested a proposed fix for a bug. I am somewhat concerned how to spread this information across all kernel developers. There is effectively no way to do this. We can't expect people to read docs, they generally don't. I guess I just document this at "See https://goo.gl/tpsmEJ for more information" and then we can point other people there if/when this concern pops up again.
make[2]: *** No rule to make target 'arch/sh/boot/dts/.dtb.S', needed by 'arch/sh/boot/dts/.dtb.o'.
Hi Rob, FYI, the error/warning still remains. tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: f2ce1065e767fc7da106a5f5381d1e8f842dc6f4 commit: 37c8a5fafa3bb7dcdd51774be353be6cb2912b86 kbuild: consolidate Devicetree dtb build rules date: 7 weeks ago config: sh-j2_defconfig (attached as .config) compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 37c8a5fafa3bb7dcdd51774be353be6cb2912b86 # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=sh All errors (new ones prefixed by >>): >> make[2]: *** No rule to make target 'arch/sh/boot/dts/.dtb.S', needed by >> 'arch/sh/boot/dts/.dtb.o'. make[2]: Target '__build' not remade because of errors. --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
make[2]: *** No rule to make target 'arch/sh/boot/dts/.dtb.S', needed by 'arch/sh/boot/dts/.dtb.o'.
Hi Rob, FYI, the error/warning still remains. tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: f2ce1065e767fc7da106a5f5381d1e8f842dc6f4 commit: 37c8a5fafa3bb7dcdd51774be353be6cb2912b86 kbuild: consolidate Devicetree dtb build rules date: 7 weeks ago config: sh-j2_defconfig (attached as .config) compiler: sh4-linux-gnu-gcc (Debian 7.2.0-11) 7.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 37c8a5fafa3bb7dcdd51774be353be6cb2912b86 # save the attached .config to linux build tree GCC_VERSION=7.2.0 make.cross ARCH=sh All errors (new ones prefixed by >>): >> make[2]: *** No rule to make target 'arch/sh/boot/dts/.dtb.S', needed by >> 'arch/sh/boot/dts/.dtb.o'. make[2]: Target '__build' not remade because of errors. --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
RE: [RFC v4 0/2] WhiteEgret LSM module
We appreciate your comments.
We refine source code according to your comments.
>> This is an interesting idea, and an evolution since the initial
>> approach which was submitted based upon xattr attributes. I still
>> find the idea of using attributes simpler to manage though, since
>> they're easy to add, and audit for.
>>
>> I suspect the biggest objection to this module is that maintaining a
>> whitelist at all is often impractical.
>
>If existing implementations were perfect enough for everyone, we would not
>have done a lot of trial and error. ;-)
>
>>
>> My (trivial/toy/silly) module went the attribute-reading way:
>>
>> https://github.com/skx/linux-security-modules/tree/master/security/whi
>> telist
>>
>> For a completely crazy take upon the idea of userspace decisions
>> though you can laugh at my attempt here:
>>
>> https://github.com/skx/linux-security-modules/tree/master/security/can
>> -exec
>>
>> I callback to userspace for every decision, via
>> call_usermodehelper_setup. The crazy part is that it actually works
>> at all!
>>
>> Steve
>
>Oh, checking only execve() requests? I have implemented it (using AKARI as a
>codebase) which does on-access ClamAV scan, without using
>call_usermodehelper().
>The tutorial is (written in Japanese language) available at
>http://I-love.SAKURA.ne.jp/tomoyo/scamp2017-kumaneko.html . Since
>WhiteEgret is using a daemon rather than one-time agent, I'm sure that this
>example implementation helps how to make WhiteEgret interface robust.
Thank you for introducing useful information.
>
>
>
>Here begins the feedback.
>
>(1) Please drop module exit functions, for currently it is impossible to unload
>"loadable kernel module based LSM modules".
>
>(2) Please add __init to all init functions, for it helps finding redundant
>code.
>
>(3) Please test with CONFIG_PROVE_LOCKING=y, for there is a deadlock bug
>which
>lockdep can find.
>
>(4) Please test with CONFIG_KASAN=y, for there is a use-after-free bug.
>
>(5) Please test with CONFIG_DEBUG_ATOMIC_SLEEP=y, for there is a
>sleep-in-atomic bug.
We use to fix bugs with these kernel options.
>
>(6) Please avoid redundant NULL checks.
>
>(7) Please annotate __user to userspace pointers.
>
>(8) Please do check size when copying kernel <=> user memory.
>
>(9) Setting "struct file_operations"->owner is pointless, for currently it is
>impossible to unload "loadable kernel module based LSM modules".
>
>(10) Please check the compiler output, for there is an incompatible argument
>warning.
>
>(11) Please don't try to defer pending signals. If a process got a fatal
>signal,
> the process should be able to terminate as soon as possible.
We concern whether the LSM can restart system calls in any situation.
We study behavior of restarting system calls, and we reconsider way of handling
signals.
>
>(12) Please understand when "struct file_operations"->open/release hooks are
>called,
> for current "from_task" approach is not robust (and hence the tutorial
>above).
We improve our LSM implementation to be compatible with various WEUA
implementations.
>
>
>
>A delta diff is shown below, but you don't want to apply it as-is.
>
>diff --git a/security/whiteegret/init.c b/security/whiteegret/init.c index
>b78f581..c447655 100644
>--- a/security/whiteegret/init.c
>+++ b/security/whiteegret/init.c
>@@ -23,9 +23,6 @@
>
> static int we_security_bprm_check(struct linux_binprm *bprm) {
>- if (!bprm)
>- return 0;
>-
> if (we_security_bprm_check_main(bprm) == -EACCES)
> return -EACCES;
>
>@@ -48,8 +45,6 @@ static int we_security_mmap_check(struct file *file,
>unsigned long reqprot,
> defined(CONFIG_SECURITY_WHITEEGRET_HOOK_FILE_WRITE)
> static int we_security_access_check(struct file *file, int mask) {
>- if (!file)
>- return 0;
> return we_security_access_check_main(file, mask); } #endif @@
>-58,23 +53,18 @@ static int we_security_access_check(struct file *file, int
>mask)
> defined(CONFIG_SECURITY_WHITEEGRET_HOOK_WRITE_OPEN)
> static int we_security_open_check(struct file *file) {
>- if (!file)
>- return 0;
> return we_security_open_check_main(file);
> }
> #endif
>
> #ifdef CONFIG_SECURITY_WHITEEGRET_HOOK_WRITE
>-static int we_security_rename_check(struct path *old_dir,
>+static int we_security_rename_check(const struct path *old_dir,
> struct dentry *old_dentry,
>- struct path *new_dir,
>+ const struct path *new_dir,
> struct dentry *new_dentry)
> {
> struct path new_path;
>
>- if (!new_dir)
>- return 0;
>-
> new_path.mnt = new_dir->mnt;
> new_path.dentry = new_dentry;
> return we_security_rename_check_main(_path);
>@@ -85,17 +75,11 @@ static int we_security_rename_check(struct path
>*old_dir, static int we_task_alloc_check(struct
RE: [RFC v4 0/2] WhiteEgret LSM module
We appreciate your comments.
We refine source code according to your comments.
>> This is an interesting idea, and an evolution since the initial
>> approach which was submitted based upon xattr attributes. I still
>> find the idea of using attributes simpler to manage though, since
>> they're easy to add, and audit for.
>>
>> I suspect the biggest objection to this module is that maintaining a
>> whitelist at all is often impractical.
>
>If existing implementations were perfect enough for everyone, we would not
>have done a lot of trial and error. ;-)
>
>>
>> My (trivial/toy/silly) module went the attribute-reading way:
>>
>> https://github.com/skx/linux-security-modules/tree/master/security/whi
>> telist
>>
>> For a completely crazy take upon the idea of userspace decisions
>> though you can laugh at my attempt here:
>>
>> https://github.com/skx/linux-security-modules/tree/master/security/can
>> -exec
>>
>> I callback to userspace for every decision, via
>> call_usermodehelper_setup. The crazy part is that it actually works
>> at all!
>>
>> Steve
>
>Oh, checking only execve() requests? I have implemented it (using AKARI as a
>codebase) which does on-access ClamAV scan, without using
>call_usermodehelper().
>The tutorial is (written in Japanese language) available at
>http://I-love.SAKURA.ne.jp/tomoyo/scamp2017-kumaneko.html . Since
>WhiteEgret is using a daemon rather than one-time agent, I'm sure that this
>example implementation helps how to make WhiteEgret interface robust.
Thank you for introducing useful information.
>
>
>
>Here begins the feedback.
>
>(1) Please drop module exit functions, for currently it is impossible to unload
>"loadable kernel module based LSM modules".
>
>(2) Please add __init to all init functions, for it helps finding redundant
>code.
>
>(3) Please test with CONFIG_PROVE_LOCKING=y, for there is a deadlock bug
>which
>lockdep can find.
>
>(4) Please test with CONFIG_KASAN=y, for there is a use-after-free bug.
>
>(5) Please test with CONFIG_DEBUG_ATOMIC_SLEEP=y, for there is a
>sleep-in-atomic bug.
We use to fix bugs with these kernel options.
>
>(6) Please avoid redundant NULL checks.
>
>(7) Please annotate __user to userspace pointers.
>
>(8) Please do check size when copying kernel <=> user memory.
>
>(9) Setting "struct file_operations"->owner is pointless, for currently it is
>impossible to unload "loadable kernel module based LSM modules".
>
>(10) Please check the compiler output, for there is an incompatible argument
>warning.
>
>(11) Please don't try to defer pending signals. If a process got a fatal
>signal,
> the process should be able to terminate as soon as possible.
We concern whether the LSM can restart system calls in any situation.
We study behavior of restarting system calls, and we reconsider way of handling
signals.
>
>(12) Please understand when "struct file_operations"->open/release hooks are
>called,
> for current "from_task" approach is not robust (and hence the tutorial
>above).
We improve our LSM implementation to be compatible with various WEUA
implementations.
>
>
>
>A delta diff is shown below, but you don't want to apply it as-is.
>
>diff --git a/security/whiteegret/init.c b/security/whiteegret/init.c index
>b78f581..c447655 100644
>--- a/security/whiteegret/init.c
>+++ b/security/whiteegret/init.c
>@@ -23,9 +23,6 @@
>
> static int we_security_bprm_check(struct linux_binprm *bprm) {
>- if (!bprm)
>- return 0;
>-
> if (we_security_bprm_check_main(bprm) == -EACCES)
> return -EACCES;
>
>@@ -48,8 +45,6 @@ static int we_security_mmap_check(struct file *file,
>unsigned long reqprot,
> defined(CONFIG_SECURITY_WHITEEGRET_HOOK_FILE_WRITE)
> static int we_security_access_check(struct file *file, int mask) {
>- if (!file)
>- return 0;
> return we_security_access_check_main(file, mask); } #endif @@
>-58,23 +53,18 @@ static int we_security_access_check(struct file *file, int
>mask)
> defined(CONFIG_SECURITY_WHITEEGRET_HOOK_WRITE_OPEN)
> static int we_security_open_check(struct file *file) {
>- if (!file)
>- return 0;
> return we_security_open_check_main(file);
> }
> #endif
>
> #ifdef CONFIG_SECURITY_WHITEEGRET_HOOK_WRITE
>-static int we_security_rename_check(struct path *old_dir,
>+static int we_security_rename_check(const struct path *old_dir,
> struct dentry *old_dentry,
>- struct path *new_dir,
>+ const struct path *new_dir,
> struct dentry *new_dentry)
> {
> struct path new_path;
>
>- if (!new_dir)
>- return 0;
>-
> new_path.mnt = new_dir->mnt;
> new_path.dentry = new_dentry;
> return we_security_rename_check_main(_path);
>@@ -85,17 +75,11 @@ static int we_security_rename_check(struct path
>*old_dir, static int we_task_alloc_check(struct
[PATCH] debugobjects: scale the static pool size
The current value of the early boot static pool size is not big enough for systems with large number of CPUs with timer or/and workqueue objects selected. As the results, systems have 60+ CPUs with both timer and workqueue objects enabled could trigger "ODEBUG: Out of memory. ODEBUG disabled". Hence, fixed it by computing it according to CONFIG_NR_CPUS and CONFIG_DEBUG_OBJECTS_* options. Signed-off-by: Qian Cai --- lib/debugobjects.c | 53 +- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/lib/debugobjects.c b/lib/debugobjects.c index 70935ed91125..372dc34206d5 100644 --- a/lib/debugobjects.c +++ b/lib/debugobjects.c @@ -23,7 +23,53 @@ #define ODEBUG_HASH_BITS 14 #define ODEBUG_HASH_SIZE (1 << ODEBUG_HASH_BITS) +/* + * Some debug objects are allocated during the early boot. Enabling some + * options like timers or workqueue objects may increase the size required + * significantly with large number of CPUs. For example, + * + * No. CPUs x 2 (worker pool) objects: + * + * start_kernel + * workqueue_init_early + * init_worker_pool + * init_timer_key + * debug_object_init + * + * No. CPUs objects (CONFIG_HIGH_RES_TIMERS): + * + * sched_init + * hrtick_rq_init + * hrtimer_init + * + * CONFIG_DEBUG_OBJECTS_WORK: + * No. CPUs x 6 (workqueue) objects: + * + * workqueue_init_early + * alloc_workqueue + * __alloc_workqueue_key + * alloc_and_link_pwqs + * init_pwq + * + * Also, plus No. CPUs objects: + * + * perf_event_init + *__init_srcu_struct + * init_srcu_struct_fields + *init_srcu_struct_nodes + * __init_work + * + * Increase the number a bit more in case the implmentatins are changed in + * the future. + */ +#if defined(CONFIG_NR_CPUS) && defined(CONFIG_DEBUG_OBJECTS_TIMERS) && \ +!defined(CONFIG_DEBUG_OBJECTS_WORK) +#define ODEBUG_POOL_SIZE (CONFIG_NR_CPUS * 10) +#elif defined(CONFIG_NR_CPUS) && defined(CONFIG_DEBUG_OBJECTS_WORK) +#define ODEBUG_POOL_SIZE (CONFIG_NR_CPUS * 30) +#else #define ODEBUG_POOL_SIZE 1024 +#endif /* CONFIG_NR_CPUS */ #define ODEBUG_POOL_MIN_LEVEL 256 #define ODEBUG_CHUNK_SHIFT PAGE_SHIFT @@ -58,8 +104,13 @@ static int debug_objects_fixups __read_mostly; static int debug_objects_warnings __read_mostly; static int debug_objects_enabled __read_mostly = CONFIG_DEBUG_OBJECTS_ENABLE_DEFAULT; +/* + * This is only used after replaced static objects, so no need to scale it + * to use the early boot static pool size and it has already been scaled + * according to actual No. CPUs in the box within debug_objects_mem_init(). + */ static int debug_objects_pool_size __read_mostly - = ODEBUG_POOL_SIZE; + = 1024; static int debug_objects_pool_min_level __read_mostly = ODEBUG_POOL_MIN_LEVEL; static struct debug_obj_descr *descr_test __read_mostly; -- 2.17.2 (Apple Git-113)
[PATCH] debugobjects: scale the static pool size
The current value of the early boot static pool size is not big enough for systems with large number of CPUs with timer or/and workqueue objects selected. As the results, systems have 60+ CPUs with both timer and workqueue objects enabled could trigger "ODEBUG: Out of memory. ODEBUG disabled". Hence, fixed it by computing it according to CONFIG_NR_CPUS and CONFIG_DEBUG_OBJECTS_* options. Signed-off-by: Qian Cai --- lib/debugobjects.c | 53 +- 1 file changed, 52 insertions(+), 1 deletion(-) diff --git a/lib/debugobjects.c b/lib/debugobjects.c index 70935ed91125..372dc34206d5 100644 --- a/lib/debugobjects.c +++ b/lib/debugobjects.c @@ -23,7 +23,53 @@ #define ODEBUG_HASH_BITS 14 #define ODEBUG_HASH_SIZE (1 << ODEBUG_HASH_BITS) +/* + * Some debug objects are allocated during the early boot. Enabling some + * options like timers or workqueue objects may increase the size required + * significantly with large number of CPUs. For example, + * + * No. CPUs x 2 (worker pool) objects: + * + * start_kernel + * workqueue_init_early + * init_worker_pool + * init_timer_key + * debug_object_init + * + * No. CPUs objects (CONFIG_HIGH_RES_TIMERS): + * + * sched_init + * hrtick_rq_init + * hrtimer_init + * + * CONFIG_DEBUG_OBJECTS_WORK: + * No. CPUs x 6 (workqueue) objects: + * + * workqueue_init_early + * alloc_workqueue + * __alloc_workqueue_key + * alloc_and_link_pwqs + * init_pwq + * + * Also, plus No. CPUs objects: + * + * perf_event_init + *__init_srcu_struct + * init_srcu_struct_fields + *init_srcu_struct_nodes + * __init_work + * + * Increase the number a bit more in case the implmentatins are changed in + * the future. + */ +#if defined(CONFIG_NR_CPUS) && defined(CONFIG_DEBUG_OBJECTS_TIMERS) && \ +!defined(CONFIG_DEBUG_OBJECTS_WORK) +#define ODEBUG_POOL_SIZE (CONFIG_NR_CPUS * 10) +#elif defined(CONFIG_NR_CPUS) && defined(CONFIG_DEBUG_OBJECTS_WORK) +#define ODEBUG_POOL_SIZE (CONFIG_NR_CPUS * 30) +#else #define ODEBUG_POOL_SIZE 1024 +#endif /* CONFIG_NR_CPUS */ #define ODEBUG_POOL_MIN_LEVEL 256 #define ODEBUG_CHUNK_SHIFT PAGE_SHIFT @@ -58,8 +104,13 @@ static int debug_objects_fixups __read_mostly; static int debug_objects_warnings __read_mostly; static int debug_objects_enabled __read_mostly = CONFIG_DEBUG_OBJECTS_ENABLE_DEFAULT; +/* + * This is only used after replaced static objects, so no need to scale it + * to use the early boot static pool size and it has already been scaled + * according to actual No. CPUs in the box within debug_objects_mem_init(). + */ static int debug_objects_pool_size __read_mostly - = ODEBUG_POOL_SIZE; + = 1024; static int debug_objects_pool_min_level __read_mostly = ODEBUG_POOL_MIN_LEVEL; static struct debug_obj_descr *descr_test __read_mostly; -- 2.17.2 (Apple Git-113)
Re: [PATCH 1/3] ARM: davinci: define gpio interrupts as separate resources
On 11/20/2018 2:22 AM, Sekhar Nori wrote:
On 13/11/18 7:20 PM, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski
Since commit eb3744a2dd01 ("gpio: davinci: Do not assume continuous
IRQ numbering") the davinci GPIO driver fails to probe if we boot
in legacy mode from any of the board files. Since the driver now
expects every interrupt to be defined as a separate resource, split
the definition in devices-da8xx.c instead of having a single continuous
interrupt range.
Fixes: eb3744a2dd01 ("gpio: davinci: Do not assume continuous IRQ numbering")
Cc: sta...@vger.kernel.org
Signed-off-by: Bartosz Golaszewski
There are a number of other boards that need such fixing too. And the
commit in question does not do a good job of explaining why it was
needed in the first place. The description just repeats what can be
inferred by reading the patch.
Cc Lokesh
Sekhar,
DT explicitly mentions every IRQ number. The patch in discussion
explicitly calls platform_get_irq for all the interrupts which to me is
the right thing to do as: platform_get_irq-->
of_irq_get-->irq_create_of_mapping--> sequence is to be done for every IRQ.
k3-am654 definitely will need explicit calls to platform_get_irq as it
will be involving interrupt router and interrupt numbers need not be
continuous.
So i do not think reverting the patch is the right idea.
Regards,
Keerthy
gpio: davinci: Do not assume continuous IRQ numbering
Currently the driver assumes that the interrupts are continuous
and does platform_get_irq only once and assumes the rest are continuous,
instead call platform_get_irq for all the interrupts and store them
in an array for later use.
Signed-off-by: Keerthy
Reviewed-by: Grygorii Strashko
Signed-off-by: Linus Walleij
Can we revert the offending commit instead?
Thanks,
Sekhar
Re: [PATCH 1/3] ARM: davinci: define gpio interrupts as separate resources
On 11/20/2018 2:22 AM, Sekhar Nori wrote:
On 13/11/18 7:20 PM, Bartosz Golaszewski wrote:
From: Bartosz Golaszewski
Since commit eb3744a2dd01 ("gpio: davinci: Do not assume continuous
IRQ numbering") the davinci GPIO driver fails to probe if we boot
in legacy mode from any of the board files. Since the driver now
expects every interrupt to be defined as a separate resource, split
the definition in devices-da8xx.c instead of having a single continuous
interrupt range.
Fixes: eb3744a2dd01 ("gpio: davinci: Do not assume continuous IRQ numbering")
Cc: sta...@vger.kernel.org
Signed-off-by: Bartosz Golaszewski
There are a number of other boards that need such fixing too. And the
commit in question does not do a good job of explaining why it was
needed in the first place. The description just repeats what can be
inferred by reading the patch.
Cc Lokesh
Sekhar,
DT explicitly mentions every IRQ number. The patch in discussion
explicitly calls platform_get_irq for all the interrupts which to me is
the right thing to do as: platform_get_irq-->
of_irq_get-->irq_create_of_mapping--> sequence is to be done for every IRQ.
k3-am654 definitely will need explicit calls to platform_get_irq as it
will be involving interrupt router and interrupt numbers need not be
continuous.
So i do not think reverting the patch is the right idea.
Regards,
Keerthy
gpio: davinci: Do not assume continuous IRQ numbering
Currently the driver assumes that the interrupts are continuous
and does platform_get_irq only once and assumes the rest are continuous,
instead call platform_get_irq for all the interrupts and store them
in an array for later use.
Signed-off-by: Keerthy
Reviewed-by: Grygorii Strashko
Signed-off-by: Linus Walleij
Can we revert the offending commit instead?
Thanks,
Sekhar
Re: [PATCH v1 01/11] clk: mediatek: add new clkmux register API
On Tue, 2018-11-13 at 08:00 -0800, Nicolas Boichat wrote:
> On Mon, Nov 5, 2018 at 10:42 PM Weiyi Lu wrote:
> >
> > From: Owen Chen
> >
> > On both MT8183 & MT6765, there add "set/clr" register for
> > each clkmux setting, and one update register to trigger value change.
> > It is designed to prevent read-modify-write racing issue.
> > The sw design need to add a new API to handle this hw change with
> > a new mtk_clk_mux/mtk_mux struct in new file "clk-mux.c", "clk-mux.h".
> >
> > Signed-off-by: Owen Chen
> > Signed-off-by: Weiyi Lu
> > ---
> > drivers/clk/mediatek/Makefile | 2 +-
> > drivers/clk/mediatek/clk-mux.c | 252 +
> > drivers/clk/mediatek/clk-mux.h | 101 +
> > 3 files changed, 354 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/clk/mediatek/clk-mux.c
> > create mode 100644 drivers/clk/mediatek/clk-mux.h
> >
> > diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
> > index 844b55d2770d..b97980dbb738 100644
> > --- a/drivers/clk/mediatek/Makefile
> > +++ b/drivers/clk/mediatek/Makefile
> > @@ -1,5 +1,5 @@
> > # SPDX-License-Identifier: GPL-2.0
> > -obj-$(CONFIG_COMMON_CLK_MEDIATEK) += clk-mtk.o clk-pll.o clk-gate.o
> > clk-apmixed.o clk-cpumux.o reset.o
> > +obj-$(CONFIG_COMMON_CLK_MEDIATEK) += clk-mtk.o clk-pll.o clk-gate.o
> > clk-apmixed.o clk-cpumux.o reset.o clk-mux.o
> > obj-$(CONFIG_COMMON_CLK_MT6797) += clk-mt6797.o
> > obj-$(CONFIG_COMMON_CLK_MT6797_IMGSYS) += clk-mt6797-img.o
> > obj-$(CONFIG_COMMON_CLK_MT6797_MMSYS) += clk-mt6797-mm.o
> > diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
> > new file mode 100644
> > index ..18bc9a146be0
> > --- /dev/null
> > +++ b/drivers/clk/mediatek/clk-mux.c
> > @@ -0,0 +1,252 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018 MediaTek Inc.
> > + * Author: Owen Chen
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "clk-mtk.h"
> > +#include "clk-mux.h"
> > +
> > +static inline struct mtk_clk_mux
> > + *to_mtk_clk_mux(struct clk_hw *hw)
> > +{
> > + return container_of(hw, struct mtk_clk_mux, hw);
> > +}
> > +
> > +static int mtk_clk_mux_enable(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 mask = BIT(mux->gate_shift);
> > +
> > + regmap_update_bits(mux->regmap, mux->mux_ofs, mask, 0);
> > +
> > + return 0;
>
> Might as well return regmap_update_bits(...).
>
OK, will fix in next version
> > +}
> > +
> > +static void mtk_clk_mux_disable(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 mask = BIT(mux->gate_shift);
> > +
> > + regmap_update_bits(mux->regmap, mux->mux_ofs, mask, mask);
> > +}
> > +
> > +static int mtk_clk_mux_enable_setclr(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 val;
> > +
> > + val = BIT(mux->gate_shift);
> > + regmap_write(mux->regmap, mux->mux_clr_ofs, val);
> > +
> > + return 0;
>
> ditto: return regmap_write(...) .
>
will fix in next version
> > +}
> > +
> > +static void mtk_clk_mux_disable_setclr(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 val;
> > +
> > + val = BIT(mux->gate_shift);
> > + regmap_write(mux->regmap, mux->mux_set_ofs, val);
> > +}
> > +
> > +static int mtk_clk_mux_is_enabled(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 val = 0;
>
> No need to init to 0.
>
will fix in next version
> > +
> > + regmap_read(mux->regmap, mux->mux_ofs, );
> > +
> > + return ((val & BIT(mux->gate_shift)) == 0) ? 1 : 0;
>
> Just (val & BIT(mux->gate_shift)) != 0. Or more simply val &
> BIT(mux->gate_shift).
>
will fix in next version
> > +}
> > +
> > +static u8 mtk_clk_mux_get_parent(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + int num_parents = clk_hw_get_num_parents(hw);
> > + u32 mask = GENMASK(mux->mux_width - 1, 0);
> > + u32 val;
> > +
> > + regmap_read(mux->regmap, mux->mux_ofs, );
> > + val = (val >> mux->mux_shift) & mask;
> > +
> > + if (val >= num_parents || val >= U8_MAX)
>
> val > U8_MAX, technically.
>
> > + return -ERANGE;
>
> This looks wrong, -34 will be cast to an u8 and appear to be valid...
>
will fix in next version
> > +
> > + return val;
> > +}
> > +
> > +static int mtk_clk_mux_set_parent_lock(struct clk_hw *hw, u8 index)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 mask = GENMASK(mux->mux_width - 1, 0);
> > + u32 val;
> > + unsigned long flags = 0;
>
> No need to init to 0.
>
will fix in next version.
> > +
> > + if (mux->lock)
> > + spin_lock_irqsave(mux->lock, flags);
> > +
Re: [PATCH v1 01/11] clk: mediatek: add new clkmux register API
On Tue, 2018-11-13 at 08:00 -0800, Nicolas Boichat wrote:
> On Mon, Nov 5, 2018 at 10:42 PM Weiyi Lu wrote:
> >
> > From: Owen Chen
> >
> > On both MT8183 & MT6765, there add "set/clr" register for
> > each clkmux setting, and one update register to trigger value change.
> > It is designed to prevent read-modify-write racing issue.
> > The sw design need to add a new API to handle this hw change with
> > a new mtk_clk_mux/mtk_mux struct in new file "clk-mux.c", "clk-mux.h".
> >
> > Signed-off-by: Owen Chen
> > Signed-off-by: Weiyi Lu
> > ---
> > drivers/clk/mediatek/Makefile | 2 +-
> > drivers/clk/mediatek/clk-mux.c | 252 +
> > drivers/clk/mediatek/clk-mux.h | 101 +
> > 3 files changed, 354 insertions(+), 1 deletion(-)
> > create mode 100644 drivers/clk/mediatek/clk-mux.c
> > create mode 100644 drivers/clk/mediatek/clk-mux.h
> >
> > diff --git a/drivers/clk/mediatek/Makefile b/drivers/clk/mediatek/Makefile
> > index 844b55d2770d..b97980dbb738 100644
> > --- a/drivers/clk/mediatek/Makefile
> > +++ b/drivers/clk/mediatek/Makefile
> > @@ -1,5 +1,5 @@
> > # SPDX-License-Identifier: GPL-2.0
> > -obj-$(CONFIG_COMMON_CLK_MEDIATEK) += clk-mtk.o clk-pll.o clk-gate.o
> > clk-apmixed.o clk-cpumux.o reset.o
> > +obj-$(CONFIG_COMMON_CLK_MEDIATEK) += clk-mtk.o clk-pll.o clk-gate.o
> > clk-apmixed.o clk-cpumux.o reset.o clk-mux.o
> > obj-$(CONFIG_COMMON_CLK_MT6797) += clk-mt6797.o
> > obj-$(CONFIG_COMMON_CLK_MT6797_IMGSYS) += clk-mt6797-img.o
> > obj-$(CONFIG_COMMON_CLK_MT6797_MMSYS) += clk-mt6797-mm.o
> > diff --git a/drivers/clk/mediatek/clk-mux.c b/drivers/clk/mediatek/clk-mux.c
> > new file mode 100644
> > index ..18bc9a146be0
> > --- /dev/null
> > +++ b/drivers/clk/mediatek/clk-mux.c
> > @@ -0,0 +1,252 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Copyright (c) 2018 MediaTek Inc.
> > + * Author: Owen Chen
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include "clk-mtk.h"
> > +#include "clk-mux.h"
> > +
> > +static inline struct mtk_clk_mux
> > + *to_mtk_clk_mux(struct clk_hw *hw)
> > +{
> > + return container_of(hw, struct mtk_clk_mux, hw);
> > +}
> > +
> > +static int mtk_clk_mux_enable(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 mask = BIT(mux->gate_shift);
> > +
> > + regmap_update_bits(mux->regmap, mux->mux_ofs, mask, 0);
> > +
> > + return 0;
>
> Might as well return regmap_update_bits(...).
>
OK, will fix in next version
> > +}
> > +
> > +static void mtk_clk_mux_disable(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 mask = BIT(mux->gate_shift);
> > +
> > + regmap_update_bits(mux->regmap, mux->mux_ofs, mask, mask);
> > +}
> > +
> > +static int mtk_clk_mux_enable_setclr(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 val;
> > +
> > + val = BIT(mux->gate_shift);
> > + regmap_write(mux->regmap, mux->mux_clr_ofs, val);
> > +
> > + return 0;
>
> ditto: return regmap_write(...) .
>
will fix in next version
> > +}
> > +
> > +static void mtk_clk_mux_disable_setclr(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 val;
> > +
> > + val = BIT(mux->gate_shift);
> > + regmap_write(mux->regmap, mux->mux_set_ofs, val);
> > +}
> > +
> > +static int mtk_clk_mux_is_enabled(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 val = 0;
>
> No need to init to 0.
>
will fix in next version
> > +
> > + regmap_read(mux->regmap, mux->mux_ofs, );
> > +
> > + return ((val & BIT(mux->gate_shift)) == 0) ? 1 : 0;
>
> Just (val & BIT(mux->gate_shift)) != 0. Or more simply val &
> BIT(mux->gate_shift).
>
will fix in next version
> > +}
> > +
> > +static u8 mtk_clk_mux_get_parent(struct clk_hw *hw)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + int num_parents = clk_hw_get_num_parents(hw);
> > + u32 mask = GENMASK(mux->mux_width - 1, 0);
> > + u32 val;
> > +
> > + regmap_read(mux->regmap, mux->mux_ofs, );
> > + val = (val >> mux->mux_shift) & mask;
> > +
> > + if (val >= num_parents || val >= U8_MAX)
>
> val > U8_MAX, technically.
>
> > + return -ERANGE;
>
> This looks wrong, -34 will be cast to an u8 and appear to be valid...
>
will fix in next version
> > +
> > + return val;
> > +}
> > +
> > +static int mtk_clk_mux_set_parent_lock(struct clk_hw *hw, u8 index)
> > +{
> > + struct mtk_clk_mux *mux = to_mtk_clk_mux(hw);
> > + u32 mask = GENMASK(mux->mux_width - 1, 0);
> > + u32 val;
> > + unsigned long flags = 0;
>
> No need to init to 0.
>
will fix in next version.
> > +
> > + if (mux->lock)
> > + spin_lock_irqsave(mux->lock, flags);
> > +
Re: [PATCH v9 15/15] OPTIONAL: cpufreq: dt: Register an Energy Model
On 19-11-18, 14:18, Quentin Perret wrote:
> static int cpufreq_init(struct cpufreq_policy *policy)
> {
> + struct em_data_callback em_cb = EM_DATA_CB(of_est_power);
> struct cpufreq_frequency_table *freq_table;
> struct opp_table *opp_table = NULL;
> struct private_data *priv;
> @@ -160,7 +203,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
> unsigned int transition_latency;
> bool fallback = false;
> const char *name;
> - int ret;
> + int ret, nr_opp;
>
> cpu_dev = get_cpu_device(policy->cpu);
> if (!cpu_dev) {
> @@ -237,6 +280,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
> ret = -EPROBE_DEFER;
> goto out_free_opp;
> }
> + nr_opp = ret;
>
> if (fallback) {
> cpumask_setall(policy->cpus);
> @@ -280,6 +324,8 @@ static int cpufreq_init(struct cpufreq_policy *policy)
> policy->cpuinfo.transition_latency = transition_latency;
> policy->dvfs_possible_from_any_cpu = true;
>
> + em_register_perf_domain(policy->cpus, nr_opp, _cb);
> +
> return 0;
>
> out_free_cpufreq_table:
I haven't gone deep into the series, but why don't we need something
like em_unregister_perf_domain()? That can be used if the cpufreq
driver goes away. Else loading/unloading/loading the cpufreq driver
may register the perf-domain callback again.
--
viresh
Re: [PATCH v9 15/15] OPTIONAL: cpufreq: dt: Register an Energy Model
On 19-11-18, 14:18, Quentin Perret wrote:
> static int cpufreq_init(struct cpufreq_policy *policy)
> {
> + struct em_data_callback em_cb = EM_DATA_CB(of_est_power);
> struct cpufreq_frequency_table *freq_table;
> struct opp_table *opp_table = NULL;
> struct private_data *priv;
> @@ -160,7 +203,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
> unsigned int transition_latency;
> bool fallback = false;
> const char *name;
> - int ret;
> + int ret, nr_opp;
>
> cpu_dev = get_cpu_device(policy->cpu);
> if (!cpu_dev) {
> @@ -237,6 +280,7 @@ static int cpufreq_init(struct cpufreq_policy *policy)
> ret = -EPROBE_DEFER;
> goto out_free_opp;
> }
> + nr_opp = ret;
>
> if (fallback) {
> cpumask_setall(policy->cpus);
> @@ -280,6 +324,8 @@ static int cpufreq_init(struct cpufreq_policy *policy)
> policy->cpuinfo.transition_latency = transition_latency;
> policy->dvfs_possible_from_any_cpu = true;
>
> + em_register_perf_domain(policy->cpus, nr_opp, _cb);
> +
> return 0;
>
> out_free_cpufreq_table:
I haven't gone deep into the series, but why don't we need something
like em_unregister_perf_domain()? That can be used if the cpufreq
driver goes away. Else loading/unloading/loading the cpufreq driver
may register the perf-domain callback again.
--
viresh
[PATCH] ptrace: take into account saved_sigmask in PTRACE_{GET,SET}SIGMASK
There are a few system calls (pselect, ppoll, etc) which replace a task
sigmask while they are running in a kernel-space
When a task calls one of these syscalls, the kernel saves a current
sigmask in task->saved_sigmask and sets a syscall sigmask.
On syscall-exit-stop, ptrace traps a task before restoring the
saved_sigmask, so PTRACE_GETSIGMASK returns the syscall sigmask and
PTRACE_SETSIGMASK does nothing, because its sigmask is replaced by
saved_sigmask, when the task returns to user-space.
This patch fixes this problem. PTRACE_GET_SIGMASK returns saved_sigmask
is it's set. PTRACE_SETSIGMASK drops the TIF_RESTORE_SIGMASK flag.
Cc: Oleg Nesterov
Cc: "Eric W. Biederman"
Cc: Andrew Morton
Fixes: 29000caecbe8 ("ptrace: add ability to get/set signal-blocked mask")
Signed-off-by: Andrei Vagin
---
include/linux/sched/signal.h | 18 ++
kernel/ptrace.c | 15 +--
2 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 1be35729c2c5..660d78c9af6c 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -417,10 +417,20 @@ static inline void set_restore_sigmask(void)
set_thread_flag(TIF_RESTORE_SIGMASK);
WARN_ON(!test_thread_flag(TIF_SIGPENDING));
}
+
+static inline void clear_tsk_restore_sigmask(struct task_struct *tsk)
+{
+ clear_tsk_thread_flag(tsk, TIF_RESTORE_SIGMASK);
+}
+
static inline void clear_restore_sigmask(void)
{
clear_thread_flag(TIF_RESTORE_SIGMASK);
}
+static inline bool test_tsk_restore_sigmask(struct task_struct *tsk)
+{
+ return test_tsk_thread_flag(tsk, TIF_RESTORE_SIGMASK);
+}
static inline bool test_restore_sigmask(void)
{
return test_thread_flag(TIF_RESTORE_SIGMASK);
@@ -438,6 +448,10 @@ static inline void set_restore_sigmask(void)
current->restore_sigmask = true;
WARN_ON(!test_thread_flag(TIF_SIGPENDING));
}
+static inline void clear_tsk_restore_sigmask(struct task_struct *tsk)
+{
+ tsk->restore_sigmask = false;
+}
static inline void clear_restore_sigmask(void)
{
current->restore_sigmask = false;
@@ -446,6 +460,10 @@ static inline bool test_restore_sigmask(void)
{
return current->restore_sigmask;
}
+static inline bool test_tsk_restore_sigmask(struct task_struct *tsk)
+{
+ return tsk->restore_sigmask;
+}
static inline bool test_and_clear_restore_sigmask(void)
{
if (!current->restore_sigmask)
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 21fec73d45d4..fc0d667f5792 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
/*
* Access another process' address space via ptrace.
@@ -925,18 +926,26 @@ int ptrace_request(struct task_struct *child, long
request,
ret = ptrace_setsiginfo(child, );
break;
- case PTRACE_GETSIGMASK:
+ case PTRACE_GETSIGMASK: {
+ sigset_t *mask;
+
if (addr != sizeof(sigset_t)) {
ret = -EINVAL;
break;
}
- if (copy_to_user(datavp, >blocked, sizeof(sigset_t)))
+ if (test_tsk_restore_sigmask(child))
+ mask = >saved_sigmask;
+ else
+ mask = >blocked;
+
+ if (copy_to_user(datavp, mask, sizeof(sigset_t)))
ret = -EFAULT;
else
ret = 0;
break;
+ }
case PTRACE_SETSIGMASK: {
sigset_t new_set;
@@ -962,6 +971,8 @@ int ptrace_request(struct task_struct *child, long request,
child->blocked = new_set;
spin_unlock_irq(>sighand->siglock);
+ clear_tsk_restore_sigmask(child);
+
ret = 0;
break;
}
--
2.17.2
[PATCH] ptrace: take into account saved_sigmask in PTRACE_{GET,SET}SIGMASK
There are a few system calls (pselect, ppoll, etc) which replace a task
sigmask while they are running in a kernel-space
When a task calls one of these syscalls, the kernel saves a current
sigmask in task->saved_sigmask and sets a syscall sigmask.
On syscall-exit-stop, ptrace traps a task before restoring the
saved_sigmask, so PTRACE_GETSIGMASK returns the syscall sigmask and
PTRACE_SETSIGMASK does nothing, because its sigmask is replaced by
saved_sigmask, when the task returns to user-space.
This patch fixes this problem. PTRACE_GET_SIGMASK returns saved_sigmask
is it's set. PTRACE_SETSIGMASK drops the TIF_RESTORE_SIGMASK flag.
Cc: Oleg Nesterov
Cc: "Eric W. Biederman"
Cc: Andrew Morton
Fixes: 29000caecbe8 ("ptrace: add ability to get/set signal-blocked mask")
Signed-off-by: Andrei Vagin
---
include/linux/sched/signal.h | 18 ++
kernel/ptrace.c | 15 +--
2 files changed, 31 insertions(+), 2 deletions(-)
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h
index 1be35729c2c5..660d78c9af6c 100644
--- a/include/linux/sched/signal.h
+++ b/include/linux/sched/signal.h
@@ -417,10 +417,20 @@ static inline void set_restore_sigmask(void)
set_thread_flag(TIF_RESTORE_SIGMASK);
WARN_ON(!test_thread_flag(TIF_SIGPENDING));
}
+
+static inline void clear_tsk_restore_sigmask(struct task_struct *tsk)
+{
+ clear_tsk_thread_flag(tsk, TIF_RESTORE_SIGMASK);
+}
+
static inline void clear_restore_sigmask(void)
{
clear_thread_flag(TIF_RESTORE_SIGMASK);
}
+static inline bool test_tsk_restore_sigmask(struct task_struct *tsk)
+{
+ return test_tsk_thread_flag(tsk, TIF_RESTORE_SIGMASK);
+}
static inline bool test_restore_sigmask(void)
{
return test_thread_flag(TIF_RESTORE_SIGMASK);
@@ -438,6 +448,10 @@ static inline void set_restore_sigmask(void)
current->restore_sigmask = true;
WARN_ON(!test_thread_flag(TIF_SIGPENDING));
}
+static inline void clear_tsk_restore_sigmask(struct task_struct *tsk)
+{
+ tsk->restore_sigmask = false;
+}
static inline void clear_restore_sigmask(void)
{
current->restore_sigmask = false;
@@ -446,6 +460,10 @@ static inline bool test_restore_sigmask(void)
{
return current->restore_sigmask;
}
+static inline bool test_tsk_restore_sigmask(struct task_struct *tsk)
+{
+ return tsk->restore_sigmask;
+}
static inline bool test_and_clear_restore_sigmask(void)
{
if (!current->restore_sigmask)
diff --git a/kernel/ptrace.c b/kernel/ptrace.c
index 21fec73d45d4..fc0d667f5792 100644
--- a/kernel/ptrace.c
+++ b/kernel/ptrace.c
@@ -29,6 +29,7 @@
#include
#include
#include
+#include
/*
* Access another process' address space via ptrace.
@@ -925,18 +926,26 @@ int ptrace_request(struct task_struct *child, long
request,
ret = ptrace_setsiginfo(child, );
break;
- case PTRACE_GETSIGMASK:
+ case PTRACE_GETSIGMASK: {
+ sigset_t *mask;
+
if (addr != sizeof(sigset_t)) {
ret = -EINVAL;
break;
}
- if (copy_to_user(datavp, >blocked, sizeof(sigset_t)))
+ if (test_tsk_restore_sigmask(child))
+ mask = >saved_sigmask;
+ else
+ mask = >blocked;
+
+ if (copy_to_user(datavp, mask, sizeof(sigset_t)))
ret = -EFAULT;
else
ret = 0;
break;
+ }
case PTRACE_SETSIGMASK: {
sigset_t new_set;
@@ -962,6 +971,8 @@ int ptrace_request(struct task_struct *child, long request,
child->blocked = new_set;
spin_unlock_irq(>sighand->siglock);
+ clear_tsk_restore_sigmask(child);
+
ret = 0;
break;
}
--
2.17.2
Re: Memory hotplug softlock issue
On Tue, 20 Nov 2018, Baoquan He wrote: > On 11/19/18 at 09:59pm, Michal Hocko wrote: > > On Mon 19-11-18 12:34:09, Hugh Dickins wrote: > > > I'm glad that I delayed, what I had then (migration_waitqueue instead > > > of using page_waitqueue) was not wrong, but what I've been using the > > > last couple of months is rather better (and can be put to use to solve > > > similar problems in collapsing pages on huge tmpfs. but we don't need > > > to get into that at this time): put_and_wait_on_page_locked(). > > > > > > What I have not yet done is verify it on latest kernel, and research > > > the interested Cc list (Linus and Tim Chen come immediately to mind), > > > and write the commit comment. I have some testing to do on the latest > > > kernel today, so I'll throw put_and_wait_on_page_locked() in too, > > > and post tomorrow I hope. > > > > Cool, it seems that Baoquan has a reliable test case to trigger the > > pathological case. > > Yes. I will test Hugh's patch. Thanks: I've completed some of the retesting now, so it would probably help us all better if I post the patch in this thread, even without completing its description and links and Cc list yet - there isn't even a problem description below, I still have to paste that in from the unposted patch that I made six months ago. Here is today's... [PATCH] mm: put_and_wait_on_page_locked() while page is migrated We have all assumed that it is essential to hold a page reference while waiting on a page lock: partly to guarantee that there is still a struct page when MEMORY_HOTREMOVE is configured, but also to protect against reuse of the struct page going to someone who then holds the page locked indefinitely, when the waiter can reasonably expect timely unlocking. But in fact, so long as wait_on_page_bit_common() does the put_page(), and is careful not to rely on struct page contents thereafter, there is no need to hold a reference to the page while waiting on it. That does mean that this case cannot go back through the loop: but that's fine for the page migration case, and even if used more widely, is limited by the "Stop walking if it's locked" optimization in wake_page_function(). Add interface put_and_wait_on_page_locked() to do this, using negative value of the lock arg to wait_on_page_bit_common() to implement it. No interruptible or killable variant needed yet, but they might follow: I have a vague notion that reporting -EINTR should take precedence over return from wait_on_page_bit_common() without knowing the page state, so arrange it accordingly - but that may be nothing but pedantic. shrink_page_list()'s __ClearPageLocked(): that was a surprise! this survived a lot of testing before that showed up. It does raise the question: should is_page_cache_freeable() and __remove_mapping() now treat a PG_waiters page as if an extra reference were held? Perhaps, but I don't think it matters much, since shrink_page_list() already had to win its trylock_page(), so waiters are not very common there: I noticed no difference when trying the bigger change, and it's surely not needed while put_and_wait_on_page_locked() is only for page migration. Signed-off-by: Hugh Dickins --- include/linux/pagemap.h | 2 ++ mm/filemap.c| 53 - mm/huge_memory.c| 6 ++--- mm/migrate.c| 12 -- mm/vmscan.c | 11 + 5 files changed, 57 insertions(+), 27 deletions(-) diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index 226f96f0dee0..e2d7039af6a3 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -537,6 +537,8 @@ static inline int wait_on_page_locked_killable(struct page *page) return wait_on_page_bit_killable(compound_head(page), PG_locked); } +extern void put_and_wait_on_page_locked(struct page *page); + /* * Wait for a page to complete writeback */ diff --git a/mm/filemap.c b/mm/filemap.c index 81adec8ee02c..ef82119032d8 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -981,7 +981,14 @@ static int wake_page_function(wait_queue_entry_t *wait, unsigned mode, int sync, if (wait_page->bit_nr != key->bit_nr) return 0; - /* Stop walking if it's locked */ + /* +* Stop walking if it's locked. +* Is this safe if put_and_wait_on_page_locked() is in use? +* Yes: the waker must hold a reference to this page, and if PG_locked +* has now already been set by another task, that task must also hold +* a reference to the *same usage* of this page; so there is no need +* to walk on to wake even the put_and_wait_on_page_locked() callers. +*/ if (test_bit(key->bit_nr, >page->flags)) return -1; @@ -1050,13 +1057,14 @@ static void wake_up_page(struct page *page, int bit) } static inline int wait_on_page_bit_common(wait_queue_head_t *q, - struct page *page, int bit_nr, int state, bool lock) +
Re: Memory hotplug softlock issue
On Tue, 20 Nov 2018, Baoquan He wrote: > On 11/19/18 at 09:59pm, Michal Hocko wrote: > > On Mon 19-11-18 12:34:09, Hugh Dickins wrote: > > > I'm glad that I delayed, what I had then (migration_waitqueue instead > > > of using page_waitqueue) was not wrong, but what I've been using the > > > last couple of months is rather better (and can be put to use to solve > > > similar problems in collapsing pages on huge tmpfs. but we don't need > > > to get into that at this time): put_and_wait_on_page_locked(). > > > > > > What I have not yet done is verify it on latest kernel, and research > > > the interested Cc list (Linus and Tim Chen come immediately to mind), > > > and write the commit comment. I have some testing to do on the latest > > > kernel today, so I'll throw put_and_wait_on_page_locked() in too, > > > and post tomorrow I hope. > > > > Cool, it seems that Baoquan has a reliable test case to trigger the > > pathological case. > > Yes. I will test Hugh's patch. Thanks: I've completed some of the retesting now, so it would probably help us all better if I post the patch in this thread, even without completing its description and links and Cc list yet - there isn't even a problem description below, I still have to paste that in from the unposted patch that I made six months ago. Here is today's... [PATCH] mm: put_and_wait_on_page_locked() while page is migrated We have all assumed that it is essential to hold a page reference while waiting on a page lock: partly to guarantee that there is still a struct page when MEMORY_HOTREMOVE is configured, but also to protect against reuse of the struct page going to someone who then holds the page locked indefinitely, when the waiter can reasonably expect timely unlocking. But in fact, so long as wait_on_page_bit_common() does the put_page(), and is careful not to rely on struct page contents thereafter, there is no need to hold a reference to the page while waiting on it. That does mean that this case cannot go back through the loop: but that's fine for the page migration case, and even if used more widely, is limited by the "Stop walking if it's locked" optimization in wake_page_function(). Add interface put_and_wait_on_page_locked() to do this, using negative value of the lock arg to wait_on_page_bit_common() to implement it. No interruptible or killable variant needed yet, but they might follow: I have a vague notion that reporting -EINTR should take precedence over return from wait_on_page_bit_common() without knowing the page state, so arrange it accordingly - but that may be nothing but pedantic. shrink_page_list()'s __ClearPageLocked(): that was a surprise! this survived a lot of testing before that showed up. It does raise the question: should is_page_cache_freeable() and __remove_mapping() now treat a PG_waiters page as if an extra reference were held? Perhaps, but I don't think it matters much, since shrink_page_list() already had to win its trylock_page(), so waiters are not very common there: I noticed no difference when trying the bigger change, and it's surely not needed while put_and_wait_on_page_locked() is only for page migration. Signed-off-by: Hugh Dickins --- include/linux/pagemap.h | 2 ++ mm/filemap.c| 53 - mm/huge_memory.c| 6 ++--- mm/migrate.c| 12 -- mm/vmscan.c | 11 + 5 files changed, 57 insertions(+), 27 deletions(-) diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index 226f96f0dee0..e2d7039af6a3 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -537,6 +537,8 @@ static inline int wait_on_page_locked_killable(struct page *page) return wait_on_page_bit_killable(compound_head(page), PG_locked); } +extern void put_and_wait_on_page_locked(struct page *page); + /* * Wait for a page to complete writeback */ diff --git a/mm/filemap.c b/mm/filemap.c index 81adec8ee02c..ef82119032d8 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -981,7 +981,14 @@ static int wake_page_function(wait_queue_entry_t *wait, unsigned mode, int sync, if (wait_page->bit_nr != key->bit_nr) return 0; - /* Stop walking if it's locked */ + /* +* Stop walking if it's locked. +* Is this safe if put_and_wait_on_page_locked() is in use? +* Yes: the waker must hold a reference to this page, and if PG_locked +* has now already been set by another task, that task must also hold +* a reference to the *same usage* of this page; so there is no need +* to walk on to wake even the put_and_wait_on_page_locked() callers. +*/ if (test_bit(key->bit_nr, >page->flags)) return -1; @@ -1050,13 +1057,14 @@ static void wake_up_page(struct page *page, int bit) } static inline int wait_on_page_bit_common(wait_queue_head_t *q, - struct page *page, int bit_nr, int state, bool lock) +
Re: [PATCH] MAINTAINERS: Add Kishon as maintainer of PHY bindings
Hi Rob, On 17/11/18 1:50 AM, Rob Herring wrote: On Thu, Oct 18, 2018 at 8:31 AM Rob Herring wrote: DT bindings normally go in via subsystem maintainers, so add PHY bindings under generic PHY framework. Reported-by: Gustavo A. R. Silva Cc: Kishon Vijay Abraham I Signed-off-by: Rob Herring Ping? Kishon, any objection? It's queued [1]. Thanks Kishon [1] -> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/MAINTAINERS?id=5c7412b139f189e18842d62951f6857e4937ce73 --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9ad052aeac39..b01e77ae79c4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6132,6 +6132,7 @@ T:git git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git S: Supported F: drivers/phy/ F: include/linux/phy/ +F: Documentation/devicetree/bindings/phy/ GENERIC PINCTRL I2C DEMULTIPLEXER DRIVER M: Wolfram Sang -- 2.17.1
Re: [PATCH] MAINTAINERS: Add Kishon as maintainer of PHY bindings
Hi Rob, On 17/11/18 1:50 AM, Rob Herring wrote: On Thu, Oct 18, 2018 at 8:31 AM Rob Herring wrote: DT bindings normally go in via subsystem maintainers, so add PHY bindings under generic PHY framework. Reported-by: Gustavo A. R. Silva Cc: Kishon Vijay Abraham I Signed-off-by: Rob Herring Ping? Kishon, any objection? It's queued [1]. Thanks Kishon [1] -> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/MAINTAINERS?id=5c7412b139f189e18842d62951f6857e4937ce73 --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9ad052aeac39..b01e77ae79c4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -6132,6 +6132,7 @@ T:git git://git.kernel.org/pub/scm/linux/kernel/git/kishon/linux-phy.git S: Supported F: drivers/phy/ F: include/linux/phy/ +F: Documentation/devicetree/bindings/phy/ GENERIC PINCTRL I2C DEMULTIPLEXER DRIVER M: Wolfram Sang -- 2.17.1
[PATCH -manpage 2/2] memfd_create.2: Update manpage with new memfd F_SEAL_FUTURE_WRITE seal
More details of the seal can be found in the LKML patch:
https://lore.kernel.org/lkml/20181120052137.74317-1-j...@joelfernandes.org/T/#t
Signed-off-by: Joel Fernandes (Google)
---
man2/memfd_create.2 | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/man2/memfd_create.2 b/man2/memfd_create.2
index 3cd392d1b4d9..fce2bf8d0fff 100644
--- a/man2/memfd_create.2
+++ b/man2/memfd_create.2
@@ -280,7 +280,15 @@ in order to restrict further modifications on the file.
(If placing the seal
.BR F_SEAL_WRITE ,
then it will be necessary to first unmap the shared writable mapping
-created in the previous step.)
+created in the previous step. Otherwise, behavior similar to
+.BR F_SEAL_WRITE
+can be achieved, by using
+.BR F_SEAL_FUTURE_WRITE
+which will prevent future writes via
+.BR mmap (2)
+and
+.BR write (2)
+from succeeding, while keeping existing shared writable mappings).
.IP 4.
A second process obtains a file descriptor for the
.BR tmpfs (5)
@@ -425,6 +433,7 @@ main(int argc, char *argv[])
fprintf(stderr, "\\t\\tg \- F_SEAL_GROW\\n");
fprintf(stderr, "\\t\\ts \- F_SEAL_SHRINK\\n");
fprintf(stderr, "\\t\\tw \- F_SEAL_WRITE\\n");
+fprintf(stderr, "\\t\\tW \- F_SEAL_FUTURE_WRITE\\n");
fprintf(stderr, "\\t\\tS \- F_SEAL_SEAL\\n");
exit(EXIT_FAILURE);
}
@@ -463,6 +472,8 @@ main(int argc, char *argv[])
seals |= F_SEAL_SHRINK;
if (strchr(seals_arg, \(aqw\(aq) != NULL)
seals |= F_SEAL_WRITE;
+if (strchr(seals_arg, \(aqW\(aq) != NULL)
+seals |= F_SEAL_FUTURE_WRITE;
if (strchr(seals_arg, \(aqS\(aq) != NULL)
seals |= F_SEAL_SEAL;
@@ -518,6 +529,8 @@ main(int argc, char *argv[])
printf(" GROW");
if (seals & F_SEAL_WRITE)
printf(" WRITE");
+if (seals & F_SEAL_FUTURE_WRITE)
+printf(" FUTURE_WRITE");
if (seals & F_SEAL_SHRINK)
printf(" SHRINK");
printf("\\n");
--
2.19.1.1215.g8438c0b245-goog
[PATCH -manpage 2/2] memfd_create.2: Update manpage with new memfd F_SEAL_FUTURE_WRITE seal
More details of the seal can be found in the LKML patch:
https://lore.kernel.org/lkml/20181120052137.74317-1-j...@joelfernandes.org/T/#t
Signed-off-by: Joel Fernandes (Google)
---
man2/memfd_create.2 | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/man2/memfd_create.2 b/man2/memfd_create.2
index 3cd392d1b4d9..fce2bf8d0fff 100644
--- a/man2/memfd_create.2
+++ b/man2/memfd_create.2
@@ -280,7 +280,15 @@ in order to restrict further modifications on the file.
(If placing the seal
.BR F_SEAL_WRITE ,
then it will be necessary to first unmap the shared writable mapping
-created in the previous step.)
+created in the previous step. Otherwise, behavior similar to
+.BR F_SEAL_WRITE
+can be achieved, by using
+.BR F_SEAL_FUTURE_WRITE
+which will prevent future writes via
+.BR mmap (2)
+and
+.BR write (2)
+from succeeding, while keeping existing shared writable mappings).
.IP 4.
A second process obtains a file descriptor for the
.BR tmpfs (5)
@@ -425,6 +433,7 @@ main(int argc, char *argv[])
fprintf(stderr, "\\t\\tg \- F_SEAL_GROW\\n");
fprintf(stderr, "\\t\\ts \- F_SEAL_SHRINK\\n");
fprintf(stderr, "\\t\\tw \- F_SEAL_WRITE\\n");
+fprintf(stderr, "\\t\\tW \- F_SEAL_FUTURE_WRITE\\n");
fprintf(stderr, "\\t\\tS \- F_SEAL_SEAL\\n");
exit(EXIT_FAILURE);
}
@@ -463,6 +472,8 @@ main(int argc, char *argv[])
seals |= F_SEAL_SHRINK;
if (strchr(seals_arg, \(aqw\(aq) != NULL)
seals |= F_SEAL_WRITE;
+if (strchr(seals_arg, \(aqW\(aq) != NULL)
+seals |= F_SEAL_FUTURE_WRITE;
if (strchr(seals_arg, \(aqS\(aq) != NULL)
seals |= F_SEAL_SEAL;
@@ -518,6 +529,8 @@ main(int argc, char *argv[])
printf(" GROW");
if (seals & F_SEAL_WRITE)
printf(" WRITE");
+if (seals & F_SEAL_FUTURE_WRITE)
+printf(" FUTURE_WRITE");
if (seals & F_SEAL_SHRINK)
printf(" SHRINK");
printf("\\n");
--
2.19.1.1215.g8438c0b245-goog
[PATCH -manpage 1/2] fcntl.2: Update manpage with new memfd F_SEAL_FUTURE_WRITE seal
More details of the seal can be found in the LKML patch: https://lore.kernel.org/lkml/20181120052137.74317-1-j...@joelfernandes.org/T/#t Signed-off-by: Joel Fernandes (Google) --- man2/fcntl.2 | 15 +++ 1 file changed, 15 insertions(+) diff --git a/man2/fcntl.2 b/man2/fcntl.2 index 03533d65b49d..54772f94964c 100644 --- a/man2/fcntl.2 +++ b/man2/fcntl.2 @@ -1525,6 +1525,21 @@ Furthermore, if there are any asynchronous I/O operations .RB ( io_submit (2)) pending on the file, all outstanding writes will be discarded. +.TP +.BR F_SEAL_FUTURE_WRITE +If this seal is set, the contents of the file can be modified only from +existing writeable mappings that were created prior to the seal being set. +Any attempt to create a new writeable mapping on the memfd via +.BR mmap (2) +will fail with +.BR EPERM. +Also any attempts to write to the memfd via +.BR write (2) +will fail with +.BR EPERM. +This is useful in situations where existing writable mapped regions need to be +kept intact while preventing any future writes. For example, to share a +read-only memory buffer to other processes that only the sender can write to. .\" .SS File read/write hints Write lifetime hints can be used to inform the kernel about the relative -- 2.19.1.1215.g8438c0b245-goog
[PATCH -manpage 1/2] fcntl.2: Update manpage with new memfd F_SEAL_FUTURE_WRITE seal
More details of the seal can be found in the LKML patch: https://lore.kernel.org/lkml/20181120052137.74317-1-j...@joelfernandes.org/T/#t Signed-off-by: Joel Fernandes (Google) --- man2/fcntl.2 | 15 +++ 1 file changed, 15 insertions(+) diff --git a/man2/fcntl.2 b/man2/fcntl.2 index 03533d65b49d..54772f94964c 100644 --- a/man2/fcntl.2 +++ b/man2/fcntl.2 @@ -1525,6 +1525,21 @@ Furthermore, if there are any asynchronous I/O operations .RB ( io_submit (2)) pending on the file, all outstanding writes will be discarded. +.TP +.BR F_SEAL_FUTURE_WRITE +If this seal is set, the contents of the file can be modified only from +existing writeable mappings that were created prior to the seal being set. +Any attempt to create a new writeable mapping on the memfd via +.BR mmap (2) +will fail with +.BR EPERM. +Also any attempts to write to the memfd via +.BR write (2) +will fail with +.BR EPERM. +This is useful in situations where existing writable mapped regions need to be +kept intact while preventing any future writes. For example, to share a +read-only memory buffer to other processes that only the sender can write to. .\" .SS File read/write hints Write lifetime hints can be used to inform the kernel about the relative -- 2.19.1.1215.g8438c0b245-goog
[PATCH -next 1/2] mm/memfd: make F_SEAL_FUTURE_WRITE seal more robust
A better way to do F_SEAL_FUTURE_WRITE seal was discussed [1] last week
where we don't need to modify core VFS structures to get the same
behavior of the seal. This solves several side-effects pointed out by
Andy [2].
[1] https://lore.kernel.org/lkml/2018173650.ga256...@google.com/
[2]
https://lore.kernel.org/lkml/69ce06cc-e47c-4992-848a-66eb23ee6...@amacapital.net/
Suggested-by: Andy Lutomirski
Fixes: 5e653c2923fd ("mm: Add an F_SEAL_FUTURE_WRITE seal to memfd")
Signed-off-by: Joel Fernandes (Google)
---
fs/hugetlbfs/inode.c | 2 +-
mm/memfd.c | 19 ---
mm/shmem.c | 24 +---
3 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 762028994f47..5b54bf893a67 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -558,7 +558,7 @@ static long hugetlbfs_punch_hole(struct inode *inode,
loff_t offset, loff_t len)
inode_lock(inode);
/* protected by i_mutex */
- if (info->seals & F_SEAL_WRITE) {
+ if (info->seals & (F_SEAL_WRITE | F_SEAL_FUTURE_WRITE)) {
inode_unlock(inode);
return -EPERM;
}
diff --git a/mm/memfd.c b/mm/memfd.c
index 63fff5e77114..650e65a46b9c 100644
--- a/mm/memfd.c
+++ b/mm/memfd.c
@@ -201,25 +201,6 @@ static int memfd_add_seals(struct file *file, unsigned int
seals)
}
}
- if ((seals & F_SEAL_FUTURE_WRITE) &&
- !(*file_seals & F_SEAL_FUTURE_WRITE)) {
- /*
-* The FUTURE_WRITE seal also prevents growing and shrinking
-* so we need them to be already set, or requested now.
-*/
- int test_seals = (seals | *file_seals) &
-(F_SEAL_GROW | F_SEAL_SHRINK);
-
- if (test_seals != (F_SEAL_GROW | F_SEAL_SHRINK)) {
- error = -EINVAL;
- goto unlock;
- }
-
- spin_lock(>f_lock);
- file->f_mode &= ~(FMODE_WRITE | FMODE_PWRITE);
- spin_unlock(>f_lock);
- }
-
*file_seals |= seals;
error = 0;
diff --git a/mm/shmem.c b/mm/shmem.c
index 32eb29bd72c6..cee9878c87f1 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2121,6 +2121,23 @@ int shmem_lock(struct file *file, int lock, struct
user_struct *user)
static int shmem_mmap(struct file *file, struct vm_area_struct *vma)
{
+ struct shmem_inode_info *info = SHMEM_I(file_inode(file));
+
+ /*
+* New PROT_READ and MAP_SHARED mmaps are not allowed when "future
+* write" seal active.
+*/
+ if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_WRITE) &&
+ (info->seals & F_SEAL_FUTURE_WRITE))
+ return -EPERM;
+
+ /*
+* Since the F_SEAL_FUTURE_WRITE seals allow for a MAP_SHARED read-only
+* mapping, take care to not allow mprotect to revert protections.
+*/
+ if (info->seals & F_SEAL_FUTURE_WRITE)
+ vma->vm_flags &= ~(VM_MAYWRITE);
+
file_accessed(file);
vma->vm_ops = _vm_ops;
if (IS_ENABLED(CONFIG_TRANSPARENT_HUGE_PAGECACHE) &&
@@ -2346,8 +2363,9 @@ shmem_write_begin(struct file *file, struct address_space
*mapping,
pgoff_t index = pos >> PAGE_SHIFT;
/* i_mutex is held by caller */
- if (unlikely(info->seals & (F_SEAL_WRITE | F_SEAL_GROW))) {
- if (info->seals & F_SEAL_WRITE)
+ if (unlikely(info->seals & (F_SEAL_GROW |
+ F_SEAL_WRITE | F_SEAL_FUTURE_WRITE))) {
+ if (info->seals & (F_SEAL_WRITE | F_SEAL_FUTURE_WRITE))
return -EPERM;
if ((info->seals & F_SEAL_GROW) && pos + len > inode->i_size)
return -EPERM;
@@ -2610,7 +2628,7 @@ static long shmem_fallocate(struct file *file, int mode,
loff_t offset,
DECLARE_WAIT_QUEUE_HEAD_ONSTACK(shmem_falloc_waitq);
/* protected by i_mutex */
- if (info->seals & F_SEAL_WRITE) {
+ if (info->seals & (F_SEAL_WRITE | F_SEAL_FUTURE_WRITE)) {
error = -EPERM;
goto out;
}
--
2.19.1.1215.g8438c0b245-goog
[PATCH -next 2/2] selftests/memfd: modify tests for F_SEAL_FUTURE_WRITE seal
Modify the tests for F_SEAL_FUTURE_WRITE based on the changes
introduced in previous patch.
Also add a test to make sure the reopen issue pointed by Jann Horn [1]
is fixed.
[1]
https://lore.kernel.org/lkml/CAG48ez1h=v-JYnDw81HaYJzOfrNhwYksxmc2r=cjvdqvgym...@mail.gmail.com/
Cc: Jann Horn
Signed-off-by: Joel Fernandes (Google)
---
tools/testing/selftests/memfd/memfd_test.c | 88 +++---
1 file changed, 44 insertions(+), 44 deletions(-)
diff --git a/tools/testing/selftests/memfd/memfd_test.c
b/tools/testing/selftests/memfd/memfd_test.c
index 32b207ca7372..c67d32eeb668 100644
--- a/tools/testing/selftests/memfd/memfd_test.c
+++ b/tools/testing/selftests/memfd/memfd_test.c
@@ -54,6 +54,22 @@ static int mfd_assert_new(const char *name, loff_t sz,
unsigned int flags)
return fd;
}
+static int mfd_assert_reopen_fd(int fd_in)
+{
+ int r, fd;
+ char path[100];
+
+ sprintf(path, "/proc/self/fd/%d", fd_in);
+
+ fd = open(path, O_RDWR);
+ if (fd < 0) {
+ printf("re-open of existing fd %d failed\n", fd_in);
+ abort();
+ }
+
+ return fd;
+}
+
static void mfd_fail_new(const char *name, unsigned int flags)
{
int r;
@@ -255,6 +271,25 @@ static void mfd_assert_read(int fd)
munmap(p, mfd_def_size);
}
+/* Test that PROT_READ + MAP_SHARED mappings work. */
+static void mfd_assert_read_shared(int fd)
+{
+ void *p;
+
+ /* verify PROT_READ and MAP_SHARED *is* allowed */
+ p = mmap(NULL,
+mfd_def_size,
+PROT_READ,
+MAP_SHARED,
+fd,
+0);
+ if (p == MAP_FAILED) {
+ printf("mmap() failed: %m\n");
+ abort();
+ }
+ munmap(p, mfd_def_size);
+}
+
static void mfd_assert_write(int fd)
{
ssize_t l;
@@ -698,7 +733,7 @@ static void test_seal_write(void)
*/
static void test_seal_future_write(void)
{
- int fd;
+ int fd, fd2;
void *p;
printf("%s SEAL-FUTURE-WRITE\n", memfd_str);
@@ -710,58 +745,23 @@ static void test_seal_future_write(void)
p = mfd_assert_mmap_shared(fd);
mfd_assert_has_seals(fd, 0);
- /* Not adding grow/shrink seals makes the future write
-* seal fail to get added
-*/
- mfd_fail_add_seals(fd, F_SEAL_FUTURE_WRITE);
-
- mfd_assert_add_seals(fd, F_SEAL_GROW);
- mfd_assert_has_seals(fd, F_SEAL_GROW);
-
- /* Should still fail since shrink seal has
-* not yet been added
-*/
- mfd_fail_add_seals(fd, F_SEAL_FUTURE_WRITE);
-
- mfd_assert_add_seals(fd, F_SEAL_SHRINK);
- mfd_assert_has_seals(fd, F_SEAL_GROW |
-F_SEAL_SHRINK);
- /* Now should succeed, also verifies that the seal
-* could be added with an existing writable mmap
-*/
mfd_assert_add_seals(fd, F_SEAL_FUTURE_WRITE);
- mfd_assert_has_seals(fd, F_SEAL_SHRINK |
-F_SEAL_GROW |
-F_SEAL_FUTURE_WRITE);
+ mfd_assert_has_seals(fd, F_SEAL_FUTURE_WRITE);
/* read should pass, writes should fail */
mfd_assert_read(fd);
+ mfd_assert_read_shared(fd);
mfd_fail_write(fd);
- munmap(p, mfd_def_size);
- close(fd);
-
- /* Test adding all seals (grow, shrink, future write) at once */
- fd = mfd_assert_new("kern_memfd_seal_future_write2",
- mfd_def_size,
- MFD_CLOEXEC | MFD_ALLOW_SEALING);
-
- p = mfd_assert_mmap_shared(fd);
-
- mfd_assert_has_seals(fd, 0);
- mfd_assert_add_seals(fd, F_SEAL_SHRINK |
-F_SEAL_GROW |
-F_SEAL_FUTURE_WRITE);
- mfd_assert_has_seals(fd, F_SEAL_SHRINK |
-F_SEAL_GROW |
-F_SEAL_FUTURE_WRITE);
-
- /* read should pass, writes should fail */
- mfd_assert_read(fd);
- mfd_fail_write(fd);
+ fd2 = mfd_assert_reopen_fd(fd);
+ /* read should pass, writes should still fail */
+ mfd_assert_read(fd2);
+ mfd_assert_read_shared(fd2);
+ mfd_fail_write(fd2);
munmap(p, mfd_def_size);
+ close(fd2);
close(fd);
}
--
2.19.1.1215.g8438c0b245-goog
[PATCH -next 1/2] mm/memfd: make F_SEAL_FUTURE_WRITE seal more robust
A better way to do F_SEAL_FUTURE_WRITE seal was discussed [1] last week
where we don't need to modify core VFS structures to get the same
behavior of the seal. This solves several side-effects pointed out by
Andy [2].
[1] https://lore.kernel.org/lkml/2018173650.ga256...@google.com/
[2]
https://lore.kernel.org/lkml/69ce06cc-e47c-4992-848a-66eb23ee6...@amacapital.net/
Suggested-by: Andy Lutomirski
Fixes: 5e653c2923fd ("mm: Add an F_SEAL_FUTURE_WRITE seal to memfd")
Signed-off-by: Joel Fernandes (Google)
---
fs/hugetlbfs/inode.c | 2 +-
mm/memfd.c | 19 ---
mm/shmem.c | 24 +---
3 files changed, 22 insertions(+), 23 deletions(-)
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index 762028994f47..5b54bf893a67 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -558,7 +558,7 @@ static long hugetlbfs_punch_hole(struct inode *inode,
loff_t offset, loff_t len)
inode_lock(inode);
/* protected by i_mutex */
- if (info->seals & F_SEAL_WRITE) {
+ if (info->seals & (F_SEAL_WRITE | F_SEAL_FUTURE_WRITE)) {
inode_unlock(inode);
return -EPERM;
}
diff --git a/mm/memfd.c b/mm/memfd.c
index 63fff5e77114..650e65a46b9c 100644
--- a/mm/memfd.c
+++ b/mm/memfd.c
@@ -201,25 +201,6 @@ static int memfd_add_seals(struct file *file, unsigned int
seals)
}
}
- if ((seals & F_SEAL_FUTURE_WRITE) &&
- !(*file_seals & F_SEAL_FUTURE_WRITE)) {
- /*
-* The FUTURE_WRITE seal also prevents growing and shrinking
-* so we need them to be already set, or requested now.
-*/
- int test_seals = (seals | *file_seals) &
-(F_SEAL_GROW | F_SEAL_SHRINK);
-
- if (test_seals != (F_SEAL_GROW | F_SEAL_SHRINK)) {
- error = -EINVAL;
- goto unlock;
- }
-
- spin_lock(>f_lock);
- file->f_mode &= ~(FMODE_WRITE | FMODE_PWRITE);
- spin_unlock(>f_lock);
- }
-
*file_seals |= seals;
error = 0;
diff --git a/mm/shmem.c b/mm/shmem.c
index 32eb29bd72c6..cee9878c87f1 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2121,6 +2121,23 @@ int shmem_lock(struct file *file, int lock, struct
user_struct *user)
static int shmem_mmap(struct file *file, struct vm_area_struct *vma)
{
+ struct shmem_inode_info *info = SHMEM_I(file_inode(file));
+
+ /*
+* New PROT_READ and MAP_SHARED mmaps are not allowed when "future
+* write" seal active.
+*/
+ if ((vma->vm_flags & VM_SHARED) && (vma->vm_flags & VM_WRITE) &&
+ (info->seals & F_SEAL_FUTURE_WRITE))
+ return -EPERM;
+
+ /*
+* Since the F_SEAL_FUTURE_WRITE seals allow for a MAP_SHARED read-only
+* mapping, take care to not allow mprotect to revert protections.
+*/
+ if (info->seals & F_SEAL_FUTURE_WRITE)
+ vma->vm_flags &= ~(VM_MAYWRITE);
+
file_accessed(file);
vma->vm_ops = _vm_ops;
if (IS_ENABLED(CONFIG_TRANSPARENT_HUGE_PAGECACHE) &&
@@ -2346,8 +2363,9 @@ shmem_write_begin(struct file *file, struct address_space
*mapping,
pgoff_t index = pos >> PAGE_SHIFT;
/* i_mutex is held by caller */
- if (unlikely(info->seals & (F_SEAL_WRITE | F_SEAL_GROW))) {
- if (info->seals & F_SEAL_WRITE)
+ if (unlikely(info->seals & (F_SEAL_GROW |
+ F_SEAL_WRITE | F_SEAL_FUTURE_WRITE))) {
+ if (info->seals & (F_SEAL_WRITE | F_SEAL_FUTURE_WRITE))
return -EPERM;
if ((info->seals & F_SEAL_GROW) && pos + len > inode->i_size)
return -EPERM;
@@ -2610,7 +2628,7 @@ static long shmem_fallocate(struct file *file, int mode,
loff_t offset,
DECLARE_WAIT_QUEUE_HEAD_ONSTACK(shmem_falloc_waitq);
/* protected by i_mutex */
- if (info->seals & F_SEAL_WRITE) {
+ if (info->seals & (F_SEAL_WRITE | F_SEAL_FUTURE_WRITE)) {
error = -EPERM;
goto out;
}
--
2.19.1.1215.g8438c0b245-goog
[PATCH -next 2/2] selftests/memfd: modify tests for F_SEAL_FUTURE_WRITE seal
Modify the tests for F_SEAL_FUTURE_WRITE based on the changes
introduced in previous patch.
Also add a test to make sure the reopen issue pointed by Jann Horn [1]
is fixed.
[1]
https://lore.kernel.org/lkml/CAG48ez1h=v-JYnDw81HaYJzOfrNhwYksxmc2r=cjvdqvgym...@mail.gmail.com/
Cc: Jann Horn
Signed-off-by: Joel Fernandes (Google)
---
tools/testing/selftests/memfd/memfd_test.c | 88 +++---
1 file changed, 44 insertions(+), 44 deletions(-)
diff --git a/tools/testing/selftests/memfd/memfd_test.c
b/tools/testing/selftests/memfd/memfd_test.c
index 32b207ca7372..c67d32eeb668 100644
--- a/tools/testing/selftests/memfd/memfd_test.c
+++ b/tools/testing/selftests/memfd/memfd_test.c
@@ -54,6 +54,22 @@ static int mfd_assert_new(const char *name, loff_t sz,
unsigned int flags)
return fd;
}
+static int mfd_assert_reopen_fd(int fd_in)
+{
+ int r, fd;
+ char path[100];
+
+ sprintf(path, "/proc/self/fd/%d", fd_in);
+
+ fd = open(path, O_RDWR);
+ if (fd < 0) {
+ printf("re-open of existing fd %d failed\n", fd_in);
+ abort();
+ }
+
+ return fd;
+}
+
static void mfd_fail_new(const char *name, unsigned int flags)
{
int r;
@@ -255,6 +271,25 @@ static void mfd_assert_read(int fd)
munmap(p, mfd_def_size);
}
+/* Test that PROT_READ + MAP_SHARED mappings work. */
+static void mfd_assert_read_shared(int fd)
+{
+ void *p;
+
+ /* verify PROT_READ and MAP_SHARED *is* allowed */
+ p = mmap(NULL,
+mfd_def_size,
+PROT_READ,
+MAP_SHARED,
+fd,
+0);
+ if (p == MAP_FAILED) {
+ printf("mmap() failed: %m\n");
+ abort();
+ }
+ munmap(p, mfd_def_size);
+}
+
static void mfd_assert_write(int fd)
{
ssize_t l;
@@ -698,7 +733,7 @@ static void test_seal_write(void)
*/
static void test_seal_future_write(void)
{
- int fd;
+ int fd, fd2;
void *p;
printf("%s SEAL-FUTURE-WRITE\n", memfd_str);
@@ -710,58 +745,23 @@ static void test_seal_future_write(void)
p = mfd_assert_mmap_shared(fd);
mfd_assert_has_seals(fd, 0);
- /* Not adding grow/shrink seals makes the future write
-* seal fail to get added
-*/
- mfd_fail_add_seals(fd, F_SEAL_FUTURE_WRITE);
-
- mfd_assert_add_seals(fd, F_SEAL_GROW);
- mfd_assert_has_seals(fd, F_SEAL_GROW);
-
- /* Should still fail since shrink seal has
-* not yet been added
-*/
- mfd_fail_add_seals(fd, F_SEAL_FUTURE_WRITE);
-
- mfd_assert_add_seals(fd, F_SEAL_SHRINK);
- mfd_assert_has_seals(fd, F_SEAL_GROW |
-F_SEAL_SHRINK);
- /* Now should succeed, also verifies that the seal
-* could be added with an existing writable mmap
-*/
mfd_assert_add_seals(fd, F_SEAL_FUTURE_WRITE);
- mfd_assert_has_seals(fd, F_SEAL_SHRINK |
-F_SEAL_GROW |
-F_SEAL_FUTURE_WRITE);
+ mfd_assert_has_seals(fd, F_SEAL_FUTURE_WRITE);
/* read should pass, writes should fail */
mfd_assert_read(fd);
+ mfd_assert_read_shared(fd);
mfd_fail_write(fd);
- munmap(p, mfd_def_size);
- close(fd);
-
- /* Test adding all seals (grow, shrink, future write) at once */
- fd = mfd_assert_new("kern_memfd_seal_future_write2",
- mfd_def_size,
- MFD_CLOEXEC | MFD_ALLOW_SEALING);
-
- p = mfd_assert_mmap_shared(fd);
-
- mfd_assert_has_seals(fd, 0);
- mfd_assert_add_seals(fd, F_SEAL_SHRINK |
-F_SEAL_GROW |
-F_SEAL_FUTURE_WRITE);
- mfd_assert_has_seals(fd, F_SEAL_SHRINK |
-F_SEAL_GROW |
-F_SEAL_FUTURE_WRITE);
-
- /* read should pass, writes should fail */
- mfd_assert_read(fd);
- mfd_fail_write(fd);
+ fd2 = mfd_assert_reopen_fd(fd);
+ /* read should pass, writes should still fail */
+ mfd_assert_read(fd2);
+ mfd_assert_read_shared(fd2);
+ mfd_fail_write(fd2);
munmap(p, mfd_def_size);
+ close(fd2);
close(fd);
}
--
2.19.1.1215.g8438c0b245-goog
ERROR: "numa_slit" [drivers/nvme/host/nvme-core.ko] undefined!
Hi Matias, FYI, the error/warning still remains. tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: f2ce1065e767fc7da106a5f5381d1e8f842dc6f4 commit: 73569e11032fc5a9b314b6351632cfca7793afd5 lightnvm: remove dependencies on BLK_DEV_NVME and PCI date: 6 weeks ago config: ia64-allmodconfig (attached as .config) compiler: ia64-linux-gcc (GCC) 8.1.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 73569e11032fc5a9b314b6351632cfca7793afd5 # save the attached .config to linux build tree GCC_VERSION=8.1.0 make.cross ARCH=ia64 All errors (new ones prefixed by >>): ERROR: "ia64_delay_loop" [drivers/spi/spi-thunderx.ko] undefined! >> ERROR: "numa_slit" [drivers/nvme/host/nvme-core.ko] undefined! ERROR: "__sw_hweight8" [drivers/net/wireless/mediatek/mt76/mt76.ko] undefined! ERROR: "ia64_delay_loop" [drivers/net/phy/mdio-cavium.ko] undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
ERROR: "numa_slit" [drivers/nvme/host/nvme-core.ko] undefined!
Hi Matias, FYI, the error/warning still remains. tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: f2ce1065e767fc7da106a5f5381d1e8f842dc6f4 commit: 73569e11032fc5a9b314b6351632cfca7793afd5 lightnvm: remove dependencies on BLK_DEV_NVME and PCI date: 6 weeks ago config: ia64-allmodconfig (attached as .config) compiler: ia64-linux-gcc (GCC) 8.1.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross git checkout 73569e11032fc5a9b314b6351632cfca7793afd5 # save the attached .config to linux build tree GCC_VERSION=8.1.0 make.cross ARCH=ia64 All errors (new ones prefixed by >>): ERROR: "ia64_delay_loop" [drivers/spi/spi-thunderx.ko] undefined! >> ERROR: "numa_slit" [drivers/nvme/host/nvme-core.ko] undefined! ERROR: "__sw_hweight8" [drivers/net/wireless/mediatek/mt76/mt76.ko] undefined! ERROR: "ia64_delay_loop" [drivers/net/phy/mdio-cavium.ko] undefined! --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH v2] mm: fix swap offset when replacing shmem page
On Mon, 19 Nov 2018, Yu Zhao wrote:
> On Mon, Nov 19, 2018 at 02:11:27PM -0800, Hugh Dickins wrote:
> > On Sun, 18 Nov 2018, Yu Zhao wrote:
> >
> > > We used to have a single swap address space with swp_entry_t.val
> > > as its radix tree index. This is not the case anymore. Now Each
> > > swp_type() has its own address space and should use swp_offset()
> > > as radix tree index.
> > >
> > > Signed-off-by: Yu Zhao
> >
> > This fix is a great find, thank you! But completely mis-described!
>
> Yes, now I remember making swap offset as key was done long after per
> swap device radix tree.
>
> > And could you do a smaller patch, keeping swap_index, that can go to
> > stable without getting into trouble with the recent xarrifications?
> >
> > Fixes: bde05d1ccd51 ("shmem: replace page if mapping excludes its zone")
> > Cc: sta...@vger.kernel.org # 3.5+
> >
> > Seems shmem_replace_page() has been wrong since the day I wrote it:
> > good enough to work on swap "type" 0, which is all most people ever use
> > (especially those few who need shmem_replace_page() at all), but broken
> > once there are any non-0 swp_type bits set in the higher order bits.
>
> But you did get it right when you wrote the function, which was before
> the per swap device radix tree. so
> Fixes: f6ab1f7f6b2d ("mm, swap: use offset of swap entry as key of swap
> cache")
> looks good?
Oh, you're right, thank you. Yes, the fix is to that one, in 4.9 onwards.
I don't much like my original use of the name "swap_index", when it was
not the index in a swapfile (though it was the index in the radix tree);
but it will become a correct name with your patch.
Though Matthew Wilcox seems to want us to avoid saying "radix tree"...
Hugh
Re: [PATCH v2] mm: fix swap offset when replacing shmem page
On Mon, 19 Nov 2018, Yu Zhao wrote:
> On Mon, Nov 19, 2018 at 02:11:27PM -0800, Hugh Dickins wrote:
> > On Sun, 18 Nov 2018, Yu Zhao wrote:
> >
> > > We used to have a single swap address space with swp_entry_t.val
> > > as its radix tree index. This is not the case anymore. Now Each
> > > swp_type() has its own address space and should use swp_offset()
> > > as radix tree index.
> > >
> > > Signed-off-by: Yu Zhao
> >
> > This fix is a great find, thank you! But completely mis-described!
>
> Yes, now I remember making swap offset as key was done long after per
> swap device radix tree.
>
> > And could you do a smaller patch, keeping swap_index, that can go to
> > stable without getting into trouble with the recent xarrifications?
> >
> > Fixes: bde05d1ccd51 ("shmem: replace page if mapping excludes its zone")
> > Cc: sta...@vger.kernel.org # 3.5+
> >
> > Seems shmem_replace_page() has been wrong since the day I wrote it:
> > good enough to work on swap "type" 0, which is all most people ever use
> > (especially those few who need shmem_replace_page() at all), but broken
> > once there are any non-0 swp_type bits set in the higher order bits.
>
> But you did get it right when you wrote the function, which was before
> the per swap device radix tree. so
> Fixes: f6ab1f7f6b2d ("mm, swap: use offset of swap entry as key of swap
> cache")
> looks good?
Oh, you're right, thank you. Yes, the fix is to that one, in 4.9 onwards.
I don't much like my original use of the name "swap_index", when it was
not the index in a swapfile (though it was the index in the radix tree);
but it will become a correct name with your patch.
Though Matthew Wilcox seems to want us to avoid saying "radix tree"...
Hugh
Re: [PATCH 12/16] clocksource: Add clock driver for RDA8810PL SoC
Hi Marc,
On Mon, Nov 19, 2018 at 05:57:12PM +, Marc Zyngier wrote:
> On 19/11/2018 17:09, Manivannan Sadhasivam wrote:
> > Add clock driver for RDA Micro RDA8810PL SoC supporting OSTIMER
> > and HWTIMER.
> >
> > Signed-off-by: Andreas Färber
> > Signed-off-by: Manivannan Sadhasivam
> > ---
> > arch/arm/mach-rda/Kconfig | 1 +
> > drivers/clocksource/Kconfig | 7 ++
> > drivers/clocksource/Makefile| 1 +
> > drivers/clocksource/timer-rda.c | 187
> > 4 files changed, 196 insertions(+)
> > create mode 100644 drivers/clocksource/timer-rda.c
> >
> > diff --git a/arch/arm/mach-rda/Kconfig b/arch/arm/mach-rda/Kconfig
> > index 29012bc68ca4..1ea753f57b2d 100644
> > --- a/arch/arm/mach-rda/Kconfig
> > +++ b/arch/arm/mach-rda/Kconfig
> > @@ -4,5 +4,6 @@ menuconfig ARCH_RDA
> > select COMMON_CLK
> > select GENERIC_IRQ_CHIP
> > select RDA_INTC
> > + select RDA_TIMER
> > help
> > This enables support for the RDA Micro 8810PL SoC family.
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index 55c77e44bb2d..f51eee3a72ea 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -105,6 +105,13 @@ config OWL_TIMER
> > help
> > Enables the support for the Actions Semi Owl timer driver.
> >
> > +config RDA_TIMER
> > + bool "RDA timer driver" if COMPILE_TEST
> > + depends on GENERIC_CLOCKEVENTS
> > + select CLKSRC_MMIO
> > + help
> > + Enables the support for the RDA Micro timer driver.
> > +
> > config SUN4I_TIMER
> > bool "Sun4i timer driver" if COMPILE_TEST
> > depends on HAS_IOMEM
> > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> > index dd9138104568..150020a90707 100644
> > --- a/drivers/clocksource/Makefile
> > +++ b/drivers/clocksource/Makefile
> > @@ -57,6 +57,7 @@ obj-$(CONFIG_OXNAS_RPS_TIMER) += timer-oxnas-rps.o
> > obj-$(CONFIG_OWL_TIMER)+= timer-owl.o
> > obj-$(CONFIG_SPRD_TIMER) += timer-sprd.o
> > obj-$(CONFIG_NPCM7XX_TIMER)+= timer-npcm7xx.o
> > +obj-$(CONFIG_RDA_TIMER)+= timer-rda.o
> >
> > obj-$(CONFIG_ARC_TIMERS) += arc_timer.o
> > obj-$(CONFIG_ARM_ARCH_TIMER) += arm_arch_timer.o
> > diff --git a/drivers/clocksource/timer-rda.c
> > b/drivers/clocksource/timer-rda.c
> > new file mode 100644
> > index ..3aa684d92c5d
> > --- /dev/null
> > +++ b/drivers/clocksource/timer-rda.c
> > @@ -0,0 +1,187 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * RDA8810PL SoC timer driver
> > + *
> > + * Copyright RDA Microelectronics Company Limited
> > + * Copyright (c) 2017 Andreas Färber
> > + * Copyright (c) 2018 Manivannan Sadhasivam
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define RDA_OSTIMER_LOADVAL_L 0x000
> > +#define RDA_OSTIMER_CTRL 0x004
> > +#define RDA_HWTIMER_LOCKVAL_L 0x024
> > +#define RDA_HWTIMER_LOCKVAL_H 0x028
> > +#define RDA_TIMER_IRQ_MASK_SET 0x02c
> > +#define RDA_TIMER_IRQ_CLR 0x034
> > +
> > +#define RDA_OSTIMER_CTRL_ENABLEBIT(24)
> > +#define RDA_OSTIMER_CTRL_REPEATBIT(28)
> > +#define RDA_OSTIMER_CTRL_LOAD BIT(30)
> > +
> > +#define RDA_TIMER_IRQ_MASK_SET_OSTIMER BIT(0)
> > +
> > +#define RDA_TIMER_IRQ_CLR_OSTIMER BIT(0)
> > +
> > +static void __iomem *rda_timer_base;
> > +
> > +static u64 rda_hwtimer_read(struct clocksource *cs)
> > +{
> > + u32 lo, hi;
> > +
> > + /* Always read low 32 bits first */
> > + lo = readl(rda_timer_base + RDA_HWTIMER_LOCKVAL_L);
> > + hi = readl(rda_timer_base + RDA_HWTIMER_LOCKVAL_H);
>
> Please use the relaxed accessors throughout this driver. There is zero
> reason to use the non-relaxed versions here.
>
Okay.
> Now, I'm pretty sure this thing isn't correct.
>
>
> lo = 0x;
>
> hi = 0x0001;
>
> Bingo. You cannot read a 64bit counter with only two 32bit accesses.
>
I think the lack of description makes confusion here. In this SoC, there
are two independent timers available: OSTIMER (56 bit) and HWTIMER (64 bit)
with optional interrupt support. I have used OSTIMER for clockevents and
HWTIMER for clocksource. Will add this information in driver.
Please let me know whether I have to model these two clocksources
differently!
> > +
> > + return ((u64)hi << 32) | lo;
> > +}
> > +
> > +static struct clocksource rda_clocksource = {
> > + .name = "rda-timer",
> > + .rating = 400,
> > + .read = rda_hwtimer_read,
> > + .mask = CLOCKSOURCE_MASK(64),
>
> This is a 64bit counter? See below.
>
Yes, this is the HWTIMER and is 64 bit.
> > + .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> > +};
> > +
> > +static int rda_ostimer_start(bool periodic, u64 cycles)
> > +{
> > + u32 ctrl, load_l;
> > +
> > + load_l = (u32)cycles;
>
[PATCH] perf vendor events intel: Fix Load_Miss_Real_Latency on SKL/SKX
From: Andi Kleen
Fix incorrect event names for the Load_Miss_Real_Latency metric for
Skylake and Skylake Server.
Fixes https://github.com/andikleen/pmu-tools/issues/158
Before:
% perf stat -M Load_Miss_Real_Latency true
event syntax error:
'..ss.pending,mem_load_retired.l1_miss_ps,mem_load_retired.fb_hit_ps}:W'
\___ parser error
Usage: perf stat [] []
-M, --metrics
monitor specified metrics or metric groups (separated
by ,)
After:
% perf stat -M Load_Miss_Real_Latency true
Performance counter stats for 'true':
279,204 l1d_pend_miss.pending # 14.0
Load_Miss_Real_Latency
4,784 mem_load_uops_retired.l1_miss
15,188 mem_load_uops_retired.hit_lfb
0.000899640 seconds time elapsed
Signed-off-by: Andi Kleen
---
tools/perf/pmu-events/arch/x86/skylake/skl-metrics.json | 2 +-
tools/perf/pmu-events/arch/x86/skylakex/skx-metrics.json | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/pmu-events/arch/x86/skylake/skl-metrics.json
b/tools/perf/pmu-events/arch/x86/skylake/skl-metrics.json
index 36c903faed0b..71e9737f4614 100644
--- a/tools/perf/pmu-events/arch/x86/skylake/skl-metrics.json
+++ b/tools/perf/pmu-events/arch/x86/skylake/skl-metrics.json
@@ -73,7 +73,7 @@
},
{
"BriefDescription": "Actual Average Latency for L1 data-cache miss
demand loads",
-"MetricExpr": "L1D_PEND_MISS.PENDING / ( MEM_LOAD_RETIRED.L1_MISS_PS +
MEM_LOAD_RETIRED.FB_HIT_PS )",
+"MetricExpr": "L1D_PEND_MISS.PENDING / ( MEM_LOAD_RETIRED.L1_MISS +
MEM_LOAD_RETIRED.FB_HIT )",
"MetricGroup": "Memory_Bound;Memory_Lat",
"MetricName": "Load_Miss_Real_Latency"
},
diff --git a/tools/perf/pmu-events/arch/x86/skylakex/skx-metrics.json
b/tools/perf/pmu-events/arch/x86/skylakex/skx-metrics.json
index 36c903faed0b..71e9737f4614 100644
--- a/tools/perf/pmu-events/arch/x86/skylakex/skx-metrics.json
+++ b/tools/perf/pmu-events/arch/x86/skylakex/skx-metrics.json
@@ -73,7 +73,7 @@
},
{
"BriefDescription": "Actual Average Latency for L1 data-cache miss
demand loads",
-"MetricExpr": "L1D_PEND_MISS.PENDING / ( MEM_LOAD_RETIRED.L1_MISS_PS +
MEM_LOAD_RETIRED.FB_HIT_PS )",
+"MetricExpr": "L1D_PEND_MISS.PENDING / ( MEM_LOAD_RETIRED.L1_MISS +
MEM_LOAD_RETIRED.FB_HIT )",
"MetricGroup": "Memory_Bound;Memory_Lat",
"MetricName": "Load_Miss_Real_Latency"
},
--
2.17.2
Re: [PATCH 12/16] clocksource: Add clock driver for RDA8810PL SoC
Hi Marc,
On Mon, Nov 19, 2018 at 05:57:12PM +, Marc Zyngier wrote:
> On 19/11/2018 17:09, Manivannan Sadhasivam wrote:
> > Add clock driver for RDA Micro RDA8810PL SoC supporting OSTIMER
> > and HWTIMER.
> >
> > Signed-off-by: Andreas Färber
> > Signed-off-by: Manivannan Sadhasivam
> > ---
> > arch/arm/mach-rda/Kconfig | 1 +
> > drivers/clocksource/Kconfig | 7 ++
> > drivers/clocksource/Makefile| 1 +
> > drivers/clocksource/timer-rda.c | 187
> > 4 files changed, 196 insertions(+)
> > create mode 100644 drivers/clocksource/timer-rda.c
> >
> > diff --git a/arch/arm/mach-rda/Kconfig b/arch/arm/mach-rda/Kconfig
> > index 29012bc68ca4..1ea753f57b2d 100644
> > --- a/arch/arm/mach-rda/Kconfig
> > +++ b/arch/arm/mach-rda/Kconfig
> > @@ -4,5 +4,6 @@ menuconfig ARCH_RDA
> > select COMMON_CLK
> > select GENERIC_IRQ_CHIP
> > select RDA_INTC
> > + select RDA_TIMER
> > help
> > This enables support for the RDA Micro 8810PL SoC family.
> > diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> > index 55c77e44bb2d..f51eee3a72ea 100644
> > --- a/drivers/clocksource/Kconfig
> > +++ b/drivers/clocksource/Kconfig
> > @@ -105,6 +105,13 @@ config OWL_TIMER
> > help
> > Enables the support for the Actions Semi Owl timer driver.
> >
> > +config RDA_TIMER
> > + bool "RDA timer driver" if COMPILE_TEST
> > + depends on GENERIC_CLOCKEVENTS
> > + select CLKSRC_MMIO
> > + help
> > + Enables the support for the RDA Micro timer driver.
> > +
> > config SUN4I_TIMER
> > bool "Sun4i timer driver" if COMPILE_TEST
> > depends on HAS_IOMEM
> > diff --git a/drivers/clocksource/Makefile b/drivers/clocksource/Makefile
> > index dd9138104568..150020a90707 100644
> > --- a/drivers/clocksource/Makefile
> > +++ b/drivers/clocksource/Makefile
> > @@ -57,6 +57,7 @@ obj-$(CONFIG_OXNAS_RPS_TIMER) += timer-oxnas-rps.o
> > obj-$(CONFIG_OWL_TIMER)+= timer-owl.o
> > obj-$(CONFIG_SPRD_TIMER) += timer-sprd.o
> > obj-$(CONFIG_NPCM7XX_TIMER)+= timer-npcm7xx.o
> > +obj-$(CONFIG_RDA_TIMER)+= timer-rda.o
> >
> > obj-$(CONFIG_ARC_TIMERS) += arc_timer.o
> > obj-$(CONFIG_ARM_ARCH_TIMER) += arm_arch_timer.o
> > diff --git a/drivers/clocksource/timer-rda.c
> > b/drivers/clocksource/timer-rda.c
> > new file mode 100644
> > index ..3aa684d92c5d
> > --- /dev/null
> > +++ b/drivers/clocksource/timer-rda.c
> > @@ -0,0 +1,187 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * RDA8810PL SoC timer driver
> > + *
> > + * Copyright RDA Microelectronics Company Limited
> > + * Copyright (c) 2017 Andreas Färber
> > + * Copyright (c) 2018 Manivannan Sadhasivam
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define RDA_OSTIMER_LOADVAL_L 0x000
> > +#define RDA_OSTIMER_CTRL 0x004
> > +#define RDA_HWTIMER_LOCKVAL_L 0x024
> > +#define RDA_HWTIMER_LOCKVAL_H 0x028
> > +#define RDA_TIMER_IRQ_MASK_SET 0x02c
> > +#define RDA_TIMER_IRQ_CLR 0x034
> > +
> > +#define RDA_OSTIMER_CTRL_ENABLEBIT(24)
> > +#define RDA_OSTIMER_CTRL_REPEATBIT(28)
> > +#define RDA_OSTIMER_CTRL_LOAD BIT(30)
> > +
> > +#define RDA_TIMER_IRQ_MASK_SET_OSTIMER BIT(0)
> > +
> > +#define RDA_TIMER_IRQ_CLR_OSTIMER BIT(0)
> > +
> > +static void __iomem *rda_timer_base;
> > +
> > +static u64 rda_hwtimer_read(struct clocksource *cs)
> > +{
> > + u32 lo, hi;
> > +
> > + /* Always read low 32 bits first */
> > + lo = readl(rda_timer_base + RDA_HWTIMER_LOCKVAL_L);
> > + hi = readl(rda_timer_base + RDA_HWTIMER_LOCKVAL_H);
>
> Please use the relaxed accessors throughout this driver. There is zero
> reason to use the non-relaxed versions here.
>
Okay.
> Now, I'm pretty sure this thing isn't correct.
>
>
> lo = 0x;
>
> hi = 0x0001;
>
> Bingo. You cannot read a 64bit counter with only two 32bit accesses.
>
I think the lack of description makes confusion here. In this SoC, there
are two independent timers available: OSTIMER (56 bit) and HWTIMER (64 bit)
with optional interrupt support. I have used OSTIMER for clockevents and
HWTIMER for clocksource. Will add this information in driver.
Please let me know whether I have to model these two clocksources
differently!
> > +
> > + return ((u64)hi << 32) | lo;
> > +}
> > +
> > +static struct clocksource rda_clocksource = {
> > + .name = "rda-timer",
> > + .rating = 400,
> > + .read = rda_hwtimer_read,
> > + .mask = CLOCKSOURCE_MASK(64),
>
> This is a 64bit counter? See below.
>
Yes, this is the HWTIMER and is 64 bit.
> > + .flags = CLOCK_SOURCE_IS_CONTINUOUS,
> > +};
> > +
> > +static int rda_ostimer_start(bool periodic, u64 cycles)
> > +{
> > + u32 ctrl, load_l;
> > +
> > + load_l = (u32)cycles;
>
[PATCH] perf vendor events intel: Fix Load_Miss_Real_Latency on SKL/SKX
From: Andi Kleen
Fix incorrect event names for the Load_Miss_Real_Latency metric for
Skylake and Skylake Server.
Fixes https://github.com/andikleen/pmu-tools/issues/158
Before:
% perf stat -M Load_Miss_Real_Latency true
event syntax error:
'..ss.pending,mem_load_retired.l1_miss_ps,mem_load_retired.fb_hit_ps}:W'
\___ parser error
Usage: perf stat [] []
-M, --metrics
monitor specified metrics or metric groups (separated
by ,)
After:
% perf stat -M Load_Miss_Real_Latency true
Performance counter stats for 'true':
279,204 l1d_pend_miss.pending # 14.0
Load_Miss_Real_Latency
4,784 mem_load_uops_retired.l1_miss
15,188 mem_load_uops_retired.hit_lfb
0.000899640 seconds time elapsed
Signed-off-by: Andi Kleen
---
tools/perf/pmu-events/arch/x86/skylake/skl-metrics.json | 2 +-
tools/perf/pmu-events/arch/x86/skylakex/skx-metrics.json | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/perf/pmu-events/arch/x86/skylake/skl-metrics.json
b/tools/perf/pmu-events/arch/x86/skylake/skl-metrics.json
index 36c903faed0b..71e9737f4614 100644
--- a/tools/perf/pmu-events/arch/x86/skylake/skl-metrics.json
+++ b/tools/perf/pmu-events/arch/x86/skylake/skl-metrics.json
@@ -73,7 +73,7 @@
},
{
"BriefDescription": "Actual Average Latency for L1 data-cache miss
demand loads",
-"MetricExpr": "L1D_PEND_MISS.PENDING / ( MEM_LOAD_RETIRED.L1_MISS_PS +
MEM_LOAD_RETIRED.FB_HIT_PS )",
+"MetricExpr": "L1D_PEND_MISS.PENDING / ( MEM_LOAD_RETIRED.L1_MISS +
MEM_LOAD_RETIRED.FB_HIT )",
"MetricGroup": "Memory_Bound;Memory_Lat",
"MetricName": "Load_Miss_Real_Latency"
},
diff --git a/tools/perf/pmu-events/arch/x86/skylakex/skx-metrics.json
b/tools/perf/pmu-events/arch/x86/skylakex/skx-metrics.json
index 36c903faed0b..71e9737f4614 100644
--- a/tools/perf/pmu-events/arch/x86/skylakex/skx-metrics.json
+++ b/tools/perf/pmu-events/arch/x86/skylakex/skx-metrics.json
@@ -73,7 +73,7 @@
},
{
"BriefDescription": "Actual Average Latency for L1 data-cache miss
demand loads",
-"MetricExpr": "L1D_PEND_MISS.PENDING / ( MEM_LOAD_RETIRED.L1_MISS_PS +
MEM_LOAD_RETIRED.FB_HIT_PS )",
+"MetricExpr": "L1D_PEND_MISS.PENDING / ( MEM_LOAD_RETIRED.L1_MISS +
MEM_LOAD_RETIRED.FB_HIT )",
"MetricGroup": "Memory_Bound;Memory_Lat",
"MetricName": "Load_Miss_Real_Latency"
},
--
2.17.2
[PATCH] perf script: Fix LBR skid dump problems in brstackinsn
From: Andi Kleen
This is a fix for another instance of the skid problem Milian
recently found [1]
The LBRs don't freeze at the exact same time as the PMI is triggered.
The perf script brstackinsn code that dumps LBR assembler
assumes that the last branch in the LBR leads to the sample point.
But with skid it's possible that the CPU executes one or more branches
before the sample, but which do not appear in the LBR.
What happens then is either that the sample point is before
the last LBR branch. In this case the dumper sees a negative
length and ignores it. Or it the sample point is long after
the last branch. Then the dumper sees a very long block and dumps
it upto its block limit (16k bytes), which is noise in the output.
On typical sample session this can happen regularly.
This patch tries to detect and handle the situation. On the last
block that is dumped by the LBR dumper we always stop on the first
branch. If the block length is negative just scan forward to the
first branch. Otherwise scan until a branch is found.
The PT decoder already has a function that uses the instruction
decoder to detect branches, so we can just reuse it here.
Then when a terminating branch is found print an indication
and stop dumping. This might miss a few instructions, but at least
shows no runaway blocks.
Cc: milian.wo...@kdab.com
Cc: adrian.hun...@intel.com
Signed-off-by: Andi Kleen
---
tools/perf/builtin-script.c| 18 +-
tools/perf/util/dump-insn.c| 8
tools/perf/util/dump-insn.h| 2 ++
.../intel-pt-decoder/intel-pt-insn-decoder.c | 8
4 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 4da5e32b9e03..11868bf39e66 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1049,9 +1049,18 @@ static int perf_sample__fprintf_brstackinsn(struct
perf_sample *sample,
/*
* Print final block upto sample
+*
+* Due to pipeline delays the LBRs might be missing a branch
+* or two, which can result in very large or negative blocks
+* between final branch and sample. When this happens just
+* continue walking after the last TO until we hit a branch.
*/
start = br->entries[0].to;
end = sample->ip;
+ if (end < start) {
+ /* Missing jump. Scan 128 bytes for the next branch */
+ end = start + 128;
+ }
len = grab_bb(buffer, start, end, machine, thread, ,
, true);
printed += ip__fprintf_sym(start, thread, x.cpumode, x.cpu, ,
attr, fp);
if (len <= 0) {
@@ -1060,7 +1069,6 @@ static int perf_sample__fprintf_brstackinsn(struct
perf_sample *sample,
machine, thread, , , false);
if (len <= 0)
goto out;
-
printed += fprintf(fp, "\t%016" PRIx64 "\t%s\n", sample->ip,
dump_insn(, sample->ip, buffer, len, NULL));
goto out;
@@ -1070,6 +1078,14 @@ static int perf_sample__fprintf_brstackinsn(struct
perf_sample *sample,
dump_insn(, start + off, buffer + off, len
- off, ));
if (ilen == 0)
break;
+ if (arch_is_branch(buffer + off, len - off, x.is64bit) &&
+ start + off != sample->ip) {
+ /*
+* Hit a missing branch. Just stop.
+*/
+ printed += fprintf(fp, "\t... not reaching sample
...\n");
+ break;
+ }
}
out:
return printed;
diff --git a/tools/perf/util/dump-insn.c b/tools/perf/util/dump-insn.c
index 10988d3de7ce..2bd8585db93c 100644
--- a/tools/perf/util/dump-insn.c
+++ b/tools/perf/util/dump-insn.c
@@ -13,3 +13,11 @@ const char *dump_insn(struct perf_insn *x __maybe_unused,
*lenp = 0;
return "?";
}
+
+__weak
+int arch_is_branch(const unsigned char *buf __maybe_unused,
+ size_t len __maybe_unused,
+ int x86_64 __maybe_unused)
+{
+ return 0;
+}
diff --git a/tools/perf/util/dump-insn.h b/tools/perf/util/dump-insn.h
index 0e06280a8860..650125061530 100644
--- a/tools/perf/util/dump-insn.h
+++ b/tools/perf/util/dump-insn.h
@@ -20,4 +20,6 @@ struct perf_insn {
const char *dump_insn(struct perf_insn *x, u64 ip,
u8 *inbuf, int inlen, int *lenp);
+int arch_is_branch(const unsigned char *buf, size_t len, int x86_64);
+
#endif
diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
index 54818828023b..1c0e289f01e6 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
@@ -180,6 +180,14 @@ int
[PATCH] perf script: Fix LBR skid dump problems in brstackinsn
From: Andi Kleen
This is a fix for another instance of the skid problem Milian
recently found [1]
The LBRs don't freeze at the exact same time as the PMI is triggered.
The perf script brstackinsn code that dumps LBR assembler
assumes that the last branch in the LBR leads to the sample point.
But with skid it's possible that the CPU executes one or more branches
before the sample, but which do not appear in the LBR.
What happens then is either that the sample point is before
the last LBR branch. In this case the dumper sees a negative
length and ignores it. Or it the sample point is long after
the last branch. Then the dumper sees a very long block and dumps
it upto its block limit (16k bytes), which is noise in the output.
On typical sample session this can happen regularly.
This patch tries to detect and handle the situation. On the last
block that is dumped by the LBR dumper we always stop on the first
branch. If the block length is negative just scan forward to the
first branch. Otherwise scan until a branch is found.
The PT decoder already has a function that uses the instruction
decoder to detect branches, so we can just reuse it here.
Then when a terminating branch is found print an indication
and stop dumping. This might miss a few instructions, but at least
shows no runaway blocks.
Cc: milian.wo...@kdab.com
Cc: adrian.hun...@intel.com
Signed-off-by: Andi Kleen
---
tools/perf/builtin-script.c| 18 +-
tools/perf/util/dump-insn.c| 8
tools/perf/util/dump-insn.h| 2 ++
.../intel-pt-decoder/intel-pt-insn-decoder.c | 8
4 files changed, 35 insertions(+), 1 deletion(-)
diff --git a/tools/perf/builtin-script.c b/tools/perf/builtin-script.c
index 4da5e32b9e03..11868bf39e66 100644
--- a/tools/perf/builtin-script.c
+++ b/tools/perf/builtin-script.c
@@ -1049,9 +1049,18 @@ static int perf_sample__fprintf_brstackinsn(struct
perf_sample *sample,
/*
* Print final block upto sample
+*
+* Due to pipeline delays the LBRs might be missing a branch
+* or two, which can result in very large or negative blocks
+* between final branch and sample. When this happens just
+* continue walking after the last TO until we hit a branch.
*/
start = br->entries[0].to;
end = sample->ip;
+ if (end < start) {
+ /* Missing jump. Scan 128 bytes for the next branch */
+ end = start + 128;
+ }
len = grab_bb(buffer, start, end, machine, thread, ,
, true);
printed += ip__fprintf_sym(start, thread, x.cpumode, x.cpu, ,
attr, fp);
if (len <= 0) {
@@ -1060,7 +1069,6 @@ static int perf_sample__fprintf_brstackinsn(struct
perf_sample *sample,
machine, thread, , , false);
if (len <= 0)
goto out;
-
printed += fprintf(fp, "\t%016" PRIx64 "\t%s\n", sample->ip,
dump_insn(, sample->ip, buffer, len, NULL));
goto out;
@@ -1070,6 +1078,14 @@ static int perf_sample__fprintf_brstackinsn(struct
perf_sample *sample,
dump_insn(, start + off, buffer + off, len
- off, ));
if (ilen == 0)
break;
+ if (arch_is_branch(buffer + off, len - off, x.is64bit) &&
+ start + off != sample->ip) {
+ /*
+* Hit a missing branch. Just stop.
+*/
+ printed += fprintf(fp, "\t... not reaching sample
...\n");
+ break;
+ }
}
out:
return printed;
diff --git a/tools/perf/util/dump-insn.c b/tools/perf/util/dump-insn.c
index 10988d3de7ce..2bd8585db93c 100644
--- a/tools/perf/util/dump-insn.c
+++ b/tools/perf/util/dump-insn.c
@@ -13,3 +13,11 @@ const char *dump_insn(struct perf_insn *x __maybe_unused,
*lenp = 0;
return "?";
}
+
+__weak
+int arch_is_branch(const unsigned char *buf __maybe_unused,
+ size_t len __maybe_unused,
+ int x86_64 __maybe_unused)
+{
+ return 0;
+}
diff --git a/tools/perf/util/dump-insn.h b/tools/perf/util/dump-insn.h
index 0e06280a8860..650125061530 100644
--- a/tools/perf/util/dump-insn.h
+++ b/tools/perf/util/dump-insn.h
@@ -20,4 +20,6 @@ struct perf_insn {
const char *dump_insn(struct perf_insn *x, u64 ip,
u8 *inbuf, int inlen, int *lenp);
+int arch_is_branch(const unsigned char *buf, size_t len, int x86_64);
+
#endif
diff --git a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
index 54818828023b..1c0e289f01e6 100644
--- a/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
+++ b/tools/perf/util/intel-pt-decoder/intel-pt-insn-decoder.c
@@ -180,6 +180,14 @@ int
linux-next: Tree for Nov 20
Hi all, Changes since 20181119: The regulator tree gained a build failure so I used the version from next-20181119. The tip tree still had its build failure for which I applied a fix patch. The staging tree gained a conflict against the drm tree. Non-merge commits (relative to Linus' tree): 3686 3827 files changed, 163514 insertions(+), 119003 deletions(-) I have created today's linux-next tree at git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (patches at http://www.kernel.org/pub/linux/kernel/next/ ). If you are tracking the linux-next tree using git, you should not use "git pull" to do so as that will try to merge the new linux-next release with the old one. You should use "git fetch" and checkout or reset to the new master. You can see which trees have been included by looking in the Next/Trees file in the source. There are also quilt-import.log and merge.log files in the Next directory. Between each merge, the tree was built with a ppc64_defconfig for powerpc, an allmodconfig for x86_64, a multi_v7_defconfig for arm and a native build of tools/perf. After the final fixups (if any), I do an x86_64 modules_install followed by builds for x86_64 allnoconfig, powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig and pseries_le_defconfig and i386, sparc and sparc64 defconfig. And finally, a simple boot test of the powerpc pseries_le_defconfig kernel in qemu (with and without kvm enabled). Below is a summary of the state of the merge. I am currently merging 286 trees (counting Linus' and 67 trees of bug fix patches pending for the current merge release). Stats about the size of the tree over time can be seen at http://neuling.org/linux-next-size.html . Status of my local build tests will be at http://kisskb.ellerman.id.au/linux-next . If maintainers want to give advice about cross compilers/configs that work, we are always open to add more builds. Thanks to Randy Dunlap for doing many randconfig builds. And to Paul Gortmaker for triage and bug fixes. -- Cheers, Stephen Rothwell $ git checkout master $ git reset --hard stable Merging origin/master (f2ce1065e767 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net) Merging fixes/master (7c6c54b505b8 Merge branch 'i2c/for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux) Merging kbuild-current/fixes (ccda4af0f4b9 Linux 4.20-rc2) Merging arc-current/for-curr (121e38e5acdc ARC: mm: fix uninitialised signal code in do_page_fault) Merging arm-current/fixes (e46daee53bb5 ARM: 8806/1: kprobes: Fix false positive with FORTIFY_SOURCE) Merging arm64-fixes/for-next/fixes (24cc61d8cb5a arm64: memblock: don't permit memblock resizing until linear mapping is up) Merging m68k-current/for-linus (58c116fb7dc6 m68k/sun3: Remove is_medusa and m68k_pgtable_cachemode) Merging powerpc-fixes/fixes (b2fed34a628d selftests/powerpc: Adjust wild_bctr to build with old binutils) Merging sparc/master (86322ba9571a arch/sparc: Use kzalloc_node) Merging fscrypt-current/for-stable (ae64f9bd1d36 Linux 4.15-rc2) Merging net/master (02968ccf0125 sctp: count sk_wmem_alloc by skb truesize in sctp_packet_transmit) Merging bpf/master (569a933b03f3 bpf: allocate local storage buffers using GFP_ATOMIC) Merging ipsec/master (ca92e173ab34 xfrm: Fix bucket count reported to userspace) Merging netfilter/master (38d37baa8123 ipvs: call ip_vs_dst_notifier earlier than ipv6_dev_notf) Merging ipvs/master (feb9f55c33e5 netfilter: nft_dynset: allow dynamic updates of non-anonymous set) Merging wireless-drivers/master (1770f0fa978e mt76: fix uninitialized mutex access setting rts threshold) Merging mac80211/master (113f3aaa81bd cfg80211: Prevent regulatory restore during STA disconnect in concurrent interfaces) Merging rdma-fixes/for-rc (99b77fef3c6c net/mlx5: Fix XRC SRQ umem valid bits) Merging sound-current/for-linus (a6b0961b3989 ALSA: hda/ca0132 - fix AE-5 pincfg) Merging sound-asoc-fixes/for-linus (aa3b61b00b20 Merge branch 'asoc-4.20' into asoc-linus) Merging regmap-fixes/for-linus (9ff01193a20d Linux 4.20-rc3) Merging regulator-fixes/for-linus (ee9a51949580 Merge branch 'regulator-4.20' into regulator-linus) Merging spi-fixes/for-linus (c25bc4f2c8df Merge branch 'spi-4.20' into spi-linus) Merging pci-current/for-linus (1a87119b7bcf Revert "ACPI/PCI: Pay attention to device-specific _PXM node values") Merging driver-core.current/driver-core-linus (a66d972465d1 devres: Align data[] to ARCH_KMALLOC_MINALIGN) Merging tty.current/tty-linus (ccda4af0f4b9 Linux 4.20-rc2) Merging usb.current/usb-linus (2f31a67f01a8 usb: xhci: Prevent bus suspend if a port connect change or polling state is detected) Merging usb-gadget-fixes/fixes (2fc6d4be35fb usb: dwc3: gadget: fix ISOC TRB type on unaligned transfers) Merging usb-serial-fixes/usb-linus (ccda4af0f4b9 Linux 4.20-rc2) Merging usb-chipidea-fixes/ci-fo
linux-next: Tree for Nov 20
Hi all, Changes since 20181119: The regulator tree gained a build failure so I used the version from next-20181119. The tip tree still had its build failure for which I applied a fix patch. The staging tree gained a conflict against the drm tree. Non-merge commits (relative to Linus' tree): 3686 3827 files changed, 163514 insertions(+), 119003 deletions(-) I have created today's linux-next tree at git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (patches at http://www.kernel.org/pub/linux/kernel/next/ ). If you are tracking the linux-next tree using git, you should not use "git pull" to do so as that will try to merge the new linux-next release with the old one. You should use "git fetch" and checkout or reset to the new master. You can see which trees have been included by looking in the Next/Trees file in the source. There are also quilt-import.log and merge.log files in the Next directory. Between each merge, the tree was built with a ppc64_defconfig for powerpc, an allmodconfig for x86_64, a multi_v7_defconfig for arm and a native build of tools/perf. After the final fixups (if any), I do an x86_64 modules_install followed by builds for x86_64 allnoconfig, powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig and pseries_le_defconfig and i386, sparc and sparc64 defconfig. And finally, a simple boot test of the powerpc pseries_le_defconfig kernel in qemu (with and without kvm enabled). Below is a summary of the state of the merge. I am currently merging 286 trees (counting Linus' and 67 trees of bug fix patches pending for the current merge release). Stats about the size of the tree over time can be seen at http://neuling.org/linux-next-size.html . Status of my local build tests will be at http://kisskb.ellerman.id.au/linux-next . If maintainers want to give advice about cross compilers/configs that work, we are always open to add more builds. Thanks to Randy Dunlap for doing many randconfig builds. And to Paul Gortmaker for triage and bug fixes. -- Cheers, Stephen Rothwell $ git checkout master $ git reset --hard stable Merging origin/master (f2ce1065e767 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net) Merging fixes/master (7c6c54b505b8 Merge branch 'i2c/for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux) Merging kbuild-current/fixes (ccda4af0f4b9 Linux 4.20-rc2) Merging arc-current/for-curr (121e38e5acdc ARC: mm: fix uninitialised signal code in do_page_fault) Merging arm-current/fixes (e46daee53bb5 ARM: 8806/1: kprobes: Fix false positive with FORTIFY_SOURCE) Merging arm64-fixes/for-next/fixes (24cc61d8cb5a arm64: memblock: don't permit memblock resizing until linear mapping is up) Merging m68k-current/for-linus (58c116fb7dc6 m68k/sun3: Remove is_medusa and m68k_pgtable_cachemode) Merging powerpc-fixes/fixes (b2fed34a628d selftests/powerpc: Adjust wild_bctr to build with old binutils) Merging sparc/master (86322ba9571a arch/sparc: Use kzalloc_node) Merging fscrypt-current/for-stable (ae64f9bd1d36 Linux 4.15-rc2) Merging net/master (02968ccf0125 sctp: count sk_wmem_alloc by skb truesize in sctp_packet_transmit) Merging bpf/master (569a933b03f3 bpf: allocate local storage buffers using GFP_ATOMIC) Merging ipsec/master (ca92e173ab34 xfrm: Fix bucket count reported to userspace) Merging netfilter/master (38d37baa8123 ipvs: call ip_vs_dst_notifier earlier than ipv6_dev_notf) Merging ipvs/master (feb9f55c33e5 netfilter: nft_dynset: allow dynamic updates of non-anonymous set) Merging wireless-drivers/master (1770f0fa978e mt76: fix uninitialized mutex access setting rts threshold) Merging mac80211/master (113f3aaa81bd cfg80211: Prevent regulatory restore during STA disconnect in concurrent interfaces) Merging rdma-fixes/for-rc (99b77fef3c6c net/mlx5: Fix XRC SRQ umem valid bits) Merging sound-current/for-linus (a6b0961b3989 ALSA: hda/ca0132 - fix AE-5 pincfg) Merging sound-asoc-fixes/for-linus (aa3b61b00b20 Merge branch 'asoc-4.20' into asoc-linus) Merging regmap-fixes/for-linus (9ff01193a20d Linux 4.20-rc3) Merging regulator-fixes/for-linus (ee9a51949580 Merge branch 'regulator-4.20' into regulator-linus) Merging spi-fixes/for-linus (c25bc4f2c8df Merge branch 'spi-4.20' into spi-linus) Merging pci-current/for-linus (1a87119b7bcf Revert "ACPI/PCI: Pay attention to device-specific _PXM node values") Merging driver-core.current/driver-core-linus (a66d972465d1 devres: Align data[] to ARCH_KMALLOC_MINALIGN) Merging tty.current/tty-linus (ccda4af0f4b9 Linux 4.20-rc2) Merging usb.current/usb-linus (2f31a67f01a8 usb: xhci: Prevent bus suspend if a port connect change or polling state is detected) Merging usb-gadget-fixes/fixes (2fc6d4be35fb usb: dwc3: gadget: fix ISOC TRB type on unaligned transfers) Merging usb-serial-fixes/usb-linus (ccda4af0f4b9 Linux 4.20-rc2) Merging usb-chipidea-fixes/ci-fo
Re: [PATCH 4/4] mmc: sdhci-omap: Remove redundant structure assignments
On 19/11/18 4:46 PM, Faiz Abbas wrote: The sdhci_execute_tuning() function has assignment of private pointers multiple times. Remove the redundant assignment. Signed-off-by: Faiz Abbas Acked-by: Kishon Vijay Abraham I --- drivers/mmc/host/sdhci-omap.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c index bf268b38ddc8..b3cb39d0db6f 100644 --- a/drivers/mmc/host/sdhci-omap.c +++ b/drivers/mmc/host/sdhci-omap.c @@ -295,10 +295,6 @@ static int sdhci_omap_execute_tuning(struct mmc_host *mmc, u32 opcode) int ret = 0; u32 reg; - pltfm_host = sdhci_priv(host); - omap_host = sdhci_pltfm_priv(pltfm_host); - dev = omap_host->dev; - /* clock tuning is not needed for upto 52MHz */ if (ios->clock <= 5200) return 0;
Re: [PATCH 4/4] mmc: sdhci-omap: Remove redundant structure assignments
On 19/11/18 4:46 PM, Faiz Abbas wrote: The sdhci_execute_tuning() function has assignment of private pointers multiple times. Remove the redundant assignment. Signed-off-by: Faiz Abbas Acked-by: Kishon Vijay Abraham I --- drivers/mmc/host/sdhci-omap.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c index bf268b38ddc8..b3cb39d0db6f 100644 --- a/drivers/mmc/host/sdhci-omap.c +++ b/drivers/mmc/host/sdhci-omap.c @@ -295,10 +295,6 @@ static int sdhci_omap_execute_tuning(struct mmc_host *mmc, u32 opcode) int ret = 0; u32 reg; - pltfm_host = sdhci_priv(host); - omap_host = sdhci_pltfm_priv(pltfm_host); - dev = omap_host->dev; - /* clock tuning is not needed for upto 52MHz */ if (ios->clock <= 5200) return 0;
Re: [PATCH 3/4] mmc: sdhci-omap: Add platform specific reset callback
On 19/11/18 4:46 PM, Faiz Abbas wrote:
The TRM (SPRUIC2C - January 2017 - Revised May 2018 [1]) forbids
assertion of data reset while tuning is happening. Implement a
platform specific callback that takes care of this condition.
[1] http://www.ti.com/lit/pdf/spruic2 Section 25.5.1.2.4
Signed-off-by: Faiz Abbas
Once patch 2 is merged with this patch, you can add
Acked-by: Kishon Vijay Abraham I
---
drivers/mmc/host/sdhci-omap.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index cfffcf58be3f..bf268b38ddc8 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -688,6 +688,18 @@ static void sdhci_omap_set_uhs_signaling(struct sdhci_host
*host,
sdhci_omap_start_clock(omap_host);
}
+void sdhci_omap_reset(struct sdhci_host *host, u8 mask)
+{
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
+
+ /* Don't reset data lines during tuning operation */
+ if (omap_host->is_tuning)
+ mask &= ~SDHCI_RESET_DATA;
+
+ sdhci_reset(host, mask);
+}
+
static struct sdhci_ops sdhci_omap_ops = {
.set_clock = sdhci_omap_set_clock,
.set_power = sdhci_omap_set_power,
@@ -696,7 +708,7 @@ static struct sdhci_ops sdhci_omap_ops = {
.get_min_clock = sdhci_omap_get_min_clock,
.set_bus_width = sdhci_omap_set_bus_width,
.platform_send_init_74_clocks = sdhci_omap_init_74_clocks,
- .reset = sdhci_reset,
+ .reset = sdhci_omap_reset,
.set_uhs_signaling = sdhci_omap_set_uhs_signaling,
};
Re: [PATCH 3/4] mmc: sdhci-omap: Add platform specific reset callback
On 19/11/18 4:46 PM, Faiz Abbas wrote:
The TRM (SPRUIC2C - January 2017 - Revised May 2018 [1]) forbids
assertion of data reset while tuning is happening. Implement a
platform specific callback that takes care of this condition.
[1] http://www.ti.com/lit/pdf/spruic2 Section 25.5.1.2.4
Signed-off-by: Faiz Abbas
Once patch 2 is merged with this patch, you can add
Acked-by: Kishon Vijay Abraham I
---
drivers/mmc/host/sdhci-omap.c | 14 +-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index cfffcf58be3f..bf268b38ddc8 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -688,6 +688,18 @@ static void sdhci_omap_set_uhs_signaling(struct sdhci_host
*host,
sdhci_omap_start_clock(omap_host);
}
+void sdhci_omap_reset(struct sdhci_host *host, u8 mask)
+{
+ struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
+ struct sdhci_omap_host *omap_host = sdhci_pltfm_priv(pltfm_host);
+
+ /* Don't reset data lines during tuning operation */
+ if (omap_host->is_tuning)
+ mask &= ~SDHCI_RESET_DATA;
+
+ sdhci_reset(host, mask);
+}
+
static struct sdhci_ops sdhci_omap_ops = {
.set_clock = sdhci_omap_set_clock,
.set_power = sdhci_omap_set_power,
@@ -696,7 +708,7 @@ static struct sdhci_ops sdhci_omap_ops = {
.get_min_clock = sdhci_omap_get_min_clock,
.set_bus_width = sdhci_omap_set_bus_width,
.platform_send_init_74_clocks = sdhci_omap_init_74_clocks,
- .reset = sdhci_reset,
+ .reset = sdhci_omap_reset,
.set_uhs_signaling = sdhci_omap_set_uhs_signaling,
};
Re: [PATCH v1 2/2] signal: add procfd_signal() syscall
Daniel Colascione writes: > On Mon, Nov 19, 2018 at 1:37 PM Christian Brauner > wrote: >> >> On Mon, Nov 19, 2018 at 01:26:22PM -0800, Daniel Colascione wrote: >> > On Mon, Nov 19, 2018 at 1:21 PM, Christian Brauner >> > wrote: >> > > That can be done without a loop by comparing the level counter for the >> > > two pid namespaces. >> > > >> > >> >> > >> And you can rewrite pidns_get_parent to use it. So you would instead be >> > >> doing: >> > >> >> > >> if (pidns_is_descendant(proc_pid_ns, task_active_pid_ns(current))) >> > >> return -EPERM; >> > >> >> > >> (Or you can just copy the 5-line loop into procfd_signal -- though I >> > >> imagine we'll need this for all of the procfd_* APIs.) >> > >> > Why is any of this even necessary? Why does the child namespace we're >> > considering even have a file descriptor to its ancestor's procfs? If >> >> Because you can send file descriptors between processes and container >> runtimes tend to do that. > > Right. But why *would* a container runtime send one of these procfs > FDs to a container? > >> > it has one of these FDs, it can already *read* all sorts of >> > information it really shouldn't be able to acquire, so the additional >> > ability to send a signal (subject to the usual permission checks) >> > feels like sticking a finger in a dike that's already well-perforated. >> > IMHO, we shouldn't bother with this check. The patch would be simpler >> > without it. >> >> We will definitely not allow signaling processes in an ancestor pid >> namespace! That is a security issue! I can imagine container runtimes >> killing their monitoring process etc. pp. Not happening, unless someone >> with deep expertise in signals can convince me otherwise. > > If parent namespace procfs FDs or mounts really can leak into child > namespaces as easily as Aleksa says, then I don't mind adding the > check. I was under the impression that if you find yourself in this > situation, you already have a big problem. There is one big reason to have the check, and I have not seen it mentioned yet in this thread. When SI_USER is set we report the pid of the sender of the signal in si_pid. When the signal comes from the kernel si_pid == 0. When signal is sent from an ancestor pid namespace si_pid also equals 0 (which is reasonable). A signal out to a process in a parent pid namespace such as SIGCHLD is reasonable as we can map the pid. I really don't see the point of forbidding that. From the perspective of the process in the parent pid namespace it is just another process in it's pid namespace. So it should pose no problem from the perspective of the receiving process. A signal to a process in a pid namespace that is neither a parent nor a descendent pid namespace would be a problem, as there is no well defined notion of what si_pid should be set to. So for that case perhaps we should have something like a noprocess pid that we can set. Perhaps we could set si_pid to 0x. That would take a small extension to pid_nr_ns. File descriptors are not namespaced. It is completely legitimate to use file descriptors to get around limitations of namespaces. Adding limitations to a file descriptor based api because someone else can't set up their processes in such a way as to get the restrictions they are looking for seems very sad. Frankly I think it is one of the better features of namespaces that we have to carefully handle and define these cases so that when the inevitable leaks happen you are not immediately in a world of hurt. All of the other permission checks etc continue to do their job. Plus you are prepared for the case when someone wants their containers to have an interesting communication primitive. Eric
Re: [PATCH v1 2/2] signal: add procfd_signal() syscall
Daniel Colascione writes: > On Mon, Nov 19, 2018 at 1:37 PM Christian Brauner > wrote: >> >> On Mon, Nov 19, 2018 at 01:26:22PM -0800, Daniel Colascione wrote: >> > On Mon, Nov 19, 2018 at 1:21 PM, Christian Brauner >> > wrote: >> > > That can be done without a loop by comparing the level counter for the >> > > two pid namespaces. >> > > >> > >> >> > >> And you can rewrite pidns_get_parent to use it. So you would instead be >> > >> doing: >> > >> >> > >> if (pidns_is_descendant(proc_pid_ns, task_active_pid_ns(current))) >> > >> return -EPERM; >> > >> >> > >> (Or you can just copy the 5-line loop into procfd_signal -- though I >> > >> imagine we'll need this for all of the procfd_* APIs.) >> > >> > Why is any of this even necessary? Why does the child namespace we're >> > considering even have a file descriptor to its ancestor's procfs? If >> >> Because you can send file descriptors between processes and container >> runtimes tend to do that. > > Right. But why *would* a container runtime send one of these procfs > FDs to a container? > >> > it has one of these FDs, it can already *read* all sorts of >> > information it really shouldn't be able to acquire, so the additional >> > ability to send a signal (subject to the usual permission checks) >> > feels like sticking a finger in a dike that's already well-perforated. >> > IMHO, we shouldn't bother with this check. The patch would be simpler >> > without it. >> >> We will definitely not allow signaling processes in an ancestor pid >> namespace! That is a security issue! I can imagine container runtimes >> killing their monitoring process etc. pp. Not happening, unless someone >> with deep expertise in signals can convince me otherwise. > > If parent namespace procfs FDs or mounts really can leak into child > namespaces as easily as Aleksa says, then I don't mind adding the > check. I was under the impression that if you find yourself in this > situation, you already have a big problem. There is one big reason to have the check, and I have not seen it mentioned yet in this thread. When SI_USER is set we report the pid of the sender of the signal in si_pid. When the signal comes from the kernel si_pid == 0. When signal is sent from an ancestor pid namespace si_pid also equals 0 (which is reasonable). A signal out to a process in a parent pid namespace such as SIGCHLD is reasonable as we can map the pid. I really don't see the point of forbidding that. From the perspective of the process in the parent pid namespace it is just another process in it's pid namespace. So it should pose no problem from the perspective of the receiving process. A signal to a process in a pid namespace that is neither a parent nor a descendent pid namespace would be a problem, as there is no well defined notion of what si_pid should be set to. So for that case perhaps we should have something like a noprocess pid that we can set. Perhaps we could set si_pid to 0x. That would take a small extension to pid_nr_ns. File descriptors are not namespaced. It is completely legitimate to use file descriptors to get around limitations of namespaces. Adding limitations to a file descriptor based api because someone else can't set up their processes in such a way as to get the restrictions they are looking for seems very sad. Frankly I think it is one of the better features of namespaces that we have to carefully handle and define these cases so that when the inevitable leaks happen you are not immediately in a world of hurt. All of the other permission checks etc continue to do their job. Plus you are prepared for the case when someone wants their containers to have an interesting communication primitive. Eric
Re: [PATCH 1/4] mmc: sdhci-omap: Fix DCRC error handling during tuning
Hi,
On 19/11/18 4:46 PM, Faiz Abbas wrote:
Commit 7d33c3581536 ("mmc: sdhci-omap: Workaround for Errata i802")
disabled DCRC interrupts during tuning. This write to the interrupt
enable register gets overwritten in sdhci_prepare_data() and the
interrupt is not in fact disabled. Fix this by disabling the interrupt
in the host->ier variable.
Signed-off-by: Faiz Abbas
---
drivers/mmc/host/sdhci-omap.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index 88347ce78f23..87138067e334 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -290,7 +290,6 @@ static int sdhci_omap_execute_tuning(struct mmc_host *mmc,
u32 opcode)
u32 start_window = 0, max_window = 0;
u8 cur_match, prev_match = 0;
u32 length = 0, max_len = 0;
- u32 ier = host->ier;
u32 phase_delay = 0;
int ret = 0;
u32 reg;
@@ -317,9 +316,7 @@ static int sdhci_omap_execute_tuning(struct mmc_host *mmc,
u32 opcode)
* during the tuning procedure. So disable it during the
* tuning procedure.
*/
- ier &= ~SDHCI_INT_DATA_CRC;
- sdhci_writel(host, ier, SDHCI_INT_ENABLE);
- sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
+ host->ier &= ~SDHCI_INT_DATA_CRC;
while (phase_delay <= MAX_PHASE_DELAY) {
sdhci_omap_set_dll(omap_host, phase_delay);
@@ -366,6 +363,8 @@ static int sdhci_omap_execute_tuning(struct mmc_host *mmc,
u32 opcode)
ret:
sdhci_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
+ /* Reenable forbidden interrupt */
+ host->ier |= SDHCI_INT_DATA_CRC;
It's better to have a backup of host->ier and restore the value here (in
case DATA_CRC is disabled for other cases).
Thanks
Kishon
Re: [PATCH 1/4] mmc: sdhci-omap: Fix DCRC error handling during tuning
Hi,
On 19/11/18 4:46 PM, Faiz Abbas wrote:
Commit 7d33c3581536 ("mmc: sdhci-omap: Workaround for Errata i802")
disabled DCRC interrupts during tuning. This write to the interrupt
enable register gets overwritten in sdhci_prepare_data() and the
interrupt is not in fact disabled. Fix this by disabling the interrupt
in the host->ier variable.
Signed-off-by: Faiz Abbas
---
drivers/mmc/host/sdhci-omap.c | 7 +++
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/drivers/mmc/host/sdhci-omap.c b/drivers/mmc/host/sdhci-omap.c
index 88347ce78f23..87138067e334 100644
--- a/drivers/mmc/host/sdhci-omap.c
+++ b/drivers/mmc/host/sdhci-omap.c
@@ -290,7 +290,6 @@ static int sdhci_omap_execute_tuning(struct mmc_host *mmc,
u32 opcode)
u32 start_window = 0, max_window = 0;
u8 cur_match, prev_match = 0;
u32 length = 0, max_len = 0;
- u32 ier = host->ier;
u32 phase_delay = 0;
int ret = 0;
u32 reg;
@@ -317,9 +316,7 @@ static int sdhci_omap_execute_tuning(struct mmc_host *mmc,
u32 opcode)
* during the tuning procedure. So disable it during the
* tuning procedure.
*/
- ier &= ~SDHCI_INT_DATA_CRC;
- sdhci_writel(host, ier, SDHCI_INT_ENABLE);
- sdhci_writel(host, ier, SDHCI_SIGNAL_ENABLE);
+ host->ier &= ~SDHCI_INT_DATA_CRC;
while (phase_delay <= MAX_PHASE_DELAY) {
sdhci_omap_set_dll(omap_host, phase_delay);
@@ -366,6 +363,8 @@ static int sdhci_omap_execute_tuning(struct mmc_host *mmc,
u32 opcode)
ret:
sdhci_reset(host, SDHCI_RESET_CMD | SDHCI_RESET_DATA);
+ /* Reenable forbidden interrupt */
+ host->ier |= SDHCI_INT_DATA_CRC;
It's better to have a backup of host->ier and restore the value here (in
case DATA_CRC is disabled for other cases).
Thanks
Kishon
Re: [PATCH v9 02/15] sched/cpufreq: Prepare schedutil for Energy Aware Scheduling
On 19-11-18, 14:18, Quentin Perret wrote:
> @@ -223,20 +222,33 @@ static unsigned long sugov_get_util(struct sugov_cpu
> *sg_cpu)
> - if ((util + cpu_util_dl(rq)) >= max)
> - return max;
> + if (type == FREQUENCY_UTIL) {
> + /*
> + * For frequency selection we do not make cpu_util_dl() a
> + * permanent part of this sum because we want to use
> + * cpu_bw_dl() later on, but we need to check if the
> + * CFS+RT+DL sum is saturated (ie. no idle time) such
> + * that we select f_max when there is no idle time.
> + *
> + * NOTE: numerical errors or stop class might cause us
> + * to not quite hit saturation when we should --
> + * something for later.
> + */
> +
> + if ((util + cpu_util_dl(rq)) >= max)
> + return max;
> + } else {
> + /*
> + * OTOH, for energy computation we need the estimated
> + * running time, so include util_dl and ignore dl_bw.
> + */
> + util += cpu_util_dl(rq);
> + if (util >= max)
> + return max;
> + }
Maybe write above as:
dl_util = cpu_util_dl(rq);
if ((util + dl_util) >= max)
return max;
if (type != FREQUENCY_UTIL)
util += dl_util;
as both the if/else parts were doing almost the same thing.
--
viresh
Re: [PATCH v9 02/15] sched/cpufreq: Prepare schedutil for Energy Aware Scheduling
On 19-11-18, 14:18, Quentin Perret wrote:
> @@ -223,20 +222,33 @@ static unsigned long sugov_get_util(struct sugov_cpu
> *sg_cpu)
> - if ((util + cpu_util_dl(rq)) >= max)
> - return max;
> + if (type == FREQUENCY_UTIL) {
> + /*
> + * For frequency selection we do not make cpu_util_dl() a
> + * permanent part of this sum because we want to use
> + * cpu_bw_dl() later on, but we need to check if the
> + * CFS+RT+DL sum is saturated (ie. no idle time) such
> + * that we select f_max when there is no idle time.
> + *
> + * NOTE: numerical errors or stop class might cause us
> + * to not quite hit saturation when we should --
> + * something for later.
> + */
> +
> + if ((util + cpu_util_dl(rq)) >= max)
> + return max;
> + } else {
> + /*
> + * OTOH, for energy computation we need the estimated
> + * running time, so include util_dl and ignore dl_bw.
> + */
> + util += cpu_util_dl(rq);
> + if (util >= max)
> + return max;
> + }
Maybe write above as:
dl_util = cpu_util_dl(rq);
if ((util + dl_util) >= max)
return max;
if (type != FREQUENCY_UTIL)
util += dl_util;
as both the if/else parts were doing almost the same thing.
--
viresh
Re: [RFC PATCH] dt-bindings: opp: Extend qcom-opp bindings with properties needed for CPR
On 11/9/2018 10:09 PM, Niklas Cassel wrote: On Mon, Nov 05, 2018 at 05:17:45PM -0600, Rob Herring wrote: On Mon, Oct 15, 2018 at 02:47:49PM +0200, Niklas Cassel wrote: Extend qcom-opp bindings with properties needed for Core Power Reduction (CPR). CPR is included in a great variety of Qualcomm SoC, e.g. msm8916 and msm8996, and was first introduced in msm8974. Signed-off-by: Niklas Cassel --- Hello Rob, Rajendra, Sorry for not replying sooner. Since Rob wanted the binding to be complete before merging, this is my proposal to extend the OPP binding with properties needed to support CPR (both for msm8916 and msm8996). I've discussed the proposal with Viresh, and this proposal seems better than what I previously suggested here: https://lore.kernel.org/lkml/20181005204424.ga29...@centauri.lan/ .../devicetree/bindings/opp/qcom-opp.txt | 19 +++ 1 file changed, 19 insertions(+) diff --git a/Documentation/devicetree/bindings/opp/qcom-opp.txt b/Documentation/devicetree/bindings/opp/qcom-opp.txt index db4d970c7ec7..3ab5dd84de86 100644 --- a/Documentation/devicetree/bindings/opp/qcom-opp.txt +++ b/Documentation/devicetree/bindings/opp/qcom-opp.txt @@ -23,3 +23,22 @@ Required properties: representing a corner/level that's communicated with a remote microprocessor (usually called the RPM) which then translates it into a certain voltage on a voltage rail. I've lost the context here. Please send this all together. Will do, as soon as I've gotten your feedback on this mail. Niklas, are you still waiting for feedback on this mail from Rob?
Re: [RFC PATCH] dt-bindings: opp: Extend qcom-opp bindings with properties needed for CPR
On 11/9/2018 10:09 PM, Niklas Cassel wrote: On Mon, Nov 05, 2018 at 05:17:45PM -0600, Rob Herring wrote: On Mon, Oct 15, 2018 at 02:47:49PM +0200, Niklas Cassel wrote: Extend qcom-opp bindings with properties needed for Core Power Reduction (CPR). CPR is included in a great variety of Qualcomm SoC, e.g. msm8916 and msm8996, and was first introduced in msm8974. Signed-off-by: Niklas Cassel --- Hello Rob, Rajendra, Sorry for not replying sooner. Since Rob wanted the binding to be complete before merging, this is my proposal to extend the OPP binding with properties needed to support CPR (both for msm8916 and msm8996). I've discussed the proposal with Viresh, and this proposal seems better than what I previously suggested here: https://lore.kernel.org/lkml/20181005204424.ga29...@centauri.lan/ .../devicetree/bindings/opp/qcom-opp.txt | 19 +++ 1 file changed, 19 insertions(+) diff --git a/Documentation/devicetree/bindings/opp/qcom-opp.txt b/Documentation/devicetree/bindings/opp/qcom-opp.txt index db4d970c7ec7..3ab5dd84de86 100644 --- a/Documentation/devicetree/bindings/opp/qcom-opp.txt +++ b/Documentation/devicetree/bindings/opp/qcom-opp.txt @@ -23,3 +23,22 @@ Required properties: representing a corner/level that's communicated with a remote microprocessor (usually called the RPM) which then translates it into a certain voltage on a voltage rail. I've lost the context here. Please send this all together. Will do, as soon as I've gotten your feedback on this mail. Niklas, are you still waiting for feedback on this mail from Rob?
Re: KASAN: use-after-free Read in tick_sched_handle (3)
On Mon, Nov 19, 2018 at 01:39:02PM -0800, syzbot wrote: > Hello, > > syzbot found the following crash on: > > HEAD commit:bae4e109837b mlxsw: spectrum: Expose discard counters via .. > git tree: net-next > console output: https://syzkaller.appspot.com/x/log.txt?x=11b5e77b40 > kernel config: https://syzkaller.appspot.com/x/.config?x=d86f24333880b605 > dashboard link: https://syzkaller.appspot.com/bug?extid=999bca54de2ee169c021 > compiler: gcc (GCC) 8.0.1 20180413 (experimental) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14b7d09340 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1487a22540 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+999bca54de2ee169c...@syzkaller.appspotmail.com > > IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready > IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready > 8021q: adding VLAN 0 to HW filter on device team0 > == > kasan: CONFIG_KASAN_INLINE enabled > BUG: KASAN: use-after-free in tick_sched_handle+0x16c/0x180 > kernel/time/tick-sched.c:164 So tick_sched_timer() -> tick_sched_handle() is passed regs returned by get_irq_regs() that seem to be junk. Those regs should come from smp_apic_timer_interrupt(). Thoughts?
Re: KASAN: use-after-free Read in tick_sched_handle (3)
On Mon, Nov 19, 2018 at 01:39:02PM -0800, syzbot wrote: > Hello, > > syzbot found the following crash on: > > HEAD commit:bae4e109837b mlxsw: spectrum: Expose discard counters via .. > git tree: net-next > console output: https://syzkaller.appspot.com/x/log.txt?x=11b5e77b40 > kernel config: https://syzkaller.appspot.com/x/.config?x=d86f24333880b605 > dashboard link: https://syzkaller.appspot.com/bug?extid=999bca54de2ee169c021 > compiler: gcc (GCC) 8.0.1 20180413 (experimental) > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14b7d09340 > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1487a22540 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+999bca54de2ee169c...@syzkaller.appspotmail.com > > IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready > IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready > 8021q: adding VLAN 0 to HW filter on device team0 > == > kasan: CONFIG_KASAN_INLINE enabled > BUG: KASAN: use-after-free in tick_sched_handle+0x16c/0x180 > kernel/time/tick-sched.c:164 So tick_sched_timer() -> tick_sched_handle() is passed regs returned by get_irq_regs() that seem to be junk. Those regs should come from smp_apic_timer_interrupt(). Thoughts?
RE: [PATCH V4] binder: ipc namespace support for android binder
> On Fri, Nov 16, 2018 at 11:19:41AM -0800, Todd Kjos wrote: > > On Thu, Nov 15, 2018 at 2:54 PM gre...@linuxfoundation.org > > wrote: > > ... > > > > > > A number of us have talked about this in the plumbers Android track, and > > > a different proposal for how to solve this has been made that should be > > > much more resiliant. So I will drop this patch from my queue and wait > > > for the patches based on the discussions we had there. > > > > > > I think there's some notes/slides on the discussion online somewhere, > > > but it hasn't been published as the conference is still happening, > > > otherwise I would link to it here... > > > > Here is a link to the session where you can look at the slides [1]. > > There was consensus that "binderfs" (slide 5) was the most promising > > -- but would be behind a config option to provide backwards > > compatibility for non-container use-cases. > > > > The etherpad notes are at [2] (look at "Dynamically Allocated Binder > > Devices" section) > > > > Christian Brauner will be sending out more details. > > Ok, sorry for the delay I got caught up in other work. > > The idea is to implement binderfs which I'm starting to work on. > binderfs will be a separate pseudo-filesystem that will be mountable > per-ipc namespace. > This has the advantage that the ipc namespace is attached to the > superblock of the mount and can be easily retrieved by the binder > driver. It also implies that - in contrast to the proposed patch here - > an open() on a given binder device will not be able to change the ipc > namespace of said devices. The obvious corollary is that you can > bind-mount binder devices or the whole binderfs mount between different > ipc namespaces and given the right permissions (CAP_IPC_OWNER in the > owning userns of the ipcns) can see services on the host which is > something that people wanted to be able to do. > Additionally, each binderfs mount will come with a binder-control > device. This device is functionally similar to loop-control and will > allow for dynamic allocation (and possibly deallocation) of binder > devices. With this we can remove the restriction to hard-code the number > of binder devices at compile time. > Backwards compatibility can either be guaranteed by hiding binderfs > behind a compile flag or by special-casing the inital binderfs mount to > pre-create the standard binder devices. The jury is still out on this. > > Christian > Since you are working on this, I will withdraw this patch. We will evaluate whether to your solution in our environment after you implement it.
RE: [PATCH V4] binder: ipc namespace support for android binder
> On Fri, Nov 16, 2018 at 11:19:41AM -0800, Todd Kjos wrote: > > On Thu, Nov 15, 2018 at 2:54 PM gre...@linuxfoundation.org > > wrote: > > ... > > > > > > A number of us have talked about this in the plumbers Android track, and > > > a different proposal for how to solve this has been made that should be > > > much more resiliant. So I will drop this patch from my queue and wait > > > for the patches based on the discussions we had there. > > > > > > I think there's some notes/slides on the discussion online somewhere, > > > but it hasn't been published as the conference is still happening, > > > otherwise I would link to it here... > > > > Here is a link to the session where you can look at the slides [1]. > > There was consensus that "binderfs" (slide 5) was the most promising > > -- but would be behind a config option to provide backwards > > compatibility for non-container use-cases. > > > > The etherpad notes are at [2] (look at "Dynamically Allocated Binder > > Devices" section) > > > > Christian Brauner will be sending out more details. > > Ok, sorry for the delay I got caught up in other work. > > The idea is to implement binderfs which I'm starting to work on. > binderfs will be a separate pseudo-filesystem that will be mountable > per-ipc namespace. > This has the advantage that the ipc namespace is attached to the > superblock of the mount and can be easily retrieved by the binder > driver. It also implies that - in contrast to the proposed patch here - > an open() on a given binder device will not be able to change the ipc > namespace of said devices. The obvious corollary is that you can > bind-mount binder devices or the whole binderfs mount between different > ipc namespaces and given the right permissions (CAP_IPC_OWNER in the > owning userns of the ipcns) can see services on the host which is > something that people wanted to be able to do. > Additionally, each binderfs mount will come with a binder-control > device. This device is functionally similar to loop-control and will > allow for dynamic allocation (and possibly deallocation) of binder > devices. With this we can remove the restriction to hard-code the number > of binder devices at compile time. > Backwards compatibility can either be guaranteed by hiding binderfs > behind a compile flag or by special-casing the inital binderfs mount to > pre-create the standard binder devices. The jury is still out on this. > > Christian > Since you are working on this, I will withdraw this patch. We will evaluate whether to your solution in our environment after you implement it.
Re: [PATCH v1 02/11] clk: mediatek: add new member to mtk_pll_data
On Tue, 2018-11-13 at 08:18 -0800, Nicolas Boichat wrote:
> On Mon, Nov 5, 2018 at 10:43 PM Weiyi Lu wrote:
> >
> > From: Owen Chen
> >
> > 1. pcwibits: The integer bits of pcw for plls is extend to 8 bits,
> >add a variable to indicate this change and
> >backward-compatible.
> > 2. fmin: The pll freqency lower-bound is vary from 1GMhz to
> >1.5Ghz, add a variable to indicate platform-dependent.
> >
> > Signed-off-by: Owen Chen
> > ---
> > drivers/clk/mediatek/clk-mtk.h | 2 ++
> > drivers/clk/mediatek/clk-pll.c | 10 +++---
> > 2 files changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
> > index f83c2bbb677e..1882221fe994 100644
> > --- a/drivers/clk/mediatek/clk-mtk.h
> > +++ b/drivers/clk/mediatek/clk-mtk.h
> > @@ -215,7 +215,9 @@ struct mtk_pll_data {
> > const struct clk_ops *ops;
> > u32 rst_bar_mask;
> > unsigned long fmax;
> > + unsigned long fmin;
>
> Minor nit: I'd put fmin before fmax in the structure.
>
OK, thanks for the suggestion. will fix in next version
> > int pcwbits;
> > + int pcwibits;
> > uint32_t pcw_reg;
> > int pcw_shift;
> > const struct mtk_pll_div_table *div_table;
> > diff --git a/drivers/clk/mediatek/clk-pll.c b/drivers/clk/mediatek/clk-pll.c
> > index f54e4015b0b1..0ec2c62d9383 100644
> > --- a/drivers/clk/mediatek/clk-pll.c
> > +++ b/drivers/clk/mediatek/clk-pll.c
>
> I'd add a note next to:
> #define INTEGER_BITS7
> to say that this is the default, and can be overridden with pcwibits.
>
OK, thanks for the suggestion. will add in next version
> > @@ -69,11 +69,13 @@ static unsigned long __mtk_pll_recalc_rate(struct
> > mtk_clk_pll *pll, u32 fin,
> > {
> > int pcwbits = pll->data->pcwbits;
> > int pcwfbits;
> > + int ibits;
> > u64 vco;
> > u8 c = 0;
> >
> > /* The fractional part of the PLL divider. */
> > - pcwfbits = pcwbits > INTEGER_BITS ? pcwbits - INTEGER_BITS : 0;
> > + ibits = pll->data->pcwibits ? pll->data->pcwibits : INTEGER_BITS;
> > + pcwfbits = pcwbits > ibits ? pcwbits - ibits : 0;
> >
> > vco = (u64)fin * pcw;
> >
> > @@ -138,9 +140,10 @@ static void mtk_pll_set_rate_regs(struct mtk_clk_pll
> > *pll, u32 pcw,
> > static void mtk_pll_calc_values(struct mtk_clk_pll *pll, u32 *pcw, u32
> > *postdiv,
> > u32 freq, u32 fin)
> > {
> > - unsigned long fmin = 1000 * MHZ;
> > + unsigned long fmin = pll->data->fmin ? pll->data->fmin : 1000 * MHZ;
>
> I'd put parentheses around (1000 * MHZ), to avoid the need to think
> about precedence...
>
OK, thanks for the suggestion. will add in next version
> > const struct mtk_pll_div_table *div_table = pll->data->div_table;
> > u64 _pcw;
> > + int ibits;
> > u32 val;
> >
> > if (freq > pll->data->fmax)
> > @@ -164,7 +167,8 @@ static void mtk_pll_calc_values(struct mtk_clk_pll
> > *pll, u32 *pcw, u32 *postdiv,
> > }
> >
> > /* _pcw = freq * postdiv / fin * 2^pcwfbits */
> > - _pcw = ((u64)freq << val) << (pll->data->pcwbits - INTEGER_BITS);
> > + ibits = pll->data->pcwibits ? pll->data->pcwibits : INTEGER_BITS;
> > + _pcw = ((u64)freq << val) << (pll->data->pcwbits - ibits);
> > do_div(_pcw, fin);
> >
> > *pcw = (u32)_pcw;
> > --
> > 2.18.0
> >
>
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Re: [PATCH v1 02/11] clk: mediatek: add new member to mtk_pll_data
On Tue, 2018-11-13 at 08:18 -0800, Nicolas Boichat wrote:
> On Mon, Nov 5, 2018 at 10:43 PM Weiyi Lu wrote:
> >
> > From: Owen Chen
> >
> > 1. pcwibits: The integer bits of pcw for plls is extend to 8 bits,
> >add a variable to indicate this change and
> >backward-compatible.
> > 2. fmin: The pll freqency lower-bound is vary from 1GMhz to
> >1.5Ghz, add a variable to indicate platform-dependent.
> >
> > Signed-off-by: Owen Chen
> > ---
> > drivers/clk/mediatek/clk-mtk.h | 2 ++
> > drivers/clk/mediatek/clk-pll.c | 10 +++---
> > 2 files changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/clk/mediatek/clk-mtk.h b/drivers/clk/mediatek/clk-mtk.h
> > index f83c2bbb677e..1882221fe994 100644
> > --- a/drivers/clk/mediatek/clk-mtk.h
> > +++ b/drivers/clk/mediatek/clk-mtk.h
> > @@ -215,7 +215,9 @@ struct mtk_pll_data {
> > const struct clk_ops *ops;
> > u32 rst_bar_mask;
> > unsigned long fmax;
> > + unsigned long fmin;
>
> Minor nit: I'd put fmin before fmax in the structure.
>
OK, thanks for the suggestion. will fix in next version
> > int pcwbits;
> > + int pcwibits;
> > uint32_t pcw_reg;
> > int pcw_shift;
> > const struct mtk_pll_div_table *div_table;
> > diff --git a/drivers/clk/mediatek/clk-pll.c b/drivers/clk/mediatek/clk-pll.c
> > index f54e4015b0b1..0ec2c62d9383 100644
> > --- a/drivers/clk/mediatek/clk-pll.c
> > +++ b/drivers/clk/mediatek/clk-pll.c
>
> I'd add a note next to:
> #define INTEGER_BITS7
> to say that this is the default, and can be overridden with pcwibits.
>
OK, thanks for the suggestion. will add in next version
> > @@ -69,11 +69,13 @@ static unsigned long __mtk_pll_recalc_rate(struct
> > mtk_clk_pll *pll, u32 fin,
> > {
> > int pcwbits = pll->data->pcwbits;
> > int pcwfbits;
> > + int ibits;
> > u64 vco;
> > u8 c = 0;
> >
> > /* The fractional part of the PLL divider. */
> > - pcwfbits = pcwbits > INTEGER_BITS ? pcwbits - INTEGER_BITS : 0;
> > + ibits = pll->data->pcwibits ? pll->data->pcwibits : INTEGER_BITS;
> > + pcwfbits = pcwbits > ibits ? pcwbits - ibits : 0;
> >
> > vco = (u64)fin * pcw;
> >
> > @@ -138,9 +140,10 @@ static void mtk_pll_set_rate_regs(struct mtk_clk_pll
> > *pll, u32 pcw,
> > static void mtk_pll_calc_values(struct mtk_clk_pll *pll, u32 *pcw, u32
> > *postdiv,
> > u32 freq, u32 fin)
> > {
> > - unsigned long fmin = 1000 * MHZ;
> > + unsigned long fmin = pll->data->fmin ? pll->data->fmin : 1000 * MHZ;
>
> I'd put parentheses around (1000 * MHZ), to avoid the need to think
> about precedence...
>
OK, thanks for the suggestion. will add in next version
> > const struct mtk_pll_div_table *div_table = pll->data->div_table;
> > u64 _pcw;
> > + int ibits;
> > u32 val;
> >
> > if (freq > pll->data->fmax)
> > @@ -164,7 +167,8 @@ static void mtk_pll_calc_values(struct mtk_clk_pll
> > *pll, u32 *pcw, u32 *postdiv,
> > }
> >
> > /* _pcw = freq * postdiv / fin * 2^pcwfbits */
> > - _pcw = ((u64)freq << val) << (pll->data->pcwbits - INTEGER_BITS);
> > + ibits = pll->data->pcwibits ? pll->data->pcwibits : INTEGER_BITS;
> > + _pcw = ((u64)freq << val) << (pll->data->pcwbits - ibits);
> > do_div(_pcw, fin);
> >
> > *pcw = (u32)_pcw;
> > --
> > 2.18.0
> >
>
> ___
> Linux-mediatek mailing list
> linux-media...@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-mediatek
Re: [PATCH v2 5/9] kbuild: simplify dependency generation for CONFIG_TRIM_UNUSED_KSYMS
On Tue, 20 Nov 2018, Masahiro Yamada wrote:
> My main motivation of this commit is to clean up scripts/Kbuild.include
> and scripts/Makefile.build.
>
> Currently, CONFIG_TRIM_UNUSED_KSYMS works with a tricky gimmick;
> possibly exported symbols are detected by letting $(CPP) replace
> EXPORT_SYMBOL* with a special string '=== __KSYM_*===', which is
> post-processed by sed, and passed to fixdep. The extra preprocessing
> is costly, and hacking cmd_and_fixdep is ugly.
>
> I came up with a new way to find exported symbols; insert a dummy
> symbol __ksym_marker_* to each potentially exported symbol. Those
> dummy symbols are picked up by $(NM), post-processed by sed, then
> appended to .*.cmd files. I collected the post-process part to a
> new shell script scripts/gen_ksymdeps.sh for readability. The dummy
> symbols are put into the .discard.* section so that the linker
> script rips them off the final vmlinux or modules.
>
> A nice side-effect is building with CONFIG_TRIM_UNUSED_KSYMS will
> be much faster.
>
> Signed-off-by: Masahiro Yamada
Nice work.
Reviewed-by: Nicolas Pitre
> ---
>
> Changes in v2:
> - Let gen_ksymdeps.sh add dependencies to .*.cmd file directly
> - Fix typos
>
> include/asm-generic/export.h | 13 -
> include/linux/export.h | 18 +-
> scripts/Kbuild.include | 28
> scripts/Makefile.build | 7 +++
> scripts/basic/fixdep.c | 31 ---
> scripts/gen_ksymdeps.sh | 25 +
> 6 files changed, 53 insertions(+), 69 deletions(-)
> create mode 100755 scripts/gen_ksymdeps.sh
>
> diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
> index 4d73e6e..294d6ae 100644
> --- a/include/asm-generic/export.h
> +++ b/include/asm-generic/export.h
> @@ -59,16 +59,19 @@ __kcrctab_\name:
> .endm
> #undef __put
>
> -#if defined(__KSYM_DEPS__)
> -
> -#define __EXPORT_SYMBOL(sym, val, sec) === __KSYM_##sym ===
> -
> -#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
> +#if defined(CONFIG_TRIM_UNUSED_KSYMS)
>
> #include
> #include
>
> +.macro __ksym_marker sym
> + .section ".discard.ksym","a"
> +__ksym_marker_\sym:
> + .previous
> +.endm
> +
> #define __EXPORT_SYMBOL(sym, val, sec) \
> + __ksym_marker sym; \
> __cond_export_sym(sym, val, sec, __is_defined(__KSYM_##sym))
> #define __cond_export_sym(sym, val, sec, conf) \
> ___cond_export_sym(sym, val, sec, conf)
> diff --git a/include/linux/export.h b/include/linux/export.h
> index ce764a5..fd8711e 100644
> --- a/include/linux/export.h
> +++ b/include/linux/export.h
> @@ -92,22 +92,22 @@ struct kernel_symbol {
> */
> #define __EXPORT_SYMBOL(sym, sec)
>
> -#elif defined(__KSYM_DEPS__)
> +#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
> +
> +#include
>
> /*
> * For fine grained build dependencies, we want to tell the build system
> * about each possible exported symbol even if they're not actually exported.
> - * We use a string pattern that is unlikely to be valid code that the build
> - * system filters out from the preprocessor output (see ksym_dep_filter
> - * in scripts/Kbuild.include).
> + * We use a symbol pattern __ksym_marker_ that the build system
> filters
> + * from the $(NM) output (see scripts/gen_ksymdeps.sh). These symbols are
> + * discarded in the final link stage.
> */
> -#define __EXPORT_SYMBOL(sym, sec)=== __KSYM_##sym ===
> -
> -#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
> -
> -#include
> +#define __ksym_marker(sym) \
> + static int __ksym_marker_##sym[0] __section(".discard.ksym") __used
>
> #define __EXPORT_SYMBOL(sym, sec)\
> + __ksym_marker(sym); \
> __cond_export_sym(sym, sec, __is_defined(__KSYM_##sym))
> #define __cond_export_sym(sym, sec, conf)\
> ___cond_export_sym(sym, sec, conf)
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 6cf6a8b..4b943f4 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -260,39 +260,11 @@ if_changed_dep = $(if $(strip $(any-prereq)
> $(arg-check) ), \
> @set -e; \
> $(cmd_and_fixdep), @:)
>
> -ifndef CONFIG_TRIM_UNUSED_KSYMS
> -
> cmd_and_fixdep =
> \
> $(echo-cmd) $(cmd_$(1)); \
> scripts/basic/fixdep $(depfile) $@ '$(make-cmd)' > $(dot-target).cmd;\
> rm -f $(depfile);
>
> -else
> -
> -# Filter out exported kernel symbol names from the preprocessor output.
> -# See also __KSYM_DEPS__ in include/linux/export.h.
> -# We disable the depfile generation here, so as not to overwrite the existing
> -# depfile while fixdep is parsing it.
>
Re: [PATCH v2 5/9] kbuild: simplify dependency generation for CONFIG_TRIM_UNUSED_KSYMS
On Tue, 20 Nov 2018, Masahiro Yamada wrote:
> My main motivation of this commit is to clean up scripts/Kbuild.include
> and scripts/Makefile.build.
>
> Currently, CONFIG_TRIM_UNUSED_KSYMS works with a tricky gimmick;
> possibly exported symbols are detected by letting $(CPP) replace
> EXPORT_SYMBOL* with a special string '=== __KSYM_*===', which is
> post-processed by sed, and passed to fixdep. The extra preprocessing
> is costly, and hacking cmd_and_fixdep is ugly.
>
> I came up with a new way to find exported symbols; insert a dummy
> symbol __ksym_marker_* to each potentially exported symbol. Those
> dummy symbols are picked up by $(NM), post-processed by sed, then
> appended to .*.cmd files. I collected the post-process part to a
> new shell script scripts/gen_ksymdeps.sh for readability. The dummy
> symbols are put into the .discard.* section so that the linker
> script rips them off the final vmlinux or modules.
>
> A nice side-effect is building with CONFIG_TRIM_UNUSED_KSYMS will
> be much faster.
>
> Signed-off-by: Masahiro Yamada
Nice work.
Reviewed-by: Nicolas Pitre
> ---
>
> Changes in v2:
> - Let gen_ksymdeps.sh add dependencies to .*.cmd file directly
> - Fix typos
>
> include/asm-generic/export.h | 13 -
> include/linux/export.h | 18 +-
> scripts/Kbuild.include | 28
> scripts/Makefile.build | 7 +++
> scripts/basic/fixdep.c | 31 ---
> scripts/gen_ksymdeps.sh | 25 +
> 6 files changed, 53 insertions(+), 69 deletions(-)
> create mode 100755 scripts/gen_ksymdeps.sh
>
> diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
> index 4d73e6e..294d6ae 100644
> --- a/include/asm-generic/export.h
> +++ b/include/asm-generic/export.h
> @@ -59,16 +59,19 @@ __kcrctab_\name:
> .endm
> #undef __put
>
> -#if defined(__KSYM_DEPS__)
> -
> -#define __EXPORT_SYMBOL(sym, val, sec) === __KSYM_##sym ===
> -
> -#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
> +#if defined(CONFIG_TRIM_UNUSED_KSYMS)
>
> #include
> #include
>
> +.macro __ksym_marker sym
> + .section ".discard.ksym","a"
> +__ksym_marker_\sym:
> + .previous
> +.endm
> +
> #define __EXPORT_SYMBOL(sym, val, sec) \
> + __ksym_marker sym; \
> __cond_export_sym(sym, val, sec, __is_defined(__KSYM_##sym))
> #define __cond_export_sym(sym, val, sec, conf) \
> ___cond_export_sym(sym, val, sec, conf)
> diff --git a/include/linux/export.h b/include/linux/export.h
> index ce764a5..fd8711e 100644
> --- a/include/linux/export.h
> +++ b/include/linux/export.h
> @@ -92,22 +92,22 @@ struct kernel_symbol {
> */
> #define __EXPORT_SYMBOL(sym, sec)
>
> -#elif defined(__KSYM_DEPS__)
> +#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
> +
> +#include
>
> /*
> * For fine grained build dependencies, we want to tell the build system
> * about each possible exported symbol even if they're not actually exported.
> - * We use a string pattern that is unlikely to be valid code that the build
> - * system filters out from the preprocessor output (see ksym_dep_filter
> - * in scripts/Kbuild.include).
> + * We use a symbol pattern __ksym_marker_ that the build system
> filters
> + * from the $(NM) output (see scripts/gen_ksymdeps.sh). These symbols are
> + * discarded in the final link stage.
> */
> -#define __EXPORT_SYMBOL(sym, sec)=== __KSYM_##sym ===
> -
> -#elif defined(CONFIG_TRIM_UNUSED_KSYMS)
> -
> -#include
> +#define __ksym_marker(sym) \
> + static int __ksym_marker_##sym[0] __section(".discard.ksym") __used
>
> #define __EXPORT_SYMBOL(sym, sec)\
> + __ksym_marker(sym); \
> __cond_export_sym(sym, sec, __is_defined(__KSYM_##sym))
> #define __cond_export_sym(sym, sec, conf)\
> ___cond_export_sym(sym, sec, conf)
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 6cf6a8b..4b943f4 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -260,39 +260,11 @@ if_changed_dep = $(if $(strip $(any-prereq)
> $(arg-check) ), \
> @set -e; \
> $(cmd_and_fixdep), @:)
>
> -ifndef CONFIG_TRIM_UNUSED_KSYMS
> -
> cmd_and_fixdep =
> \
> $(echo-cmd) $(cmd_$(1)); \
> scripts/basic/fixdep $(depfile) $@ '$(make-cmd)' > $(dot-target).cmd;\
> rm -f $(depfile);
>
> -else
> -
> -# Filter out exported kernel symbol names from the preprocessor output.
> -# See also __KSYM_DEPS__ in include/linux/export.h.
> -# We disable the depfile generation here, so as not to overwrite the existing
> -# depfile while fixdep is parsing it.
>
Re: [PATCH 09/16] irqchip: Add RDA8810PL interrupt driver
Hi Marc,
Thanks for the quick review!
On Mon, Nov 19, 2018 at 05:36:49PM +, Marc Zyngier wrote:
> Manivannan,
>
> On 19/11/2018 17:09, Manivannan Sadhasivam wrote:
> > Add interrupt driver for RDA Micro RDA8810PL SoC.
> >
> > Signed-off-by: Andreas Färber
> > Signed-off-by: Manivannan Sadhasivam
> > ---
> > arch/arm/mach-rda/Kconfig | 1 +
> > drivers/irqchip/Kconfig| 4 ++
> > drivers/irqchip/Makefile | 1 +
> > drivers/irqchip/irq-rda-intc.c | 116 +
> > 4 files changed, 122 insertions(+)
> > create mode 100644 drivers/irqchip/irq-rda-intc.c
> >
> > diff --git a/arch/arm/mach-rda/Kconfig b/arch/arm/mach-rda/Kconfig
> > index dafab78d7aab..29012bc68ca4 100644
> > --- a/arch/arm/mach-rda/Kconfig
> > +++ b/arch/arm/mach-rda/Kconfig
> > @@ -3,5 +3,6 @@ menuconfig ARCH_RDA
> > depends on ARCH_MULTI_V7
> > select COMMON_CLK
> > select GENERIC_IRQ_CHIP
> > + select RDA_INTC
> > help
> > This enables support for the RDA Micro 8810PL SoC family.
> > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > index 51a5ef0e96ed..9d54645870ad 100644
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -195,6 +195,10 @@ config JCORE_AIC
> > help
> > Support for the J-Core integrated AIC.
> >
> > +config RDA_INTC
> > + bool
> > + select IRQ_DOMAIN
> > +
> > config RENESAS_INTC_IRQPIN
> > bool
> > select IRQ_DOMAIN
> > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > index 794c13d3ac3d..417108027e40 100644
> > --- a/drivers/irqchip/Makefile
> > +++ b/drivers/irqchip/Makefile
> > @@ -43,6 +43,7 @@ obj-$(CONFIG_IMGPDC_IRQ) += irq-imgpdc.o
> > obj-$(CONFIG_IRQ_MIPS_CPU) += irq-mips-cpu.o
> > obj-$(CONFIG_SIRF_IRQ) += irq-sirfsoc.o
> > obj-$(CONFIG_JCORE_AIC)+= irq-jcore-aic.o
> > +obj-$(CONFIG_RDA_INTC) += irq-rda-intc.o
> > obj-$(CONFIG_RENESAS_INTC_IRQPIN) += irq-renesas-intc-irqpin.o
> > obj-$(CONFIG_RENESAS_IRQC) += irq-renesas-irqc.o
> > obj-$(CONFIG_VERSATILE_FPGA_IRQ) += irq-versatile-fpga.o
> > diff --git a/drivers/irqchip/irq-rda-intc.c b/drivers/irqchip/irq-rda-intc.c
> > new file mode 100644
> > index ..89be55a11823
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-rda-intc.c
> > @@ -0,0 +1,116 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * RDA8810PL SoC irqchip driver
> > + *
> > + * Copyright RDA Microelectronics Company Limited
> > + * Copyright (c) 2017 Andreas Färber
> > + * Copyright (c) 2018 Manivannan Sadhasivam
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +#include
> > +
> > +#define RDA_INTC_FINALSTATUS 0x00
> > +#define RDA_INTC_STATUS0x04
> > +#define RDA_INTC_MASK_SET 0x08
> > +#define RDA_INTC_MASK_CLR 0x0c
> > +#define RDA_INTC_WAKEUP_MASK 0x18
> > +#define RDA_INTC_CPU_SLEEP 0x1c
> > +
> > +#define RDA_IRQ_MASK_ALL 0x
> > +
> > +#define RDA_NR_IRQS 32
> > +
> > +void __iomem *base;
>
> Should be static?
>
Ack.
> > +
> > +static void rda_intc_mask_irq(struct irq_data *d)
> > +{
> > + void __iomem *base = (void __iomem *)irq_data_get_irq_chip_data(d);
>
> Aliases with the above. Please choose whether you want a global or a
> per-interrupt base.
>
My bad. I wanted to have a global interrupt base for rda_handle_irq.
Will remove the per interrupt base.
> > +
> > + writel(BIT(d->hwirq), base + RDA_INTC_MASK_CLR);
>
> writel_relaxed()
>
Ack.
> > +}
> > +
> > +static void rda_intc_unmask_irq(struct irq_data *d)
> > +{
> > + void __iomem *base = (void __iomem *)irq_data_get_irq_chip_data(d);
> > +
> > + writel(BIT(d->hwirq), base + RDA_INTC_MASK_SET);
>
> Same here.
Ack.
>
> > +}
> > +
> > +static int rda_intc_set_type(struct irq_data *data, unsigned int flow_type)
> > +{
> > + if (flow_type & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))
> > + irq_set_handler(data->irq, handle_edge_irq);
> > + if (flow_type & (IRQF_TRIGGER_HIGH | IRQF_TRIGGER_LOW))
> > + irq_set_handler(data->irq, handle_level_irq);
>
> So you don't need to set anything in your interrupt controller for this
> to switch between level and edge? That'd be a first...
>
Interrupt controller can only handle level triggered interrupts. Should
I just remove irq_set_type callback itself?
> > +
> > + return 0;
> > +}
> > +
> > +struct irq_domain *rda_irq_domain;
>
> static?
>
Ack.
> > +
> > +static void __exception_irq_entry rda_handle_irq(struct pt_regs *regs)
> > +{
> > + u32 stat = readl(base + RDA_INTC_FINALSTATUS);
> > + u32 hwirq;
> > +
> > + while (stat) {
> > + hwirq = __fls(stat);
> > + handle_domain_irq(rda_irq_domain, hwirq, regs);
> > + stat &= ~(1 << hwirq);
> > + }
> > +}
> > +
> > +static struct irq_chip rda_irq_chip = {
> > + .name
Re: [PATCH 09/16] irqchip: Add RDA8810PL interrupt driver
Hi Marc,
Thanks for the quick review!
On Mon, Nov 19, 2018 at 05:36:49PM +, Marc Zyngier wrote:
> Manivannan,
>
> On 19/11/2018 17:09, Manivannan Sadhasivam wrote:
> > Add interrupt driver for RDA Micro RDA8810PL SoC.
> >
> > Signed-off-by: Andreas Färber
> > Signed-off-by: Manivannan Sadhasivam
> > ---
> > arch/arm/mach-rda/Kconfig | 1 +
> > drivers/irqchip/Kconfig| 4 ++
> > drivers/irqchip/Makefile | 1 +
> > drivers/irqchip/irq-rda-intc.c | 116 +
> > 4 files changed, 122 insertions(+)
> > create mode 100644 drivers/irqchip/irq-rda-intc.c
> >
> > diff --git a/arch/arm/mach-rda/Kconfig b/arch/arm/mach-rda/Kconfig
> > index dafab78d7aab..29012bc68ca4 100644
> > --- a/arch/arm/mach-rda/Kconfig
> > +++ b/arch/arm/mach-rda/Kconfig
> > @@ -3,5 +3,6 @@ menuconfig ARCH_RDA
> > depends on ARCH_MULTI_V7
> > select COMMON_CLK
> > select GENERIC_IRQ_CHIP
> > + select RDA_INTC
> > help
> > This enables support for the RDA Micro 8810PL SoC family.
> > diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
> > index 51a5ef0e96ed..9d54645870ad 100644
> > --- a/drivers/irqchip/Kconfig
> > +++ b/drivers/irqchip/Kconfig
> > @@ -195,6 +195,10 @@ config JCORE_AIC
> > help
> > Support for the J-Core integrated AIC.
> >
> > +config RDA_INTC
> > + bool
> > + select IRQ_DOMAIN
> > +
> > config RENESAS_INTC_IRQPIN
> > bool
> > select IRQ_DOMAIN
> > diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
> > index 794c13d3ac3d..417108027e40 100644
> > --- a/drivers/irqchip/Makefile
> > +++ b/drivers/irqchip/Makefile
> > @@ -43,6 +43,7 @@ obj-$(CONFIG_IMGPDC_IRQ) += irq-imgpdc.o
> > obj-$(CONFIG_IRQ_MIPS_CPU) += irq-mips-cpu.o
> > obj-$(CONFIG_SIRF_IRQ) += irq-sirfsoc.o
> > obj-$(CONFIG_JCORE_AIC)+= irq-jcore-aic.o
> > +obj-$(CONFIG_RDA_INTC) += irq-rda-intc.o
> > obj-$(CONFIG_RENESAS_INTC_IRQPIN) += irq-renesas-intc-irqpin.o
> > obj-$(CONFIG_RENESAS_IRQC) += irq-renesas-irqc.o
> > obj-$(CONFIG_VERSATILE_FPGA_IRQ) += irq-versatile-fpga.o
> > diff --git a/drivers/irqchip/irq-rda-intc.c b/drivers/irqchip/irq-rda-intc.c
> > new file mode 100644
> > index ..89be55a11823
> > --- /dev/null
> > +++ b/drivers/irqchip/irq-rda-intc.c
> > @@ -0,0 +1,116 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * RDA8810PL SoC irqchip driver
> > + *
> > + * Copyright RDA Microelectronics Company Limited
> > + * Copyright (c) 2017 Andreas Färber
> > + * Copyright (c) 2018 Manivannan Sadhasivam
> > + */
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#include
> > +#include
> > +
> > +#define RDA_INTC_FINALSTATUS 0x00
> > +#define RDA_INTC_STATUS0x04
> > +#define RDA_INTC_MASK_SET 0x08
> > +#define RDA_INTC_MASK_CLR 0x0c
> > +#define RDA_INTC_WAKEUP_MASK 0x18
> > +#define RDA_INTC_CPU_SLEEP 0x1c
> > +
> > +#define RDA_IRQ_MASK_ALL 0x
> > +
> > +#define RDA_NR_IRQS 32
> > +
> > +void __iomem *base;
>
> Should be static?
>
Ack.
> > +
> > +static void rda_intc_mask_irq(struct irq_data *d)
> > +{
> > + void __iomem *base = (void __iomem *)irq_data_get_irq_chip_data(d);
>
> Aliases with the above. Please choose whether you want a global or a
> per-interrupt base.
>
My bad. I wanted to have a global interrupt base for rda_handle_irq.
Will remove the per interrupt base.
> > +
> > + writel(BIT(d->hwirq), base + RDA_INTC_MASK_CLR);
>
> writel_relaxed()
>
Ack.
> > +}
> > +
> > +static void rda_intc_unmask_irq(struct irq_data *d)
> > +{
> > + void __iomem *base = (void __iomem *)irq_data_get_irq_chip_data(d);
> > +
> > + writel(BIT(d->hwirq), base + RDA_INTC_MASK_SET);
>
> Same here.
Ack.
>
> > +}
> > +
> > +static int rda_intc_set_type(struct irq_data *data, unsigned int flow_type)
> > +{
> > + if (flow_type & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))
> > + irq_set_handler(data->irq, handle_edge_irq);
> > + if (flow_type & (IRQF_TRIGGER_HIGH | IRQF_TRIGGER_LOW))
> > + irq_set_handler(data->irq, handle_level_irq);
>
> So you don't need to set anything in your interrupt controller for this
> to switch between level and edge? That'd be a first...
>
Interrupt controller can only handle level triggered interrupts. Should
I just remove irq_set_type callback itself?
> > +
> > + return 0;
> > +}
> > +
> > +struct irq_domain *rda_irq_domain;
>
> static?
>
Ack.
> > +
> > +static void __exception_irq_entry rda_handle_irq(struct pt_regs *regs)
> > +{
> > + u32 stat = readl(base + RDA_INTC_FINALSTATUS);
> > + u32 hwirq;
> > +
> > + while (stat) {
> > + hwirq = __fls(stat);
> > + handle_domain_irq(rda_irq_domain, hwirq, regs);
> > + stat &= ~(1 << hwirq);
> > + }
> > +}
> > +
> > +static struct irq_chip rda_irq_chip = {
> > + .name
