* next-20180723 build: 2 failures 11 warnings (next-20180723)
From: Matt Hart @ 2018-07-24 9:34 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAH+k93GQRKO8s+0qkSH+d_2GkapMAp8C776Q4bZZnL37M5MBCw@mail.gmail.com>
On 24 July 2018 at 10:32, Matt Hart <matthew.hart@linaro.org> wrote:
>
>
>
> On 24 July 2018 at 10:16, Will Deacon <will.deacon@arm.com> wrote:
>>
>> On Mon, Jul 23, 2018 at 07:11:50PM +0100, Mark Brown wrote:
>> > On Mon, Jul 23, 2018 at 05:59:12PM +0100, Build bot for Mark Brown wrote:
>> >
>> > Today's -next fails to build an arm64 allmodconfig with:
>> >
>> > > arm64-allmodconfig
>> > > ERROR: "__sync_icache_dcache" [drivers/xen/xen-privcmd.ko] undefined!
>> >
>> > using
>> >
>> > aarch64-linux-gnu-gcc (Linaro GCC 5.3-2016.05) 5.3.1 20160412
>> >
>> > however it builds perfectly fine with
>> >
>> > aarch64-linux-gnu-gcc (Linaro GCC 6.2-2016.11) 6.2.1 20161016
>> >
>> > which honestly seems like a sensible and worthwhile upgrade at this
>> > point anyway given that it's a year and a half old so I'm going to do
>> > that for my builder (perhaps even jump on a newer version) but it seemed
>> > worth highlighting in case this is considered undesirable. A similar
>> > issue is hitting on KernelCI, we should probably look at upgrading the
>> > toolchain there too.
>>
>> Hmm, it looks to me like this comes about because xen/privcmd.c is being
>> built as a module, but contains a call to set_pte_at() with a special pte:
>>
>> pte_t pte = pte_mkspecial(pfn_pte(page_to_pfn(page), r->prot));
>>
>> set_pte_at(r->mm, addr, ptep, pte);
>>
>> which on arm64 contains:
>>
>> if (pte_present(pte) && pte_user_exec(pte) && !pte_special(pte))
>> __sync_icache_dcache(pte);
>>
>> so GCC 6 can optimise away the call to the non-exported symbol, but GCC 5
>> has trouble.
>>
>> What I don't understand is why this suddenly cropped up. Did GCC 5 build
>> linux-next arm64 allmodconfig last week?
>
>
> KernelCI uses GCC5 for ARM64 and xen-privcmd.ko has been broken in linux-next (and mainline) allmodconfig for a long time
Bloody gmail and it's HTML mean't this didn't get delivered to
linux-arm-kernel list. Resent.
>
>>
>>
>> Cheers,
>>
>> Will
>
>
^ permalink raw reply
* Re: next-20180723 build: 2 failures 11 warnings (next-20180723)
From: Matt Hart @ 2018-07-24 9:34 UTC (permalink / raw)
To: Will Deacon
Cc: linaro-kernel, Kernel Build Reports Mailman List, Catalin Marinas,
Kevin Hilman, Mark Brown, linux-next, linux-arm-kernel
In-Reply-To: <CAH+k93GQRKO8s+0qkSH+d_2GkapMAp8C776Q4bZZnL37M5MBCw@mail.gmail.com>
On 24 July 2018 at 10:32, Matt Hart <matthew.hart@linaro.org> wrote:
>
>
>
> On 24 July 2018 at 10:16, Will Deacon <will.deacon@arm.com> wrote:
>>
>> On Mon, Jul 23, 2018 at 07:11:50PM +0100, Mark Brown wrote:
>> > On Mon, Jul 23, 2018 at 05:59:12PM +0100, Build bot for Mark Brown wrote:
>> >
>> > Today's -next fails to build an arm64 allmodconfig with:
>> >
>> > > arm64-allmodconfig
>> > > ERROR: "__sync_icache_dcache" [drivers/xen/xen-privcmd.ko] undefined!
>> >
>> > using
>> >
>> > aarch64-linux-gnu-gcc (Linaro GCC 5.3-2016.05) 5.3.1 20160412
>> >
>> > however it builds perfectly fine with
>> >
>> > aarch64-linux-gnu-gcc (Linaro GCC 6.2-2016.11) 6.2.1 20161016
>> >
>> > which honestly seems like a sensible and worthwhile upgrade at this
>> > point anyway given that it's a year and a half old so I'm going to do
>> > that for my builder (perhaps even jump on a newer version) but it seemed
>> > worth highlighting in case this is considered undesirable. A similar
>> > issue is hitting on KernelCI, we should probably look at upgrading the
>> > toolchain there too.
>>
>> Hmm, it looks to me like this comes about because xen/privcmd.c is being
>> built as a module, but contains a call to set_pte_at() with a special pte:
>>
>> pte_t pte = pte_mkspecial(pfn_pte(page_to_pfn(page), r->prot));
>>
>> set_pte_at(r->mm, addr, ptep, pte);
>>
>> which on arm64 contains:
>>
>> if (pte_present(pte) && pte_user_exec(pte) && !pte_special(pte))
>> __sync_icache_dcache(pte);
>>
>> so GCC 6 can optimise away the call to the non-exported symbol, but GCC 5
>> has trouble.
>>
>> What I don't understand is why this suddenly cropped up. Did GCC 5 build
>> linux-next arm64 allmodconfig last week?
>
>
> KernelCI uses GCC5 for ARM64 and xen-privcmd.ko has been broken in linux-next (and mainline) allmodconfig for a long time
Bloody gmail and it's HTML mean't this didn't get delivered to
linux-arm-kernel list. Resent.
>
>>
>>
>> Cheers,
>>
>> Will
>
>
^ permalink raw reply
* [PATCH] fsi: master-ast-cf: use correct format string for size_t
From: Arnd Bergmann @ 2018-07-24 9:34 UTC (permalink / raw)
To: Benjamin Herrenschmidt; +Cc: Arnd Bergmann, linux-kernel
On 32-bit targets, size_t is often 'unsigned int', so printing it as %lu
causes a warning:
drivers/fsi/fsi-master-ast-cf.c: In function 'fsi_master_acf_read':
drivers/fsi/fsi-master-ast-cf.c:609:23: error: format '%u' expects argument of type 'unsigned int', but argument 6 has type 'size_t' {aka 'long unsigned int'} [-Werror=format=]
dev_dbg(master->dev, "read id %d addr %x size %ud\n", id, addr, size);
drivers/fsi/fsi-master-ast-cf.c: In function 'fsi_master_acf_write':
drivers/fsi/fsi-master-ast-cf.c:634:23: error: format '%u' expects argument of type 'unsigned int', but argument 6 has type 'size_t' {aka 'long unsigned int'} [-Werror=format=]
The correct format string is %zu.
Fixes: 6a794a27daca ("fsi: master-ast-cf: Add new FSI master using Aspeed ColdFire")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/fsi/fsi-master-ast-cf.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/fsi/fsi-master-ast-cf.c b/drivers/fsi/fsi-master-ast-cf.c
index 57afaae0b691..d1bbdf8b8343 100644
--- a/drivers/fsi/fsi-master-ast-cf.c
+++ b/drivers/fsi/fsi-master-ast-cf.c
@@ -606,7 +606,7 @@ static int fsi_master_acf_read(struct fsi_master *_master, int link,
return -ENODEV;
mutex_lock(&master->lock);
- dev_dbg(master->dev, "read id %d addr %x size %ud\n", id, addr, size);
+ dev_dbg(master->dev, "read id %d addr %x size %zu\n", id, addr, size);
build_ar_command(master, &cmd, id, addr, size, NULL);
rc = fsi_master_acf_xfer(master, id, &cmd, size, val);
last_address_update(master, id, rc == 0, addr);
@@ -631,7 +631,7 @@ static int fsi_master_acf_write(struct fsi_master *_master, int link,
mutex_lock(&master->lock);
build_ar_command(master, &cmd, id, addr, size, val);
- dev_dbg(master->dev, "write id %d addr %x size %ud raw_data: %08x\n",
+ dev_dbg(master->dev, "write id %d addr %x size %zu raw_data: %08x\n",
id, addr, size, *(uint32_t *)val);
rc = fsi_master_acf_xfer(master, id, &cmd, 0, NULL);
last_address_update(master, id, rc == 0, addr);
--
2.18.0
^ permalink raw reply related
* [PATCH] arm64: fix ACPI dependencies
From: Arnd Bergmann @ 2018-07-24 9:33 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Rafael J. Wysocki, Len Brown
Cc: AKASHI Takahiro, Ard Biesheuvel, Arnd Bergmann, Mark Rutland,
Marc Zyngier, linux-arm-kernel, linux-kernel, linux-acpi
Kconfig reports a warning on x86 builds after the ARM64 dependency
was added.
drivers/acpi/Kconfig:6:error: recursive dependency detected!
drivers/acpi/Kconfig:6: symbol ACPI depends on EFI
This rephrases the dependency to keep the ARM64 details out of the
shared Kconfig file, so Kconfig no longer gets confused by it.
Fixes: 5bcd44083a08 ("drivers: acpi: add dependency of EFI for arm64")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm64/Kconfig | 1 +
drivers/acpi/Kconfig | 5 ++++-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index cdcaa6a798b2..2f987a938405 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1267,6 +1267,7 @@ config EFI
bool "UEFI runtime support"
depends on OF && !CPU_BIG_ENDIAN
depends on KERNEL_MODE_NEON
+ select ARCH_SUPPORTS_ACPI
select LIBFDT
select UCS2_STRING
select EFI_PARAMS_FROM_FDT
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index a8da730fabc6..0cda51c5d433 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -6,7 +6,7 @@
menuconfig ACPI
bool "ACPI (Advanced Configuration and Power Interface) Support"
depends on !IA64_HP_SIM
- depends on IA64 || X86 || (ARM64 && EFI)
+ depends on IA64 || X86 || ARCH_SUPPORTS_ACPI
depends on PCI
select PNP
default y if (IA64 || X86)
@@ -41,6 +41,9 @@ menuconfig ACPI
<http://www.acpi.info>
<http://www.uefi.org/acpi/specs>
+config ARCH_SUPPORTS_ACPI
+ bool
+
if ACPI
config ACPI_LEGACY_TABLES_LOOKUP
--
2.18.0
^ permalink raw reply related
* Re: [PATCH 0/7] fs_info cleanups for volume.c
From: David Sterba @ 2018-07-24 8:28 UTC (permalink / raw)
To: dsterba, Nikolay Borisov, linux-btrfs; +Cc: lufq.fnst
In-Reply-To: <20180723132501.GL26141@twin.jikos.cz>
On Mon, Jul 23, 2018 at 03:25:01PM +0200, David Sterba wrote:
> On Fri, Jul 20, 2018 at 07:37:46PM +0300, Nikolay Borisov wrote:
> > Here are a bunch of patches which cleanup extraneous fs_info parameters to
> > function which already take a structure that holds a reference to the fs_info.
> >
> > Except for patches 4 and 5, everything else is correct - due to those functions
> > always taking a transaction. 4 and 5 in turn reference the fs_info from
> > struct btrfs_device. Inspecting the callers I managed to convince myself that
> > those function are always called with well-formed btrfs_device i.e one which
> > has its fs_info member initialised. Reviewers might want to pay extra
> > attention to that but otherwise they are trivial.
>
> 4 and 5 look good to me, a device without a valid fs_info has a short
> timespan and should not appear anywhere besides the helpers that set up
> fs_devices etc. Series added to misc-next, thanks.
btrfs/164 crashed in device rm that could be related to the patches, I
haven't analyzed that but this series looks most suspicious since the
last round of tests that did not see that crash:
[ 6712.084324] general protection fault: 0000 [#1] PREEMPT SMP
[ 6712.090096] CPU: 0 PID: 2694 Comm: btrfs Not tainted4.18.0-rc6-1.ge195904-vanilla+ #279
[ 6712.098398] Hardware name: empty empty/S3993, BIOS PAQEX0-302/24/2008
[ 6712.105072] RIP: 0010:__list_del_entry_valid+0x25/0xc0
[ 6712.129603] RSP: 0018:ffffb01281e2bbd0 EFLAGS: 00010a83
[ 6712.134969] RAX: 6b6b6b6b6b6b6b6b RBX: ffff9972756dafd8 RCX:dead000000000200
[ 6712.142246] RDX: 6b6b6b6b6b6b6b6b RSI: ffffffffc10252cf RDI:ffff9972756db100
[ 6712.149507] RBP: ffff9972756db100 R08: 0000000000000000 R09:0000000000000001
[ 6712.156788] R10: ffffb01281e2bbd8 R11: 0000000000000000 R12:ffff99728a2d7500
[ 6712.164059] R13: ffff9972756c0910 R14: ffff99728a2d7450 R15:6b6b6b6b6b6b6a43
[ 6712.171332] FS: 00007f3100bb58c0(0000) GS:ffff9972a6600000(0000)knlGS:0000000000000000
[ 6712.179615] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 6712.185507] CR2: 00007f336c989000 CR3: 00000001f6868000 CR4:00000000000006f0
[ 6712.192779] Call Trace:
[ 6712.195423] btrfs_update_commit_device_size+0x75/0xf0 [btrfs]
[ 6712.201424] btrfs_commit_transaction+0x57d/0xa90 [btrfs]
[ 6712.206999] btrfs_rm_device+0x627/0x850 [btrfs]
[ 6712.211800] btrfs_ioctl+0x2b03/0x3120 [btrfs]
[ 6712.216404] ? __might_fault+0x3e/0x90
[ 6712.220277] ? lock_acquire+0x9f/0x270
[ 6712.224156] ? __audit_syscall_entry+0xd6/0x170
[ 6712.228835] ? ktime_get_coarse_real_ts64+0x67/0x100
[ 6712.233940] ? do_vfs_ioctl+0xa5/0x6f0
[ 6712.237836] do_vfs_ioctl+0xa5/0x6f0
[ 6712.241554] ? syscall_trace_enter+0x1e8/0x3e0
[ 6712.246130] ksys_ioctl+0x70/0x80
[ 6712.249593] __x64_sys_ioctl+0x16/0x20
[ 6712.253484] do_syscall_64+0x62/0x1c0
[ 6712.257291] entry_SYSCALL_64_after_hwframe+0x49/0xbe
[ 6712.262482] RIP: 0033:0x7f30ffc52417
[ 6712.285413] RSP: 002b:00007ffd636f32c8 EFLAGS: 00000202 ORIG_RAX:0000000000000010
[ 6712.293191] RAX: ffffffffffffffda RBX: 00007ffd636f5460 RCX:00007f30ffc52417
[ 6712.300462] RDX: 00007ffd636f4300 RSI: 000000005000943a RDI:0000000000000003
[ 6712.307742] RBP: 00007ffd636f4300 R08: 0000000000000000 R09:0000000000100000
[ 6712.315005] R10: 000000000fa99fa0 R11: 0000000000000202 R12:0000000000000000
[ 6712.322286] R13: 0000000000000000 R14: 0000000000000003 R15:00007ffd636f5468
[ 6712.391596] ---[ end trace f05bf71894e4ee4d ]---
^ permalink raw reply
* [PATCH] arm64: fix ACPI dependencies
From: Arnd Bergmann @ 2018-07-24 9:33 UTC (permalink / raw)
To: linux-arm-kernel
Kconfig reports a warning on x86 builds after the ARM64 dependency
was added.
drivers/acpi/Kconfig:6:error: recursive dependency detected!
drivers/acpi/Kconfig:6: symbol ACPI depends on EFI
This rephrases the dependency to keep the ARM64 details out of the
shared Kconfig file, so Kconfig no longer gets confused by it.
Fixes: 5bcd44083a08 ("drivers: acpi: add dependency of EFI for arm64")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm64/Kconfig | 1 +
drivers/acpi/Kconfig | 5 ++++-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index cdcaa6a798b2..2f987a938405 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1267,6 +1267,7 @@ config EFI
bool "UEFI runtime support"
depends on OF && !CPU_BIG_ENDIAN
depends on KERNEL_MODE_NEON
+ select ARCH_SUPPORTS_ACPI
select LIBFDT
select UCS2_STRING
select EFI_PARAMS_FROM_FDT
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index a8da730fabc6..0cda51c5d433 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -6,7 +6,7 @@
menuconfig ACPI
bool "ACPI (Advanced Configuration and Power Interface) Support"
depends on !IA64_HP_SIM
- depends on IA64 || X86 || (ARM64 && EFI)
+ depends on IA64 || X86 || ARCH_SUPPORTS_ACPI
depends on PCI
select PNP
default y if (IA64 || X86)
@@ -41,6 +41,9 @@ menuconfig ACPI
<http://www.acpi.info>
<http://www.uefi.org/acpi/specs>
+config ARCH_SUPPORTS_ACPI
+ bool
+
if ACPI
config ACPI_LEGACY_TABLES_LOOKUP
--
2.18.0
^ permalink raw reply related
* Re: [PATCH v4] x86/mm: Add mem access rights to NPT
From: Jan Beulich @ 2018-07-24 9:33 UTC (permalink / raw)
To: aisaila, Razvan Cojocaru; +Cc: George Dunlap, Andrew Cooper, tamas, xen-devel
In-Reply-To: <31963758-9946-7e88-3d40-e801f0a37606@bitdefender.com>
>>> On 24.07.18 at 11:28, <rcojocaru@bitdefender.com> wrote:
> On 07/24/2018 11:55 AM, Jan Beulich wrote:
>>> + if ( cpu_has_svm && !p2m->mem_access_settings )
>>> + {
>>> + p2m->mem_access_settings = xmalloc(struct radix_tree_root);
>>> +
>>> + if( !p2m->mem_access_settings )
>> Style.
>>
>>> + {
>>> + xfree(d->arch.monitor.msr_bitmap);
>>> + return -ENOMEM;
>>> + }
>>> + radix_tree_init(p2m->mem_access_settings);
>>> + }
>> What's the SVM connection here? Please don't forget that p2m-pt.c
>> also serves the shadow case. Perhaps struct p2m_domain should
>> contain a boolean indicator whether this auxiliary data structure is
>> needed?
>
> Would it not work to simply check for "if ( cpu_has_svm &&
> p2m_is_hostp2m(p2m) && !p2m->mem_access_settings )" here?
>
> In the shadow case, will not p2m->p2m_class be p2m_nested?
Maybe, but that wasn't the point of my remark. I want to
get rid of the cpu_has_svm here, not have it amended.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply
* [PATCH] arm64: fix ACPI dependencies
From: Arnd Bergmann @ 2018-07-24 9:33 UTC (permalink / raw)
To: Catalin Marinas, Will Deacon, Rafael J. Wysocki, Len Brown
Cc: Mark Rutland, linux-acpi, Arnd Bergmann, Ard Biesheuvel,
Marc Zyngier, linux-kernel, AKASHI Takahiro, linux-arm-kernel
Kconfig reports a warning on x86 builds after the ARM64 dependency
was added.
drivers/acpi/Kconfig:6:error: recursive dependency detected!
drivers/acpi/Kconfig:6: symbol ACPI depends on EFI
This rephrases the dependency to keep the ARM64 details out of the
shared Kconfig file, so Kconfig no longer gets confused by it.
Fixes: 5bcd44083a08 ("drivers: acpi: add dependency of EFI for arm64")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/arm64/Kconfig | 1 +
drivers/acpi/Kconfig | 5 ++++-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index cdcaa6a798b2..2f987a938405 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1267,6 +1267,7 @@ config EFI
bool "UEFI runtime support"
depends on OF && !CPU_BIG_ENDIAN
depends on KERNEL_MODE_NEON
+ select ARCH_SUPPORTS_ACPI
select LIBFDT
select UCS2_STRING
select EFI_PARAMS_FROM_FDT
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index a8da730fabc6..0cda51c5d433 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -6,7 +6,7 @@
menuconfig ACPI
bool "ACPI (Advanced Configuration and Power Interface) Support"
depends on !IA64_HP_SIM
- depends on IA64 || X86 || (ARM64 && EFI)
+ depends on IA64 || X86 || ARCH_SUPPORTS_ACPI
depends on PCI
select PNP
default y if (IA64 || X86)
@@ -41,6 +41,9 @@ menuconfig ACPI
<http://www.acpi.info>
<http://www.uefi.org/acpi/specs>
+config ARCH_SUPPORTS_ACPI
+ bool
+
if ACPI
config ACPI_LEGACY_TABLES_LOOKUP
--
2.18.0
^ permalink raw reply related
* Re: [PATCH] mk: fix application compilation with lmnl and mlx5
From: Adrien Mazarguil @ 2018-07-24 9:32 UTC (permalink / raw)
To: Nelio Laranjeiro; +Cc: dev, Shahaf Shuler, Yongseok Koh
In-Reply-To: <14690e825609ee181e3cd522302d4788ef436f35.1532424524.git.nelio.laranjeiro@6wind.com>
On Tue, Jul 24, 2018 at 11:29:09AM +0200, Nelio Laranjeiro wrote:
> When Mellanox MLX5 PMD is compiled with
> CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS=y, the external dependency on libmln
> is missing.
>
> Fixes: 4d5cce06231a ("net/mlx5: lay groundwork for switch offloads")
> Cc: adrien.mazarguil@6wind.com
>
> Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Except for libmln => libmnl typo,
Acked-by: Adrien Mazarguil <adrien.mazarguil@6wind.com>
Thanks.
--
Adrien Mazarguil
6WIND
^ permalink raw reply
* Re: automation: Creating a patchwork instance to improve pre-commit build testing
From: Jan Beulich @ 2018-07-24 9:31 UTC (permalink / raw)
To: Andrew Cooper
Cc: Lars Kurth, Iurii Artemenko, Wei Liu, Doug Goldstein,
Minios-devel, committers, xen-devel, Matt Spencer
In-Reply-To: <8a113daf-30c4-e406-9041-d25561e61be7@citrix.com>
>>> On 24.07.18 at 11:14, <andrew.cooper3@citrix.com> wrote:
> On 24/07/18 10:06, Jan Beulich wrote:
>>>>> On 23.07.18 at 18:40, <lars.kurth@citrix.com> wrote:
>>> # How does this impact me?
>>> The contribution workflow is *not* impacted by this change, but once up and
>>> running the following will happen once you post a patch or patch series to
>>> xen-devel:
>>> * Patchwork will take patch series from the mailing list and applies it
>>> * CI/DC testing is triggered
>>> * A test report will be sent as a mail to the patch or the series (aka the
> 00 patch of the series)
>>>
>>> This does mean though that series which do not build or show other issues,
>>> will likely not be reviewed until the tests pass. This would lessen the
>>> burden on reviewers, as they will know whether the code submitted builds on
> a
>>> wide array of environments.
>> So how are dependencies between series intended to be dealt with? It
>> is not uncommon for someone to say "applies only on top of xyz". The
>> implication of "will likely not be reviewed until the tests pass" seems
>> unsuitable to me in such a case.
>
> 99% of submissions aren't "applies on top of xyz".
>
> Alternative, how about we see about not blocking underlying patches for
> unreasonable periods of time?
Well, I'm all for it, but I don't expect us to get there any time soon.
Just take the recent example of my indirect call patching series
depending on another series that I had submitted over 4 months ago.
Along those lines, the oldest (non-RFC) series I have in my to-be-
reviewed folder is from November - what if the author submitted
anything depending on it?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply
* Re: linux-next: build warning after merge of the arm64 tree
From: Ard Biesheuvel @ 2018-07-24 9:30 UTC (permalink / raw)
To: Arnd Bergmann
Cc: AKASHI Takahiro, Stephen Rothwell, Catalin Marinas, Will Deacon,
Linux-Next Mailing List, Linux Kernel Mailing List
In-Reply-To: <CAK8P3a0bOYPtsCb8_etWtFCsnjB-xkMPFiy7iBubThi0b-LtVg@mail.gmail.com>
On 24 July 2018 at 10:12, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tue, Jul 24, 2018 at 2:26 AM, AKASHI Takahiro
> <takahiro.akashi@linaro.org> wrote:
>> On Tue, Jul 24, 2018 at 09:05:45AM +1000, Stephen Rothwell wrote:
>>> Hi all,
>>>
>>> After merging the arm64 tree, today's linux-next build (x86_64
>>> allmodconfig) produced this warning:
>>
>> Where can I find this specific branch?
>>
>>> drivers/acpi/Kconfig:6:error: recursive dependency detected!
>>> drivers/acpi/Kconfig:6: symbol ACPI depends on EFI
>>
>> My patch created an additional dependency like:
>> menu ACPI
>> ...
>> depends on IA64 || X86 || (ARM64 && EFI)
>>
>>> arch/x86/Kconfig:1920: symbol EFI depends on ACPI
>>
>> So I don't understand why this change makes a warning on x86_64 build.
>> Is this a real recursive dependency?
>
> On x86, we have
>
> config EFI
> bool "EFI runtime service support"
> depends on ACPI
>
> Kconfig cannot know that CONFIG_ARM64 is never set on x86, while
> X86 is always set. So to Kconfig it it appears like a recursive
> dependency.
>
> The best workaround I can see is to have an intermediate symbol
> on ARM64 that is invisible to Kconfig on x86, like
>
> config ARCH_SUPPORTS_ACPI
> def_bool EFI
>
> and then have ACPI depend on that.
>
I suppose you mean
depends on IA64 || X86 || ARCH_SUPPORTS_ACPI
right?
Instead, perhaps we could have
config ARCH_SUPPORTS_ACPI
def_bool IA64 || X86
and make CONFIG_ACPI depend on that. Then, we can set it conditionally
in arch/arm64/Kconfig if CONFIG_EFI is enabled.
^ permalink raw reply
* [PATCH] mk: fix application compilation with lmnl and mlx5
From: Nelio Laranjeiro @ 2018-07-24 9:29 UTC (permalink / raw)
To: dev, Shahaf Shuler, Yongseok Koh; +Cc: adrien.mazarguil
When Mellanox MLX5 PMD is compiled with
CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS=y, the external dependency on libmln
is missing.
Fixes: 4d5cce06231a ("net/mlx5: lay groundwork for switch offloads")
Cc: adrien.mazarguil@6wind.com
Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
---
mk/rte.app.mk | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index f4d28c2da..ff39d37aa 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -149,7 +149,7 @@ else
_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -lrte_pmd_mlx4 -libverbs -lmlx4
endif
ifeq ($(CONFIG_RTE_LIBRTE_MLX5_DLOPEN_DEPS),y)
-_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += -lrte_pmd_mlx5 -ldl
+_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += -lrte_pmd_mlx5 -ldl -lmnl
else
_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += -lrte_pmd_mlx5 -libverbs -lmlx5 -lmnl
endif
--
2.18.0
^ permalink raw reply related
* Re: linux-next: manual merge of the arm64 tree with Linus' tree
From: Will Deacon @ 2018-07-24 9:29 UTC (permalink / raw)
To: Stephen Rothwell
Cc: Catalin Marinas, Linux-Next Mailing List,
Linux Kernel Mailing List, Olof Johansson, Paul Kocialkowski,
Laura Abbott, Greg Hackmann, Masahiro Yamada
In-Reply-To: <20180724085015.029efd54@canb.auug.org.au>
Hi Stephen,
On Tue, Jul 24, 2018 at 08:50:15AM +1000, Stephen Rothwell wrote:
> Today's linux-next merge of the arm64 tree got a conflict in:
>
> arch/arm64/Makefile
>
> between commits:
>
> 38fc42486775 ("arm64: Use aarch64elf and aarch64elfb emulation mode variants")
> 2893af07e507 ("arm64: add endianness option to LDFLAGS instead of LD")
> 96f95a17c1cf ("Revert "arm64: Use aarch64elf and aarch64elfb emulation mode variants"")
>
> from Linus' tree and commit:
>
> c931d34ea085 ("arm64: build with baremetal linker target instead of Linux when available")
>
> from the arm64 tree.
>
> I fixed it up (I just used the latter version) and can carry the fix as
> necessary. This is now fixed as far as linux-next is concerned, but any
> non trivial conflicts should be mentioned to your upstream maintainer
> when your tree is submitted for merging. You may also want to consider
> cooperating with the maintainer of the conflicting tree to minimise any
> particularly complex conflicts.
Thanks; that is the correct resolution.
Will
^ permalink raw reply
* [PATCH 01/10] drm/atomic: Add __drm_atomic_helper_plane_reset
From: Alexandru-Cosmin Gheorghe @ 2018-07-24 9:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20180724101656.0f472b0a@bbrezillon>
On Tue, Jul 24, 2018 at 10:16:56AM +0200, Boris Brezillon wrote:
> On Tue, 24 Jul 2018 09:14:02 +0100
> Alexandru-Cosmin Gheorghe <Alexandru-Cosmin.Gheorghe@arm.com> wrote:
>
> > On Tue, Jul 24, 2018 at 09:48:53AM +0200, Philipp Zabel wrote:
> > > Hi Alexandru,
> > >
> > > On Fri, 2018-07-20 at 22:15 +0100, Alexandru Gheorghe wrote:
> > > > There are a lot of drivers that subclass drm_plane_state, all of them
> > > > duplicate the code that links toghether the plane with plane_state.
> > > >
> > > > On top of that, drivers that enable core properties also have to
> > > > duplicate the code for initializing the properties to their default
> > > > values, which in all cases are the same as the defaults from core.
> > > >
> > > > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> > > > ---
> > > > drivers/gpu/drm/drm_atomic_helper.c | 32 +++++++++++++++++++++--------
> > > > include/drm/drm_atomic_helper.h | 2 ++
> > > > 2 files changed, 25 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > > > index 8008a7de2e10..e1c6f101652e 100644
> > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > > @@ -3507,6 +3507,28 @@ void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
> > > > }
> > > > EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
> > > >
> > > > +/**
> > > > + * __drm_atomic_helper_plane_reset - resets planes state to default values
> > > > + * @plane: plane object
> > > > + * @new_state: atomic plane state
> > > > + *
> > > > + * Initializes plane state to default. This is useful for drivers that subclass
> > > > + * the plane state.
> > > > + */
> > > > +void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> > > > + struct drm_plane_state *state)
> > > > +{
> > > > + if (state) {
> > > > + state->plane = plane;
> > > > + state->rotation = DRM_MODE_ROTATE_0;
> > > > + /* Reset the alpha value to fully opaque if it matters */
> > > > + if (plane->alpha_property)
> > > > + state->alpha = plane->alpha_property->values[1];
> > > > + }
> > >
> > > Is this function supposed to be callable with state == NULL ?
> > >
> > > > + plane->state = state;
> > >
> > > If so, the comment could mention that this sets plane->state to NULL if
> > > state == NULL, and a few of the call sites could be simplified.
> > >
> > > If not, I would remove the conditional if (state) {} part and also
> > > mention this in the comment.
> >
> > Yes, It's intended to be callable with null.
>
> May I ask why? I'd assume drivers that call this function to pass a
> non-NULL plane state. What's the use case for passing a NULL state here?
Drivers check it, drm_atomic_helper_plane_reset doesn't.
Looking at the existing __drm_atomic_helper_plane_* it seems they all
assume state not null, so I think it probably makes more sense to do
that for this function as well.
--
Cheers,
Alex G
^ permalink raw reply
* Re: [PATCH 01/10] drm/atomic: Add __drm_atomic_helper_plane_reset
From: Alexandru-Cosmin Gheorghe @ 2018-07-24 9:28 UTC (permalink / raw)
To: Boris Brezillon
Cc: alexandre.belloni, airlied, liviu.dudau, dri-devel,
laurent.pinchart, thellstrom, krzk, maxime.ripard, wens, kgene,
malidp, linux-graphics-maintainer, sunpeng.li, linux-samsung-soc,
nd, Tony.Cheng, linux-arm-kernel, sw0312.kim, nicolas.ferre,
shirish.s, kyungmin.park, alexander.deucher, christian.koenig
In-Reply-To: <20180724101656.0f472b0a@bbrezillon>
On Tue, Jul 24, 2018 at 10:16:56AM +0200, Boris Brezillon wrote:
> On Tue, 24 Jul 2018 09:14:02 +0100
> Alexandru-Cosmin Gheorghe <Alexandru-Cosmin.Gheorghe@arm.com> wrote:
>
> > On Tue, Jul 24, 2018 at 09:48:53AM +0200, Philipp Zabel wrote:
> > > Hi Alexandru,
> > >
> > > On Fri, 2018-07-20 at 22:15 +0100, Alexandru Gheorghe wrote:
> > > > There are a lot of drivers that subclass drm_plane_state, all of them
> > > > duplicate the code that links toghether the plane with plane_state.
> > > >
> > > > On top of that, drivers that enable core properties also have to
> > > > duplicate the code for initializing the properties to their default
> > > > values, which in all cases are the same as the defaults from core.
> > > >
> > > > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> > > > ---
> > > > drivers/gpu/drm/drm_atomic_helper.c | 32 +++++++++++++++++++++--------
> > > > include/drm/drm_atomic_helper.h | 2 ++
> > > > 2 files changed, 25 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> > > > index 8008a7de2e10..e1c6f101652e 100644
> > > > --- a/drivers/gpu/drm/drm_atomic_helper.c
> > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c
> > > > @@ -3507,6 +3507,28 @@ void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
> > > > }
> > > > EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
> > > >
> > > > +/**
> > > > + * __drm_atomic_helper_plane_reset - resets planes state to default values
> > > > + * @plane: plane object
> > > > + * @new_state: atomic plane state
> > > > + *
> > > > + * Initializes plane state to default. This is useful for drivers that subclass
> > > > + * the plane state.
> > > > + */
> > > > +void __drm_atomic_helper_plane_reset(struct drm_plane *plane,
> > > > + struct drm_plane_state *state)
> > > > +{
> > > > + if (state) {
> > > > + state->plane = plane;
> > > > + state->rotation = DRM_MODE_ROTATE_0;
> > > > + /* Reset the alpha value to fully opaque if it matters */
> > > > + if (plane->alpha_property)
> > > > + state->alpha = plane->alpha_property->values[1];
> > > > + }
> > >
> > > Is this function supposed to be callable with state == NULL ?
> > >
> > > > + plane->state = state;
> > >
> > > If so, the comment could mention that this sets plane->state to NULL if
> > > state == NULL, and a few of the call sites could be simplified.
> > >
> > > If not, I would remove the conditional if (state) {} part and also
> > > mention this in the comment.
> >
> > Yes, It's intended to be callable with null.
>
> May I ask why? I'd assume drivers that call this function to pass a
> non-NULL plane state. What's the use case for passing a NULL state here?
Drivers check it, drm_atomic_helper_plane_reset doesn't.
Looking at the existing __drm_atomic_helper_plane_* it seems they all
assume state not null, so I think it probably makes more sense to do
that for this function as well.
--
Cheers,
Alex G
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH v0] arm64: dts: rockchip: add support for ROC-RK3399-PC board
From: Heiko Stuebner @ 2018-07-24 9:28 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1532161826-2537-1-git-send-email-djw@t-chip.com.cn>
Hi Levin,
Am Samstag, 21. Juli 2018, 10:30:26 CEST schrieb djw at t-chip.com.cn:
> From: Levin Du <djw@t-chip.com.cn>
>
> ROC-RK3399-PC is the first power efficient 4GB DDR4 single board
maybe "is a power efficient" instead of "the first" ;-)
[...]
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dts b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dts
> new file mode 100644
> index 0000000..207f2e3
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dts
> @@ -0,0 +1,717 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2017 T-Chip Intelligent Technology Co., Ltd
> + */
> +
> +/dts-v1/;
> +#include <dt-bindings/pwm/pwm.h>
> +#include "rk3399.dtsi"
> +#include "rk3399-opp.dtsi"
> +
> +/ {
> + model = "Firefly ROC-RK3399-PC Board";
> + compatible = "firefly,roc-rk3399-pc", "rockchip,rk3399";
> +
> + chosen {
> + bootargs = "earlycon=uart8250,mmio32,0xff1a0000 swiotlb=1";
I don't think we want to hard-code linux bootargs in the generic devicetree
> + stdout-path = "serial2:1500000n8";
> + };
> +
> + backlight: backlight {
> + compatible = "pwm-backlight";
> + enable-gpios = <&gpio1 RK_PB5 GPIO_ACTIVE_HIGH>;
> + pwms = <&pwm0 0 25000 0>;
> + brightness-levels = <
> + 0 1 2 3 4 5 6 7
As Rob noted in the px30 evb patch, there is now a property helping
to drop these long lists of brightness levels, see
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=1e5e7cc794b5a332c23216dade0a2e937d694b7f
> + 8 9 10 11 12 13 14 15
> + 16 17 18 19 20 21 22 23
> + 24 25 26 27 28 29 30 31
> + 32 33 34 35 36 37 38 39
> + 40 41 42 43 44 45 46 47
> + 48 49 50 51 52 53 54 55
> + 56 57 58 59 60 61 62 63
> + 64 65 66 67 68 69 70 71
> + 72 73 74 75 76 77 78 79
> + 80 81 82 83 84 85 86 87
> + 88 89 90 91 92 93 94 95
> + 96 97 98 99 100 101 102 103
> + 104 105 106 107 108 109 110 111
> + 112 113 114 115 116 117 118 119
> + 120 121 122 123 124 125 126 127
> + 128 129 130 131 132 133 134 135
> + 136 137 138 139 140 141 142 143
> + 144 145 146 147 148 149 150 151
> + 152 153 154 155 156 157 158 159
> + 160 161 162 163 164 165 166 167
> + 168 169 170 171 172 173 174 175
> + 176 177 178 179 180 181 182 183
> + 184 185 186 187 188 189 190 191
> + 192 193 194 195 196 197 198 199
> + 200 201 202 203 204 205 206 207
> + 208 209 210 211 212 213 214 215
> + 216 217 218 219 220 221 222 223
> + 224 225 226 227 228 229 230 231
> + 232 233 234 235 236 237 238 239
> + 240 241 242 243 244 245 246 247
> + 248 249 250 251 252 253 254 255>;
> + default-brightness-level = <200>;
> + };
[...]
> + vcc_vbus_typec0: vcc-vbus-typec0 {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc_vbus_typec0";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + };
> +
> + vcc12v_sys: mp8859-dcdc1 {
The mp8859 seems to be an i2c-device, as also shown by the
nearly empty mp8859 entry below, so shouldn't this regulator
be defined there?
> + compatible = "regulator-fixed";
> + regulator-name = "vcc12v_sys";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <12000000>;
> + regulator-max-microvolt = <12000000>;
> + vin-supply = <&vcc_vbus_typec0>;
> + };
[...]
> + vcc_hub_en: vcc_hub_en-regulator {
> + compatible = "regulator-fixed";
> + enable-active-high;
> + gpio = <&gpio2 RK_PA4 GPIO_ACTIVE_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&hub_rst>;
> + regulator-name = "vcc_hub_en";
> + regulator-always-on;
missing vin-supply
> + };
> +
[...]
> + vdd_cpu_b: regulator at 40 {
> + compatible = "silergy,syr827";
> + reg = <0x40>;
> + fcs,suspend-voltage-selector = <1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&vsel1_gpio>;
> + regulator-name = "vdd_cpu_b";
> + regulator-min-microvolt = <712500>;
> + regulator-max-microvolt = <1500000>;
> + regulator-ramp-delay = <1000>;
> + regulator-always-on;
> + regulator-boot-on;
> + vsel-gpios = <&gpio1 18 GPIO_ACTIVE_HIGH>;
vsel-gpios is not defined in the mainline dt-binding of the fan5355/syr82*
> + vin-supply = <&vcc3v3_sys>;
> +
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> + };
> +
> + vdd_gpu: regulator at 41 {
> + compatible = "silergy,syr828";
> + reg = <0x41>;
> + fcs,suspend-voltage-selector = <1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&vsel2_gpio>;
> + regulator-name = "vdd_gpu";
> + regulator-min-microvolt = <712500>;
> + regulator-max-microvolt = <1500000>;
> + regulator-ramp-delay = <1000>;
> + regulator-always-on;
> + regulator-boot-on;
> + vsel-gpios = <&gpio1 14 GPIO_ACTIVE_HIGH>;
same as above
> + vin-supply = <&vcc3v3_sys>;
> +
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> + };
> +};
> +
> +&i2c1 {
> + i2c-scl-rising-time-ns = <300>;
> + i2c-scl-falling-time-ns = <15>;
> + status = "okay";
> +};
> +
> +&i2c3 {
> + i2c-scl-rising-time-ns = <450>;
> + i2c-scl-falling-time-ns = <15>;
> + status = "okay";
> +};
> +
> +&i2c4 {
> + i2c-scl-rising-time-ns = <600>;
> + i2c-scl-falling-time-ns = <20>;
> + status = "okay";
> +
> + mp8859: mp8859 at 66 {
> + compatible = "mps,mp8859";
missing a dt-binding and also a real regulator implementation?
> + reg = <0x66>;
> + };
> +
> + fusb1: usb-typec at 22 {
> + compatible = "fcs,fusb302";
> + reg = <0x22>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&fusb1_int>;
> + fcs,int-n = <&gpio1 RK_PA1 GPIO_ACTIVE_HIGH>;
mainline binding expects an "interrupts" property not the
fcs,int-n from above
> + status = "okay";
> + };
> +
> +};
> +
> +&i2c7 {
> + i2c-scl-rising-time-ns = <600>;
> + i2c-scl-falling-time-ns = <20>;
> + status = "okay";
> +
> + fusb0: usb-typec at 22 {
> + compatible = "fcs,fusb302";
> + reg = <0x22>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&fusb0_int>;
> + fcs,int-n = <&gpio1 RK_PA2 GPIO_ACTIVE_HIGH>;
same as above
> + status = "okay";
> + };
> +};
> +
> +&i2s0 {
> + rockchip,playback-channels = <8>;
> + rockchip,capture-channels = <8>;
> + #sound-dai-cells = <0>;
sound-dai-cells are in rk3399.dtsi now ... see
https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git/commit/?id=4486baca66e7a96a6ded3957cb522872a49870ce
> + status = "okay";
> +};
> +
> +&i2s1 {
> + rockchip,playback-channels = <2>;
> + rockchip,capture-channels = <2>;
> + #sound-dai-cells = <0>;
same as above
> + status = "okay";
> +};
> +
> +&i2s2 {
> + #sound-dai-cells = <0>;
same as above
> + status = "okay";
> +};
> +
> +&tcphy0 {
> + extcon = <&fusb0>;
the extcon is not described in the mainline fusb302 binding.
> + status = "okay";
> +};
> +
> +&tcphy1 {
> + extcon = <&fusb1>;
same as above
> + status = "okay";
> +};
> +
> +&tsadc {
> + /* tshut mode 0:CRU 1:GPIO */
> + rockchip,hw-tshut-mode = <1>;
> + /* tshut polarity 0:LOW 1:HIGH */
> + rockchip,hw-tshut-polarity = <1>;
> + status = "okay";
> +};
> +
> +&u2phy0 {
> + status = "okay";
> +
> + u2phy0_otg: otg-port {
> + phy-supply = <&vcc5v0_host>;
> + //status = "disabled";
don't leave commented stuff in the dt please.
> + status = "okay";
> + };
> +
> + u2phy0_host: host-port {
> + phy-supply = <&vcc5v0_host>;
> + status = "okay";
> + };
> +};
> +
> +&u2phy1 {
> + status = "okay";
> +
> + u2phy1_otg: otg-port {
> + phy-supply = <&vcc5v0_host>;
> + //status = "disabled";
same as above
> + status = "okay";
> + };
> +
> + u2phy1_host: host-port {
> + phy-supply = <&vcc5v0_host>;
> + status = "okay";
> + };
> +};
Thanks
Heiko
^ permalink raw reply
* Re: [PATCH v0] arm64: dts: rockchip: add support for ROC-RK3399-PC board
From: Heiko Stuebner @ 2018-07-24 9:28 UTC (permalink / raw)
To: djw
Cc: linux-rockchip, Wayne Chou, devicetree, Jianqun Xu, Jacob Chen,
Brian Norris, Klaus Goger, linux-kernel, Heinrich Schuchardt,
Shawn Lin, Ezequiel Garcia, Jagan Teki, Masahiro Yamada,
Will Deacon, Mark Rutland, Rob Herring, Catalin Marinas,
linux-arm-kernel, Enric Balletbo i Serra
In-Reply-To: <1532161826-2537-1-git-send-email-djw@t-chip.com.cn>
Hi Levin,
Am Samstag, 21. Juli 2018, 10:30:26 CEST schrieb djw@t-chip.com.cn:
> From: Levin Du <djw@t-chip.com.cn>
>
> ROC-RK3399-PC is the first power efficient 4GB DDR4 single board
maybe "is a power efficient" instead of "the first" ;-)
[...]
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dts b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dts
> new file mode 100644
> index 0000000..207f2e3
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-roc-pc.dts
> @@ -0,0 +1,717 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2017 T-Chip Intelligent Technology Co., Ltd
> + */
> +
> +/dts-v1/;
> +#include <dt-bindings/pwm/pwm.h>
> +#include "rk3399.dtsi"
> +#include "rk3399-opp.dtsi"
> +
> +/ {
> + model = "Firefly ROC-RK3399-PC Board";
> + compatible = "firefly,roc-rk3399-pc", "rockchip,rk3399";
> +
> + chosen {
> + bootargs = "earlycon=uart8250,mmio32,0xff1a0000 swiotlb=1";
I don't think we want to hard-code linux bootargs in the generic devicetree
> + stdout-path = "serial2:1500000n8";
> + };
> +
> + backlight: backlight {
> + compatible = "pwm-backlight";
> + enable-gpios = <&gpio1 RK_PB5 GPIO_ACTIVE_HIGH>;
> + pwms = <&pwm0 0 25000 0>;
> + brightness-levels = <
> + 0 1 2 3 4 5 6 7
As Rob noted in the px30 evb patch, there is now a property helping
to drop these long lists of brightness levels, see
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=1e5e7cc794b5a332c23216dade0a2e937d694b7f
> + 8 9 10 11 12 13 14 15
> + 16 17 18 19 20 21 22 23
> + 24 25 26 27 28 29 30 31
> + 32 33 34 35 36 37 38 39
> + 40 41 42 43 44 45 46 47
> + 48 49 50 51 52 53 54 55
> + 56 57 58 59 60 61 62 63
> + 64 65 66 67 68 69 70 71
> + 72 73 74 75 76 77 78 79
> + 80 81 82 83 84 85 86 87
> + 88 89 90 91 92 93 94 95
> + 96 97 98 99 100 101 102 103
> + 104 105 106 107 108 109 110 111
> + 112 113 114 115 116 117 118 119
> + 120 121 122 123 124 125 126 127
> + 128 129 130 131 132 133 134 135
> + 136 137 138 139 140 141 142 143
> + 144 145 146 147 148 149 150 151
> + 152 153 154 155 156 157 158 159
> + 160 161 162 163 164 165 166 167
> + 168 169 170 171 172 173 174 175
> + 176 177 178 179 180 181 182 183
> + 184 185 186 187 188 189 190 191
> + 192 193 194 195 196 197 198 199
> + 200 201 202 203 204 205 206 207
> + 208 209 210 211 212 213 214 215
> + 216 217 218 219 220 221 222 223
> + 224 225 226 227 228 229 230 231
> + 232 233 234 235 236 237 238 239
> + 240 241 242 243 244 245 246 247
> + 248 249 250 251 252 253 254 255>;
> + default-brightness-level = <200>;
> + };
[...]
> + vcc_vbus_typec0: vcc-vbus-typec0 {
> + compatible = "regulator-fixed";
> + regulator-name = "vcc_vbus_typec0";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <5000000>;
> + regulator-max-microvolt = <5000000>;
> + };
> +
> + vcc12v_sys: mp8859-dcdc1 {
The mp8859 seems to be an i2c-device, as also shown by the
nearly empty mp8859 entry below, so shouldn't this regulator
be defined there?
> + compatible = "regulator-fixed";
> + regulator-name = "vcc12v_sys";
> + regulator-always-on;
> + regulator-boot-on;
> + regulator-min-microvolt = <12000000>;
> + regulator-max-microvolt = <12000000>;
> + vin-supply = <&vcc_vbus_typec0>;
> + };
[...]
> + vcc_hub_en: vcc_hub_en-regulator {
> + compatible = "regulator-fixed";
> + enable-active-high;
> + gpio = <&gpio2 RK_PA4 GPIO_ACTIVE_HIGH>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&hub_rst>;
> + regulator-name = "vcc_hub_en";
> + regulator-always-on;
missing vin-supply
> + };
> +
[...]
> + vdd_cpu_b: regulator@40 {
> + compatible = "silergy,syr827";
> + reg = <0x40>;
> + fcs,suspend-voltage-selector = <1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&vsel1_gpio>;
> + regulator-name = "vdd_cpu_b";
> + regulator-min-microvolt = <712500>;
> + regulator-max-microvolt = <1500000>;
> + regulator-ramp-delay = <1000>;
> + regulator-always-on;
> + regulator-boot-on;
> + vsel-gpios = <&gpio1 18 GPIO_ACTIVE_HIGH>;
vsel-gpios is not defined in the mainline dt-binding of the fan5355/syr82*
> + vin-supply = <&vcc3v3_sys>;
> +
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> + };
> +
> + vdd_gpu: regulator@41 {
> + compatible = "silergy,syr828";
> + reg = <0x41>;
> + fcs,suspend-voltage-selector = <1>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&vsel2_gpio>;
> + regulator-name = "vdd_gpu";
> + regulator-min-microvolt = <712500>;
> + regulator-max-microvolt = <1500000>;
> + regulator-ramp-delay = <1000>;
> + regulator-always-on;
> + regulator-boot-on;
> + vsel-gpios = <&gpio1 14 GPIO_ACTIVE_HIGH>;
same as above
> + vin-supply = <&vcc3v3_sys>;
> +
> + regulator-state-mem {
> + regulator-off-in-suspend;
> + };
> + };
> +};
> +
> +&i2c1 {
> + i2c-scl-rising-time-ns = <300>;
> + i2c-scl-falling-time-ns = <15>;
> + status = "okay";
> +};
> +
> +&i2c3 {
> + i2c-scl-rising-time-ns = <450>;
> + i2c-scl-falling-time-ns = <15>;
> + status = "okay";
> +};
> +
> +&i2c4 {
> + i2c-scl-rising-time-ns = <600>;
> + i2c-scl-falling-time-ns = <20>;
> + status = "okay";
> +
> + mp8859: mp8859@66 {
> + compatible = "mps,mp8859";
missing a dt-binding and also a real regulator implementation?
> + reg = <0x66>;
> + };
> +
> + fusb1: usb-typec@22 {
> + compatible = "fcs,fusb302";
> + reg = <0x22>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&fusb1_int>;
> + fcs,int-n = <&gpio1 RK_PA1 GPIO_ACTIVE_HIGH>;
mainline binding expects an "interrupts" property not the
fcs,int-n from above
> + status = "okay";
> + };
> +
> +};
> +
> +&i2c7 {
> + i2c-scl-rising-time-ns = <600>;
> + i2c-scl-falling-time-ns = <20>;
> + status = "okay";
> +
> + fusb0: usb-typec@22 {
> + compatible = "fcs,fusb302";
> + reg = <0x22>;
> + pinctrl-names = "default";
> + pinctrl-0 = <&fusb0_int>;
> + fcs,int-n = <&gpio1 RK_PA2 GPIO_ACTIVE_HIGH>;
same as above
> + status = "okay";
> + };
> +};
> +
> +&i2s0 {
> + rockchip,playback-channels = <8>;
> + rockchip,capture-channels = <8>;
> + #sound-dai-cells = <0>;
sound-dai-cells are in rk3399.dtsi now ... see
https://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc.git/commit/?id=4486baca66e7a96a6ded3957cb522872a49870ce
> + status = "okay";
> +};
> +
> +&i2s1 {
> + rockchip,playback-channels = <2>;
> + rockchip,capture-channels = <2>;
> + #sound-dai-cells = <0>;
same as above
> + status = "okay";
> +};
> +
> +&i2s2 {
> + #sound-dai-cells = <0>;
same as above
> + status = "okay";
> +};
> +
> +&tcphy0 {
> + extcon = <&fusb0>;
the extcon is not described in the mainline fusb302 binding.
> + status = "okay";
> +};
> +
> +&tcphy1 {
> + extcon = <&fusb1>;
same as above
> + status = "okay";
> +};
> +
> +&tsadc {
> + /* tshut mode 0:CRU 1:GPIO */
> + rockchip,hw-tshut-mode = <1>;
> + /* tshut polarity 0:LOW 1:HIGH */
> + rockchip,hw-tshut-polarity = <1>;
> + status = "okay";
> +};
> +
> +&u2phy0 {
> + status = "okay";
> +
> + u2phy0_otg: otg-port {
> + phy-supply = <&vcc5v0_host>;
> + //status = "disabled";
don't leave commented stuff in the dt please.
> + status = "okay";
> + };
> +
> + u2phy0_host: host-port {
> + phy-supply = <&vcc5v0_host>;
> + status = "okay";
> + };
> +};
> +
> +&u2phy1 {
> + status = "okay";
> +
> + u2phy1_otg: otg-port {
> + phy-supply = <&vcc5v0_host>;
> + //status = "disabled";
same as above
> + status = "okay";
> + };
> +
> + u2phy1_host: host-port {
> + phy-supply = <&vcc5v0_host>;
> + status = "okay";
> + };
> +};
Thanks
Heiko
^ permalink raw reply
* [PATCH v2 4/4] banned.h: mark strncpy() as banned
From: Jeff King @ 2018-07-24 9:28 UTC (permalink / raw)
To: git; +Cc: Stefan Beller, Eric Sunshine, Junio C Hamano
In-Reply-To: <20180724092329.GA24250@sigill.intra.peff.net>
The strncpy() function is less horrible than strcpy(), but
is still pretty easy to misuse because of its funny
termination semantics. Namely, that if it truncates it omits
the NUL terminator, and you must remember to add it
yourself. Even if you use it correctly, it's sometimes hard
for a reader to verify this without hunting through the
code. If you're thinking about using it, consider instead:
- strlcpy() if you really just need a truncated but
NUL-terminated string (we provide a compat version, so
it's always available)
- xsnprintf() if you're sure that what you're copying
should fit
- strbuf or xstrfmt() if you need to handle
arbitrary-length heap-allocated strings
Note that there is one instance of strncpy in
compat/regex/regcomp.c, which is fine (it allocates a
sufficiently large string before copying). But this doesn't
trigger the ban-list even when compiling with NO_REGEX=1,
because:
1. we don't use git-compat-util.h when compiling it
(instead we rely on the system includes from the
upstream library); and
2. It's in an "#ifdef DEBUG" block
Since it's doesn't trigger the banned.h code, we're better
off leaving it to keep our divergence from upstream minimal.
Signed-off-by: Jeff King <peff@peff.net>
---
banned.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/banned.h b/banned.h
index c83d6cb9df..ab96954583 100644
--- a/banned.h
+++ b/banned.h
@@ -14,6 +14,8 @@
#define strcpy(x,y) BANNED(strcpy)
#undef strcat
#define strcat(x,y) BANNED(strcat)
+#undef strncpy
+#define strncpy(x,y,n) BANNED(strncpy)
#undef sprintf
#undef vsprintf
--
2.18.0.542.g2bf2fc4f7e
^ permalink raw reply related
* Re: [PATCH v4] x86/mm: Add mem access rights to NPT
From: Razvan Cojocaru @ 2018-07-24 9:28 UTC (permalink / raw)
To: Jan Beulich, aisaila; +Cc: George Dunlap, Andrew Cooper, tamas, xen-devel
In-Reply-To: <5B56E99D02000078001D706B@prv1-mh.provo.novell.com>
On 07/24/2018 11:55 AM, Jan Beulich wrote:
>> + if ( cpu_has_svm && !p2m->mem_access_settings )
>> + {
>> + p2m->mem_access_settings = xmalloc(struct radix_tree_root);
>> +
>> + if( !p2m->mem_access_settings )
> Style.
>
>> + {
>> + xfree(d->arch.monitor.msr_bitmap);
>> + return -ENOMEM;
>> + }
>> + radix_tree_init(p2m->mem_access_settings);
>> + }
> What's the SVM connection here? Please don't forget that p2m-pt.c
> also serves the shadow case. Perhaps struct p2m_domain should
> contain a boolean indicator whether this auxiliary data structure is
> needed?
Would it not work to simply check for "if ( cpu_has_svm &&
p2m_is_hostp2m(p2m) && !p2m->mem_access_settings )" here?
In the shadow case, will not p2m->p2m_class be p2m_nested?
Thanks,
Razvan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply
* [Bug 105684] Loading amdgpu hits general protection fault: 0000 [#1] SMP NOPTI
From: bugzilla-daemon @ 2018-07-24 9:27 UTC (permalink / raw)
To: dri-devel
In-Reply-To: <bug-105684-502@http.bugs.freedesktop.org/>
[-- Attachment #1.1: Type: text/plain, Size: 458 bytes --]
https://bugs.freedesktop.org/show_bug.cgi?id=105684
--- Comment #36 from jian-hong@endlessm.com ---
Created attachment 140806
--> https://bugs.freedesktop.org/attachment.cgi?id=140806&action=edit
dmesg of 4.18.0-rc6
Tested with Linux 4.18.0-rc6 kernel.
It seems the amdgpu module could be loaded correctly. I tried over 20 times.
The attachment is the full dmesg.
--
You are receiving this mail because:
You are the assignee for the bug.
[-- Attachment #1.2: Type: text/html, Size: 1390 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply
* [PATCH v2] f2fs: clear victim_secmap when section has full valid blocks
From: Yunlong Song @ 2018-07-24 9:27 UTC (permalink / raw)
To: jaegeuk, chao, yuchao0, yunlong.song, yunlong.song
Cc: miaoxie, bintian.wang, shengyong1, heyunlei, linux-f2fs-devel,
linux-kernel
In-Reply-To: <1532355022-163029-1-git-send-email-yunlong.song@huawei.com>
Without this patch, f2fs only clears victim_secmap when it finds out
that the section has no valid blocks at all, but forgets to clear the
victim_secmap when the whole section has full valid blocks.
Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
---
fs/f2fs/segment.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index cfff7cf..0a79554 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -776,7 +776,8 @@ static void __remove_dirty_segment(struct f2fs_sb_info *sbi, unsigned int segno,
if (test_and_clear_bit(segno, dirty_i->dirty_segmap[t]))
dirty_i->nr_dirty[t]--;
- if (get_valid_blocks(sbi, segno, true) == 0)
+ if (get_valid_blocks(sbi, segno, true) == 0 ||
+ get_valid_blocks(sbi, segno, true) == BLKS_PER_SEC(sbi))
clear_bit(GET_SEC_FROM_SEG(sbi, segno),
dirty_i->victim_secmap);
}
--
1.8.5.2
^ permalink raw reply related
* RE: [net-next v5 3/3] net/tls: Remove redundant array allocation.
From: Vakul Garg @ 2018-07-24 8:22 UTC (permalink / raw)
To: Dave Watson, David Miller
Cc: netdev@vger.kernel.org, borisp@mellanox.com, aviadye@mellanox.com,
Doron Roberts-Kedes
In-Reply-To: <20180723163509.GA91424@davejwatson-mba.local>
> -----Original Message-----
> From: Dave Watson [mailto:davejwatson@fb.com]
> Sent: Monday, July 23, 2018 10:05 PM
> To: David Miller <davem@davemloft.net>
> Cc: Vakul Garg <vakul.garg@nxp.com>; netdev@vger.kernel.org;
> borisp@mellanox.com; aviadye@mellanox.com; Doron Roberts-Kedes
> <doronrk@fb.com>
> Subject: Re: [net-next v5 3/3] net/tls: Remove redundant array allocation.
>
> On 07/21/18 07:25 PM, David Miller wrote:
> > From: Vakul Garg <vakul.garg@nxp.com>
> > Date: Thu, 19 Jul 2018 21:56:13 +0530
> >
> > > In function decrypt_skb(), array allocation in case when sgout is
> > > NULL is unnecessary. Instead, local variable sgin_arr[] can be used.
> > >
> > > Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
> >
> > Hmmm...
> >
> > Dave, can you take a look at this? Do you think there might have been
> > a reason you felt that you needed to dynamically allocate the
> > scatterlists when you COW and skb and do in-place decryption?
> >
> > I guess this change is ok, nsg can only get smaller when the SKB is
> > COW'd.
> >
>
> > > memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
> > > if (!sgout) {
> > > nsg = skb_cow_data(skb, 0, &unused) + 1;
> > > - sgin = kmalloc_array(nsg, sizeof(*sgin), sk->sk_allocation);
> > > sgout = sgin;
> > > }
>
> I don't think this patch is safe as-is. sgin_arr is a stack array of size
> MAX_SKB_FRAGS (+ overhead), while my read of skb_cow_data is that it
> walks the whole chain of skbs from skb->next, and can return any number of
> segments. Therefore we need to heap allocate. I think I copied the IPSEC
> code here.
>
> For perf though, we could use the stack array if skb_cow_data returns <=
> MAX_SKB_FRAGS.
>
> This code is slightly confusing though, since we don't heap allocate in the
> zerocopy case - what happens is that skb_to_sgvec returns -EMSGSIZE, and
> we fall back to the non-zerocopy case, and return again to this function,
> where we then hit the skb_cow_data path and heap allocate.
Thanks for explaining.
I am missing the point why MAX_SKB_FRAGS sized local array
sgin has been used in tls_sw_recvmsg(). What is special about MAX_SKB_FRAGS so
that we used it as a size factor for 'sgin'?
Will it be a bad idea to get rid of array 'sgin' on stack and simply kmalloc 'sgin' for
whatever the number the number of pages returned by iov_iter_npages()?
We can allocate for sgout too with the same kmalloc().
(Using a local array based 'sgin' is coming in the way to achieve sending multiple async
record decryption requests to the accelerator without waiting for previous one to complete.)
^ permalink raw reply
* [PATCH v2 3/4] banned.h: mark sprintf() as banned
From: Jeff King @ 2018-07-24 9:27 UTC (permalink / raw)
To: git; +Cc: Stefan Beller, Eric Sunshine, Junio C Hamano
In-Reply-To: <20180724092329.GA24250@sigill.intra.peff.net>
The sprintf() function (and its variadic form vsprintf) make
it easy to accidentally introduce a buffer overflow. If
you're thinking of using them, you're better off either
using a dynamic string (strbuf or xstrfmt), or xsnprintf if
you really know that you won't overflow. The last sprintf()
call went away quite a while ago in f0766bf94e (fsck: use
for_each_loose_file_in_objdir, 2015-09-24).
Note that we respect HAVE_VARIADIC_MACROS here, which some
ancient platforms lack. As a fallback, we can just "guess"
that the caller will provide 3 arguments. If they do, then
the macro will work as usual. If not, then they'll get a
slightly less useful error, like:
git.c:718:24: error: macro "sprintf" passed 3 arguments, but takes just 2
That's not ideal, but it at least alerts them to the problem
area. And anyway, we're primarily targeting people adding
new code. Most developers should be on modern enough
platforms to see the normal "good" error message.
Signed-off-by: Jeff King <peff@peff.net>
---
banned.h | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/banned.h b/banned.h
index 32e0bae01d..c83d6cb9df 100644
--- a/banned.h
+++ b/banned.h
@@ -15,4 +15,14 @@
#undef strcat
#define strcat(x,y) BANNED(strcat)
+#undef sprintf
+#undef vsprintf
+#ifdef HAVE_VARIADIC_MACROS
+#define sprintf(...) BANNED(sprintf)
+#define vsprintf(...) BANNED(vsprintf)
+#else
+#define sprintf(buf,fmt,arg) BANNED(sprintf)
+#define vsprintf(buf,fmt,arg) BANNED(sprintf)
+#endif
+
#endif /* BANNED_H */
--
2.18.0.542.g2bf2fc4f7e
^ permalink raw reply related
* [PATCH v2] f2fs: clear victim_secmap when section has full valid blocks
From: Yunlong Song @ 2018-07-24 9:27 UTC (permalink / raw)
To: jaegeuk, chao, yuchao0, yunlong.song, yunlong.song
Cc: miaoxie, bintian.wang, shengyong1, heyunlei, linux-f2fs-devel,
linux-kernel
In-Reply-To: <1532355022-163029-1-git-send-email-yunlong.song@huawei.com>
Without this patch, f2fs only clears victim_secmap when it finds out
that the section has no valid blocks at all, but forgets to clear the
victim_secmap when the whole section has full valid blocks.
Signed-off-by: Yunlong Song <yunlong.song@huawei.com>
---
fs/f2fs/segment.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index cfff7cf..0a79554 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -776,7 +776,8 @@ static void __remove_dirty_segment(struct f2fs_sb_info *sbi, unsigned int segno,
if (test_and_clear_bit(segno, dirty_i->dirty_segmap[t]))
dirty_i->nr_dirty[t]--;
- if (get_valid_blocks(sbi, segno, true) == 0)
+ if (get_valid_blocks(sbi, segno, true) == 0 ||
+ get_valid_blocks(sbi, segno, true) == BLKS_PER_SEC(sbi))
clear_bit(GET_SEC_FROM_SEG(sbi, segno),
dirty_i->victim_secmap);
}
--
1.8.5.2
^ permalink raw reply related
* Re: [RFC v5 PATCH 2/2] mm: mmap: zap pages with read mmap_sem in munmap
From: Kirill A. Shutemov @ 2018-07-24 9:26 UTC (permalink / raw)
To: Yang Shi; +Cc: mhocko, willy, ldufour, akpm, linux-mm, linux-kernel
In-Reply-To: <1531956101-8526-3-git-send-email-yang.shi@linux.alibaba.com>
On Thu, Jul 19, 2018 at 07:21:41AM +0800, Yang Shi wrote:
> When running some mmap/munmap scalability tests with large memory (i.e.
> > 300GB), the below hung task issue may happen occasionally.
>
> INFO: task ps:14018 blocked for more than 120 seconds.
> Tainted: G E 4.9.79-009.ali3000.alios7.x86_64 #1
> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this
> message.
> ps D 0 14018 1 0x00000004
> ffff885582f84000 ffff885e8682f000 ffff880972943000 ffff885ebf499bc0
> ffff8828ee120000 ffffc900349bfca8 ffffffff817154d0 0000000000000040
> 00ffffff812f872a ffff885ebf499bc0 024000d000948300 ffff880972943000
> Call Trace:
> [<ffffffff817154d0>] ? __schedule+0x250/0x730
> [<ffffffff817159e6>] schedule+0x36/0x80
> [<ffffffff81718560>] rwsem_down_read_failed+0xf0/0x150
> [<ffffffff81390a28>] call_rwsem_down_read_failed+0x18/0x30
> [<ffffffff81717db0>] down_read+0x20/0x40
> [<ffffffff812b9439>] proc_pid_cmdline_read+0xd9/0x4e0
> [<ffffffff81253c95>] ? do_filp_open+0xa5/0x100
> [<ffffffff81241d87>] __vfs_read+0x37/0x150
> [<ffffffff812f824b>] ? security_file_permission+0x9b/0xc0
> [<ffffffff81242266>] vfs_read+0x96/0x130
> [<ffffffff812437b5>] SyS_read+0x55/0xc0
> [<ffffffff8171a6da>] entry_SYSCALL_64_fastpath+0x1a/0xc5
>
> It is because munmap holds mmap_sem exclusively from very beginning to
> all the way down to the end, and doesn't release it in the middle. When
> unmapping large mapping, it may take long time (take ~18 seconds to
> unmap 320GB mapping with every single page mapped on an idle machine).
>
> Zapping pages is the most time consuming part, according to the
> suggestion from Michal Hocko [1], zapping pages can be done with holding
> read mmap_sem, like what MADV_DONTNEED does. Then re-acquire write
> mmap_sem to cleanup vmas.
>
> But, some part may need write mmap_sem, for example, vma splitting. So,
> the design is as follows:
> acquire write mmap_sem
> lookup vmas (find and split vmas)
> detach vmas
> deal with special mappings
> downgrade_write
>
> zap pages
> free page tables
> release mmap_sem
>
> The vm events with read mmap_sem may come in during page zapping, but
> since vmas have been detached before, they, i.e. page fault, gup, etc,
> will not be able to find valid vma, then just return SIGSEGV or -EFAULT
> as expected.
>
> If the vma has VM_LOCKED | VM_HUGETLB | VM_PFNMAP or uprobe, they are
> considered as special mappings. They will be dealt with before zapping
> pages with write mmap_sem held. Basically, just update vm_flags.
>
> And, since they are also manipulated by unmap_single_vma() which is
> called by unmap_vma() with read mmap_sem held in this case, to
> prevent from updating vm_flags in read critical section, a new
> parameter, called "skip_flags" is added to unmap_region(), unmap_vmas()
> and unmap_single_vma(). If it is true, then just skip unmap those
> special mappings. Currently, the only place which pass true to this
> parameter is us.
>
> With this approach we don't have to re-acquire mmap_sem again to clean
> up vmas to avoid race window which might get the address space changed.
>
> And, since the lock acquire/release cost is managed to the minimum and
> almost as same as before, the optimization could be extended to any size
> of mapping without incuring significan penalty to small mappings.
>
> For the time being, just do this in munmap syscall path. Other
> vm_munmap() or do_munmap() call sites (i.e mmap, mremap, etc) remain
> intact for stability reason.
>
> With the patches, exclusive mmap_sem hold time when munmap a 80GB
> address space on a machine with 32 cores of E5-2680 @ 2.70GHz dropped to
> us level from second.
>
> munmap_test-15002 [008] 594.380138: funcgraph_entry: | vm_munmap_zap_rlock() {
> munmap_test-15002 [008] 594.380146: funcgraph_entry: !2485684 us | unmap_region();
> munmap_test-15002 [008] 596.865836: funcgraph_exit: !2485692 us | }
>
> Here the excution time of unmap_region() is used to evaluate the time of
> holding read mmap_sem, then the remaining time is used with holding
> exclusive lock.
>
> [1] https://lwn.net/Articles/753269/
>
> Suggested-by: Michal Hocko <mhocko@kernel.org>
> Suggested-by: Kirill A. Shutemov <kirill@shutemov.name>
> Cc: Matthew Wilcox <willy@infradead.org>
> Cc: Laurent Dufour <ldufour@linux.vnet.ibm.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Signed-off-by: Yang Shi <yang.shi@linux.alibaba.com>
> ---
> include/linux/mm.h | 2 +-
> mm/memory.c | 35 +++++++++++++------
> mm/mmap.c | 99 +++++++++++++++++++++++++++++++++++++++++++++++++-----
> 3 files changed, 117 insertions(+), 19 deletions(-)
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index a0fbb9f..95a4e97 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -1321,7 +1321,7 @@ void zap_vma_ptes(struct vm_area_struct *vma, unsigned long address,
> void zap_page_range(struct vm_area_struct *vma, unsigned long address,
> unsigned long size);
> void unmap_vmas(struct mmu_gather *tlb, struct vm_area_struct *start_vma,
> - unsigned long start, unsigned long end);
> + unsigned long start, unsigned long end, bool skip_flags);
skip_flags is not specific enough. Which flags? Maybe skip_vm_flags or
smething.
>
> /**
> * mm_walk - callbacks for walk_page_range
> diff --git a/mm/memory.c b/mm/memory.c
> index 7206a63..00ecdae 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -1514,7 +1514,7 @@ void unmap_page_range(struct mmu_gather *tlb,
> static void unmap_single_vma(struct mmu_gather *tlb,
> struct vm_area_struct *vma, unsigned long start_addr,
> unsigned long end_addr,
> - struct zap_details *details)
> + struct zap_details *details, bool skip_flags)
> {
> unsigned long start = max(vma->vm_start, start_addr);
> unsigned long end;
> @@ -1525,11 +1525,13 @@ static void unmap_single_vma(struct mmu_gather *tlb,
> if (end <= vma->vm_start)
> return;
>
> - if (vma->vm_file)
> - uprobe_munmap(vma, start, end);
> + if (!skip_flags) {
> + if (vma->vm_file)
> + uprobe_munmap(vma, start, end);
>
> - if (unlikely(vma->vm_flags & VM_PFNMAP))
> - untrack_pfn(vma, 0, 0);
> + if (unlikely(vma->vm_flags & VM_PFNMAP))
> + untrack_pfn(vma, 0, 0);
> + }
>
> if (start != end) {
> if (unlikely(is_vm_hugetlb_page(vma))) {
> @@ -1546,7 +1548,19 @@ static void unmap_single_vma(struct mmu_gather *tlb,
> */
> if (vma->vm_file) {
> i_mmap_lock_write(vma->vm_file->f_mapping);
> - __unmap_hugepage_range_final(tlb, vma, start, end, NULL);
> + if (!skip_flags)
> + /*
> + * The vma is being unmapped with read
> + * mmap_sem.
> + * Can't update vm_flags, it will be
> + * updated later with exclusive lock
> + * held
> + */
Later? When? Don't we run this after mmap_sem is downgraded to read?
And wrap it into {}..
> + __unmap_hugepage_range(tlb, vma, start,
> + end, NULL);
> + else
> + __unmap_hugepage_range_final(tlb, vma,
> + start, end, NULL);
> i_mmap_unlock_write(vma->vm_file->f_mapping);
> }
> } else
> @@ -1574,13 +1588,14 @@ static void unmap_single_vma(struct mmu_gather *tlb,
> */
> void unmap_vmas(struct mmu_gather *tlb,
> struct vm_area_struct *vma, unsigned long start_addr,
> - unsigned long end_addr)
> + unsigned long end_addr, bool skip_flags)
> {
> struct mm_struct *mm = vma->vm_mm;
>
> mmu_notifier_invalidate_range_start(mm, start_addr, end_addr);
> for ( ; vma && vma->vm_start < end_addr; vma = vma->vm_next)
> - unmap_single_vma(tlb, vma, start_addr, end_addr, NULL);
> + unmap_single_vma(tlb, vma, start_addr, end_addr, NULL,
> + skip_flags);
> mmu_notifier_invalidate_range_end(mm, start_addr, end_addr);
> }
>
> @@ -1604,7 +1619,7 @@ void zap_page_range(struct vm_area_struct *vma, unsigned long start,
> update_hiwater_rss(mm);
> mmu_notifier_invalidate_range_start(mm, start, end);
> for ( ; vma && vma->vm_start < end; vma = vma->vm_next) {
> - unmap_single_vma(&tlb, vma, start, end, NULL);
> + unmap_single_vma(&tlb, vma, start, end, NULL, false);
>
> /*
> * zap_page_range does not specify whether mmap_sem should be
> @@ -1641,7 +1656,7 @@ static void zap_page_range_single(struct vm_area_struct *vma, unsigned long addr
> tlb_gather_mmu(&tlb, mm, address, end);
> update_hiwater_rss(mm);
> mmu_notifier_invalidate_range_start(mm, address, end);
> - unmap_single_vma(&tlb, vma, address, end, details);
> + unmap_single_vma(&tlb, vma, address, end, details, false);
> mmu_notifier_invalidate_range_end(mm, address, end);
> tlb_finish_mmu(&tlb, address, end);
> }
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 2504094..f5d5312 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -73,7 +73,7 @@
>
> static void unmap_region(struct mm_struct *mm,
> struct vm_area_struct *vma, struct vm_area_struct *prev,
> - unsigned long start, unsigned long end);
> + unsigned long start, unsigned long end, bool skip_flags);
>
> /* description of effects of mapping type and prot in current implementation.
> * this is due to the limited x86 page protection hardware. The expected
> @@ -1824,7 +1824,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
> fput(file);
>
> /* Undo any partial mapping done by a device driver. */
> - unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end);
> + unmap_region(mm, vma, prev, vma->vm_start, vma->vm_end, false);
> charged = 0;
> if (vm_flags & VM_SHARED)
> mapping_unmap_writable(file->f_mapping);
> @@ -2559,7 +2559,7 @@ static void remove_vma_list(struct mm_struct *mm, struct vm_area_struct *vma)
> */
> static void unmap_region(struct mm_struct *mm,
> struct vm_area_struct *vma, struct vm_area_struct *prev,
> - unsigned long start, unsigned long end)
> + unsigned long start, unsigned long end, bool skip_flags)
> {
> struct vm_area_struct *next = prev ? prev->vm_next : mm->mmap;
> struct mmu_gather tlb;
> @@ -2567,7 +2567,7 @@ static void unmap_region(struct mm_struct *mm,
> lru_add_drain();
> tlb_gather_mmu(&tlb, mm, start, end);
> update_hiwater_rss(mm);
> - unmap_vmas(&tlb, vma, start, end);
> + unmap_vmas(&tlb, vma, start, end, skip_flags);
> free_pgtables(&tlb, vma, prev ? prev->vm_end : FIRST_USER_ADDRESS,
> next ? next->vm_start : USER_PGTABLES_CEILING);
> tlb_finish_mmu(&tlb, start, end);
> @@ -2778,6 +2778,79 @@ static inline void munmap_mlock_vma(struct vm_area_struct *vma,
> }
> }
>
> +/*
> + * Zap pages with read mmap_sem held
> + *
> + * uf is the list for userfaultfd
> + */
> +static int do_munmap_zap_rlock(struct mm_struct *mm, unsigned long start,
> + size_t len, struct list_head *uf)
> +{
> + unsigned long end = 0;
> + struct vm_area_struct *start_vma = NULL, *prev, *vma;
> + int ret = 0;
> +
> + if (!munmap_addr_sanity(start, len))
> + return -EINVAL;
> +
> + len = PAGE_ALIGN(len);
> +
> + end = start + len;
> +
> + /*
> + * need write mmap_sem to split vmas and detach vmas
> + * splitting vma up-front to save PITA to clean if it is failed
Please fix all the comments to have consistent style.
> + */
> + if (down_write_killable(&mm->mmap_sem))
> + return -EINTR;
> +
> + ret = munmap_lookup_vma(mm, &start_vma, &prev, start, end);
> + if (ret != 1)
> + goto out;
> +
> + if (unlikely(uf)) {
> + ret = userfaultfd_unmap_prep(start_vma, start, end, uf);
> + if (ret)
> + goto out;
> + }
> +
> + /* Handle mlocked vmas */
> + if (mm->locked_vm)
> + munmap_mlock_vma(start_vma, end);
> +
> + /* Detach vmas from rbtree */
> + detach_vmas_to_be_unmapped(mm, start_vma, prev, end);
> +
> + /*
> + * Clear uprobe, VM_PFNMAP and hugetlb mapping in advance since they
> + * need update vm_flags with write mmap_sem
> + */
> + vma = start_vma;
> + for ( ; vma && vma->vm_start < end; vma = vma->vm_next) {
> + if (vma->vm_file)
> + uprobe_munmap(vma, vma->vm_start, vma->vm_end);
> + if (unlikely(vma->vm_flags & VM_PFNMAP))
> + untrack_pfn(vma, 0, 0);
> + if (is_vm_hugetlb_page(vma))
> + vma->vm_flags &= ~VM_MAYSHARE;
> + }
> +
> + downgrade_write(&mm->mmap_sem);
> +
> + /* zap mappings with read mmap_sem */
> + unmap_region(mm, start_vma, prev, start, end, true);
> +
> + arch_unmap(mm, start_vma, start, end);
> + remove_vma_list(mm, start_vma);
> + up_read(&mm->mmap_sem);
> +
> + return 0;
> +
> +out:
> + up_write(&mm->mmap_sem);
> + return ret;
> +}
> +
> /* Munmap is split into 2 main parts -- this part which finds
> * what needs doing, and the areas themselves, which do the
> * work. This now handles partial unmappings.
> @@ -2826,7 +2899,7 @@ int do_munmap(struct mm_struct *mm, unsigned long start, size_t len,
> * Remove the vma's, and unmap the actual pages
> */
> detach_vmas_to_be_unmapped(mm, vma, prev, end);
> - unmap_region(mm, vma, prev, start, end);
> + unmap_region(mm, vma, prev, start, end, false);
>
> arch_unmap(mm, vma, start, end);
>
> @@ -2836,6 +2909,17 @@ int do_munmap(struct mm_struct *mm, unsigned long start, size_t len,
> return 0;
> }
>
> +static int vm_munmap_zap_rlock(unsigned long start, size_t len)
> +{
> + int ret;
> + struct mm_struct *mm = current->mm;
> + LIST_HEAD(uf);
> +
> + ret = do_munmap_zap_rlock(mm, start, len, &uf);
> + userfaultfd_unmap_complete(mm, &uf);
> + return ret;
> +}
> +
> int vm_munmap(unsigned long start, size_t len)
> {
> int ret;
> @@ -2855,10 +2939,9 @@ int vm_munmap(unsigned long start, size_t len)
> SYSCALL_DEFINE2(munmap, unsigned long, addr, size_t, len)
> {
> profile_munmap(addr);
> - return vm_munmap(addr, len);
> + return vm_munmap_zap_rlock(addr, len);
> }
>
> -
> /*
> * Emulation of deprecated remap_file_pages() syscall.
> */
> @@ -3146,7 +3229,7 @@ void exit_mmap(struct mm_struct *mm)
> tlb_gather_mmu(&tlb, mm, 0, -1);
> /* update_hiwater_rss(mm) here? but nobody should be looking */
> /* Use -1 here to ensure all VMAs in the mm are unmapped */
> - unmap_vmas(&tlb, vma, 0, -1);
> + unmap_vmas(&tlb, vma, 0, -1, false);
> free_pgtables(&tlb, vma, FIRST_USER_ADDRESS, USER_PGTABLES_CEILING);
> tlb_finish_mmu(&tlb, 0, -1);
>
> --
> 1.8.3.1
>
--
Kirill A. Shutemov
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.