Re: possible deadlock in shmem_fallocate (2)
On Tue, Jan 22, 2019 at 02:59:29PM +0100, Dmitry Vyukov wrote:
> On Fri, Aug 10, 2018 at 6:18 PM Matthew Wilcox wrote:
> >
> >
> > This is another ashmem lockdep splat. Forwarding to the appropriate ashmem
> > people.
>
>
> Let's test Tetsuo's patch
>
> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> master
Just to clarify, the following patch only went in, in September:
mm: shmem.c: Correctly annotate new inodes for lockdep
thanks,
- Joel
> > On Fri, Aug 10, 2018 at 04:59:02AM -0700, syzbot wrote:
> > > Hello,
> > >
> > > syzbot found the following crash on:
> > >
> > > HEAD commit:4110b42356f3 Add linux-next specific files for 20180810
> > > git tree: linux-next
> > > console output: https://syzkaller.appspot.com/x/log.txt?x=1411d6e240
> > > kernel config: https://syzkaller.appspot.com/x/.config?x=1d80606e3795a4f5
> > > dashboard link:
> > > https://syzkaller.appspot.com/bug?extid=4b8b031b89e6b96c4b2e
> > > compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> > > syzkaller repro:https://syzkaller.appspot.com/x/repro.syz?x=175052f840
> > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1187362240
> > >
> > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > Reported-by: syzbot+4b8b031b89e6b96c4...@syzkaller.appspotmail.com
> > >
> > > random: sshd: uninitialized urandom read (32 bytes read)
> > > random: sshd: uninitialized urandom read (32 bytes read)
> > > random: sshd: uninitialized urandom read (32 bytes read)
> > >
> > > ==
> > > WARNING: possible circular locking dependency detected
> > > 4.18.0-rc8-next-20180810+ #36 Not tainted
> > > --
> > > syz-executor900/4483 is trying to acquire lock:
> > > d2bfc8fe (&sb->s_type->i_mutex_key#9){}, at: inode_lock
> > > include/linux/fs.h:765 [inline]
> > > d2bfc8fe (&sb->s_type->i_mutex_key#9){}, at:
> > > shmem_fallocate+0x18b/0x12e0 mm/shmem.c:2602
> > >
> > > but task is already holding lock:
> > > 25208078 (ashmem_mutex){+.+.}, at: ashmem_shrink_scan+0xb4/0x630
> > > drivers/staging/android/ashmem.c:448
> > >
> > > which lock already depends on the new lock.
> > >
> > >
> > > the existing dependency chain (in reverse order) is:
> > >
> > > -> #2 (ashmem_mutex){+.+.}:
> > >__mutex_lock_common kernel/locking/mutex.c:925 [inline]
> > >__mutex_lock+0x171/0x1700 kernel/locking/mutex.c:1073
> > >mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1088
> > >ashmem_mmap+0x55/0x520 drivers/staging/android/ashmem.c:361
> > >call_mmap include/linux/fs.h:1844 [inline]
> > >mmap_region+0xf27/0x1c50 mm/mmap.c:1762
> > >do_mmap+0xa10/0x1220 mm/mmap.c:1535
> > >do_mmap_pgoff include/linux/mm.h:2298 [inline]
> > >vm_mmap_pgoff+0x213/0x2c0 mm/util.c:357
> > >ksys_mmap_pgoff+0x4da/0x660 mm/mmap.c:1585
> > >__do_sys_mmap arch/x86/kernel/sys_x86_64.c:100 [inline]
> > >__se_sys_mmap arch/x86/kernel/sys_x86_64.c:91 [inline]
> > >__x64_sys_mmap+0xe9/0x1b0 arch/x86/kernel/sys_x86_64.c:91
> > >do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> > >entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > >
> > > -> #1 (&mm->mmap_sem){}:
> > >__might_fault+0x155/0x1e0 mm/memory.c:4568
> > >_copy_to_user+0x30/0x110 lib/usercopy.c:25
> > >copy_to_user include/linux/uaccess.h:155 [inline]
> > >filldir+0x1ea/0x3a0 fs/readdir.c:196
> > >dir_emit_dot include/linux/fs.h:3464 [inline]
> > >dir_emit_dots include/linux/fs.h:3475 [inline]
> > >dcache_readdir+0x13a/0x620 fs/libfs.c:193
> > >iterate_dir+0x48b/0x5d0 fs/readdir.c:51
> > >__do_sys_getdents fs/readdir.c:231 [inline]
> > >__se_sys_getdents fs/readdir.c:212 [inline]
> > >__x64_sys_getdents+0x29f/0x510 fs/readdir.c:212
> > >do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> > >entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > >
> > > -> #0 (&sb->s_type->i_mutex_key#9){}:
> > >lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
> > >down_write+0x8f/0x130 kernel/locking/rwsem.c:70
> > >inode_lock include/linux/fs.h:765 [inline]
> > >shmem_fallocate+0x18b/0x12e0 mm/shmem.c:2602
> > >ashmem_shrink_scan+0x236/0x630 drivers/staging/android/ashmem.c:455
> > >ashmem_ioctl+0x3ae/0x13a0 drivers/staging/android/ashmem.c:797
> > >vfs_ioctl fs/ioctl.c:46 [inline]
> > >file_ioctl fs/ioctl.c:501 [inline]
> > >do_vfs_ioctl+0x1de/0x1720 fs/ioctl.c:685
> > >ksys_ioctl+0xa9/0xd0 fs/ioctl.c:702
> > >__do_sys_ioctl fs/ioctl.c:709 [inline]
> > >__se_sys_ioctl fs/ioctl.c:707 [inline]
> > >__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:707
> > >do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:2
Re: [PATCH] PM / EM: Expose the Energy Model in debugfs
On Monday 07 Jan 2019 at 12:26:08 (+), Quentin Perret wrote:
> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
> index d9dc2c38764a..8ef48daa62ff 100644
> --- a/kernel/power/energy_model.c
> +++ b/kernel/power/energy_model.c
> @@ -10,6 +10,7 @@
>
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -23,6 +24,88 @@ static DEFINE_PER_CPU(struct em_perf_domain *, em_data);
> */
> static DEFINE_MUTEX(em_pd_mutex);
>
> +#ifdef CONFIG_DEBUG_FS
> +static struct dentry *rootdir;
> +
> +static int em_debug_create_cs(struct em_cap_state *cs, struct dentry *pd)
> +{
> + struct dentry *d;
> + char name[24];
> +
> + snprintf(name, sizeof(name), "cs:%lu", cs->frequency);
> +
> + d = debugfs_create_dir(name, pd);
> + if (!d)
> + return -ENOMEM;
> +
> + if (!debugfs_create_ulong("frequency", 0444, d, &cs->frequency))
> + return -ENOMEM;
Looking at the patches Greg just sent I assume all this is wrong.
I'll send a v2 without the 'if' all over.
Thanks,
Quentin
> +
> + if (!debugfs_create_ulong("power", 0444, d, &cs->power))
> + return -ENOMEM;
> +
> + if (!debugfs_create_ulong("cost", 0444, d, &cs->cost))
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
> +static int em_debug_cpus_show(struct seq_file *s, void *unused)
> +{
> + seq_printf(s, "%*pbl\n", cpumask_pr_args(to_cpumask(s->private)));
> +
> + return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(em_debug_cpus);
> +
> +static int em_debug_create_pd(struct em_perf_domain *pd, int cpu)
> +{
> + struct dentry *d;
> + char name[16];
> + int i;
> +
> + if (!rootdir)
> + return -EINVAL;
> +
> + snprintf(name, sizeof(name), "pd%d", cpu);
> +
> + /* Create the directory of the performance domain */
> + d = debugfs_create_dir(name, rootdir);
> + if (!d)
> + return -ENOMEM;
> +
> + /* Create one file per pd to expose the related CPUs */
> + if (!debugfs_create_file("cpus", 0444, d, pd->cpus,
> + &em_debug_cpus_fops))
> + return -ENOMEM;
> +
> + /* Create a sub-directory for each capacity state */
> + for (i = 0; i < pd->nr_cap_states; i++) {
> + if (em_debug_create_cs(&pd->table[i], d))
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +
> +static int __init em_debug_init(void)
> +{
> + /* Create /sys/kernel/debug/energy_model directory */
> + rootdir = debugfs_create_dir("energy_model", NULL);
> + if (!rootdir) {
> + pr_err("%s: Failed to create root directory\n", __func__);
> + return -ENOMEM;
> + }
> +
> + return 0;
> +}
> +core_initcall(em_debug_init);
> +#else /* CONFIG_DEBUG_FS */
> +static int em_debug_create_pd(struct em_perf_domain *pd, int cpu)
> +{
> + return 0;
> +}
> +#endif
> static struct em_perf_domain *em_create_pd(cpumask_t *span, int nr_states,
> struct em_data_callback *cb)
> {
> @@ -102,6 +185,9 @@ static struct em_perf_domain *em_create_pd(cpumask_t
> *span, int nr_states,
> pd->nr_cap_states = nr_states;
> cpumask_copy(to_cpumask(pd->cpus), span);
>
> + if (em_debug_create_pd(pd, cpu))
> + pr_err("Failed to create debugfs for pd%d\n", cpu);
> +
> return pd;
>
> free_cs_table:
> --
> 2.20.1
>
Re: [PATCH v5 1/2] integrity, KEYS: add a reference to platform keyring
On Mon, 2019-01-21 at 17:59 +0800, Kairui Song wrote:
> commit 9dc92c45177a ('integrity: Define a trusted platform keyring')
> introduced a .platform keyring for storing preboot keys, used for
> verifying kernel images' signature. Currently only IMA-appraisal is able
> to use the keyring to verify kernel images that have their signature
> stored in xattr.
>
> This patch exposes the .platform keyring, making it
> accessible for verifying PE signed kernel images as well.
>
> Suggested-by: Mimi Zohar
> Signed-off-by: Kairui Song
Reviewed/Tested-by: Mimi Zohar
> ---
> certs/system_keyring.c| 9 +
> include/keys/system_keyring.h | 9 +
> security/integrity/digsig.c | 3 +++
> 3 files changed, 21 insertions(+)
>
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index 81728717523d..4690ef9cda8a 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -24,6 +24,9 @@ static struct key *builtin_trusted_keys;
> #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
> static struct key *secondary_trusted_keys;
> #endif
> +#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
> +static struct key *platform_trusted_keys;
> +#endif
>
> extern __initconst const u8 system_certificate_list[];
> extern __initconst const unsigned long system_certificate_list_size;
> @@ -265,4 +268,10 @@ int verify_pkcs7_signature(const void *data, size_t len,
> }
> EXPORT_SYMBOL_GPL(verify_pkcs7_signature);
>
> +#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
> +void __init set_platform_trusted_keys(struct key *keyring) {
> + platform_trusted_keys = keyring;
> +}
> +#endif
> +
> #endif /* CONFIG_SYSTEM_DATA_VERIFICATION */
> diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
> index 359c2f936004..df766ef8f03c 100644
> --- a/include/keys/system_keyring.h
> +++ b/include/keys/system_keyring.h
> @@ -61,5 +61,14 @@ static inline struct key *get_ima_blacklist_keyring(void)
> }
> #endif /* CONFIG_IMA_BLACKLIST_KEYRING */
>
> +#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
> +
> +extern void __init set_platform_trusted_keys(struct key* keyring);
> +
> +#else
> +
> +static inline void set_platform_trusted_keys(struct key* keyring) { };
> +
> +#endif /* CONFIG_INTEGRITY_PLATFORM_KEYRING */
>
> #endif /* _KEYS_SYSTEM_KEYRING_H */
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index f45d6edecf99..e19c2eb72c51 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -87,6 +87,9 @@ static int __integrity_init_keyring(const unsigned int id,
> key_perm_t perm,
> pr_info("Can't allocate %s keyring (%d)\n",
> keyring_name[id], err);
> keyring[id] = NULL;
> + } else {
> + if (id == INTEGRITY_KEYRING_PLATFORM)
> + set_platform_trusted_keys(keyring[id]);
> }
>
> return err;
Re: [PATCH v5 2/2] kexec, KEYS: Make use of platform keyring for signature verify
On Mon, 2019-01-21 at 17:59 +0800, Kairui Song wrote:
> This patch let kexec_file_load makes use of .platform keyring as fall
> back if it failed to verify a PE signed image against secondary or
> builtin key ring, make it possible to verify kernel image signed with
> preboot keys as well.
>
> This commit adds a VERIFY_USE_PLATFORM_KEYRING similar to previous
> VERIFY_USE_SECONDARY_KEYRING indicating that verify_pkcs7_signature
> should verify the signature using platform keyring. Also, decrease
> the error message log level when verification failed with -ENOKEY,
> so that if called tried multiple time with different keyring it
> won't generate extra noises.
>
> Signed-off-by: Kairui Song
Reviewed/Tested-by: Mimi Zohar
> ---
> arch/x86/kernel/kexec-bzimage64.c | 13 ++---
> certs/system_keyring.c| 13 -
> include/linux/verification.h | 1 +
> 3 files changed, 23 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kernel/kexec-bzimage64.c
> b/arch/x86/kernel/kexec-bzimage64.c
> index 7d97e432cbbc..2c007abd3d40 100644
> --- a/arch/x86/kernel/kexec-bzimage64.c
> +++ b/arch/x86/kernel/kexec-bzimage64.c
> @@ -534,9 +534,16 @@ static int bzImage64_cleanup(void *loader_data)
> #ifdef CONFIG_KEXEC_BZIMAGE_VERIFY_SIG
> static int bzImage64_verify_sig(const char *kernel, unsigned long kernel_len)
> {
> - return verify_pefile_signature(kernel, kernel_len,
> -VERIFY_USE_SECONDARY_KEYRING,
> -VERIFYING_KEXEC_PE_SIGNATURE);
> + int ret;
> + ret = verify_pefile_signature(kernel, kernel_len,
> + VERIFY_USE_SECONDARY_KEYRING,
> + VERIFYING_KEXEC_PE_SIGNATURE);
> + if (ret == -ENOKEY && IS_ENABLED(CONFIG_INTEGRITY_PLATFORM_KEYRING)) {
> + ret = verify_pefile_signature(kernel, kernel_len,
> + VERIFY_USE_PLATFORM_KEYRING,
> + VERIFYING_KEXEC_PE_SIGNATURE);
> + }
> + return ret;
> }
> #endif
>
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index 4690ef9cda8a..7085c286f4bd 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -240,11 +240,22 @@ int verify_pkcs7_signature(const void *data, size_t len,
> #else
> trusted_keys = builtin_trusted_keys;
> #endif
> + } else if (trusted_keys == VERIFY_USE_PLATFORM_KEYRING) {
> +#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
> + trusted_keys = platform_trusted_keys;
> +#else
> + trusted_keys = NULL;
> +#endif
> + if (!trusted_keys) {
> + ret = -ENOKEY;
> + pr_devel("PKCS#7 platform keyring is not available\n");
> + goto error;
> + }
> }
> ret = pkcs7_validate_trust(pkcs7, trusted_keys);
> if (ret < 0) {
> if (ret == -ENOKEY)
> - pr_err("PKCS#7 signature not signed with a trusted
> key\n");
> + pr_devel("PKCS#7 signature not signed with a trusted
> key\n");
> goto error;
> }
>
> diff --git a/include/linux/verification.h b/include/linux/verification.h
> index cfa4730d607a..018fb5f13d44 100644
> --- a/include/linux/verification.h
> +++ b/include/linux/verification.h
> @@ -17,6 +17,7 @@
> * should be used.
> */
> #define VERIFY_USE_SECONDARY_KEYRING ((struct key *)1UL)
> +#define VERIFY_USE_PLATFORM_KEYRING ((struct key *)2UL)
>
> /*
> * The use to which an asymmetric key is being put.
Re: [PATCH v3 1/5] dt-bindings: arm: fsl: Fix bindings for LS1012A and LS1021A based boards
On Mon, Jan 21, 2019 at 10:22 PM Manivannan Sadhasivam
wrote:
>
> Fix devicetree bindings for Freescale LS1012A and LS1021A SoC based boards.
>
> Fixes: a1a38e1f4d1d ("dt-bindings: arm: Convert FSL board/soc bindings to
> json-schema")
> Signed-off-by: Manivannan Sadhasivam
> ---
> Documentation/devicetree/bindings/arm/fsl.yaml | 10 +-
> 1 file changed, 9 insertions(+), 1 deletion(-)
Reviewed-by: Rob Herring
Re: [PATCH] acpi: no need to check return value of debugfs_create functions
On Tue, Jan 22, 2019 at 04:28:42PM +0100, Rafael J. Wysocki wrote: > On Tue, Jan 22, 2019 at 4:27 PM Greg Kroah-Hartman > wrote: > > > > When calling debugfs functions, there is no need to ever check the > > return value. The function can work or not, but the code logic should > > never do something different based on this. > > > > Cc: "Rafael J. Wysocki" > > Cc: Len Brown > > Cc: Tony Luck > > Cc: Borislav Petkov > > Cc: Yangtao Li > > Cc: linux-a...@vger.kernel.org > > Signed-off-by: Greg Kroah-Hartman > > Acked-by: Rafael J. Wysocki > > Or do you want me to take this? It's up to you, I can take it, or you can, either is fine with me :) greg k-h
Re: [PATCH v3 4/5] dt-bindings: arm: fsl: Add devicetree binding for Oxalis
On Mon, Jan 21, 2019 at 10:22 PM Manivannan Sadhasivam wrote: > > Add devicetree binding for LS1012A SoC based Oxalis board. > > Signed-off-by: Manivannan Sadhasivam > --- > Documentation/devicetree/bindings/arm/fsl.yaml | 1 + > 1 file changed, 1 insertion(+) Reviewed-by: Rob Herring
Re: possible deadlock in shmem_fallocate (2)
On Tue, Jan 22, 2019 at 4:34 PM Joel Fernandes wrote:
>
> On Tue, Jan 22, 2019 at 02:59:29PM +0100, Dmitry Vyukov wrote:
> > On Fri, Aug 10, 2018 at 6:18 PM Matthew Wilcox wrote:
> > >
> > >
> > > This is another ashmem lockdep splat. Forwarding to the appropriate
> > > ashmem
> > > people.
> >
> >
> > Let's test Tetsuo's patch
> >
> > #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> > master
>
> Just to clarify, the following patch only went in, in September:
> mm: shmem.c: Correctly annotate new inodes for lockdep
Is it supposed to fix this bug? This bug still happens: last time 5 hours ago:
https://syzkaller.appspot.com/bug?extid=4b8b031b89e6b96c4b2e
> thanks,
>
> - Joel
>
>
> > > On Fri, Aug 10, 2018 at 04:59:02AM -0700, syzbot wrote:
> > > > Hello,
> > > >
> > > > syzbot found the following crash on:
> > > >
> > > > HEAD commit:4110b42356f3 Add linux-next specific files for 20180810
> > > > git tree: linux-next
> > > > console output: https://syzkaller.appspot.com/x/log.txt?x=1411d6e240
> > > > kernel config:
> > > > https://syzkaller.appspot.com/x/.config?x=1d80606e3795a4f5
> > > > dashboard link:
> > > > https://syzkaller.appspot.com/bug?extid=4b8b031b89e6b96c4b2e
> > > > compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> > > > syzkaller
> > > > repro:https://syzkaller.appspot.com/x/repro.syz?x=175052f840
> > > > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1187362240
> > > >
> > > > IMPORTANT: if you fix the bug, please add the following tag to the
> > > > commit:
> > > > Reported-by: syzbot+4b8b031b89e6b96c4...@syzkaller.appspotmail.com
> > > >
> > > > random: sshd: uninitialized urandom read (32 bytes read)
> > > > random: sshd: uninitialized urandom read (32 bytes read)
> > > > random: sshd: uninitialized urandom read (32 bytes read)
> > > >
> > > > ==
> > > > WARNING: possible circular locking dependency detected
> > > > 4.18.0-rc8-next-20180810+ #36 Not tainted
> > > > --
> > > > syz-executor900/4483 is trying to acquire lock:
> > > > d2bfc8fe (&sb->s_type->i_mutex_key#9){}, at: inode_lock
> > > > include/linux/fs.h:765 [inline]
> > > > d2bfc8fe (&sb->s_type->i_mutex_key#9){}, at:
> > > > shmem_fallocate+0x18b/0x12e0 mm/shmem.c:2602
> > > >
> > > > but task is already holding lock:
> > > > 25208078 (ashmem_mutex){+.+.}, at: ashmem_shrink_scan+0xb4/0x630
> > > > drivers/staging/android/ashmem.c:448
> > > >
> > > > which lock already depends on the new lock.
> > > >
> > > >
> > > > the existing dependency chain (in reverse order) is:
> > > >
> > > > -> #2 (ashmem_mutex){+.+.}:
> > > >__mutex_lock_common kernel/locking/mutex.c:925 [inline]
> > > >__mutex_lock+0x171/0x1700 kernel/locking/mutex.c:1073
> > > >mutex_lock_nested+0x16/0x20 kernel/locking/mutex.c:1088
> > > >ashmem_mmap+0x55/0x520 drivers/staging/android/ashmem.c:361
> > > >call_mmap include/linux/fs.h:1844 [inline]
> > > >mmap_region+0xf27/0x1c50 mm/mmap.c:1762
> > > >do_mmap+0xa10/0x1220 mm/mmap.c:1535
> > > >do_mmap_pgoff include/linux/mm.h:2298 [inline]
> > > >vm_mmap_pgoff+0x213/0x2c0 mm/util.c:357
> > > >ksys_mmap_pgoff+0x4da/0x660 mm/mmap.c:1585
> > > >__do_sys_mmap arch/x86/kernel/sys_x86_64.c:100 [inline]
> > > >__se_sys_mmap arch/x86/kernel/sys_x86_64.c:91 [inline]
> > > >__x64_sys_mmap+0xe9/0x1b0 arch/x86/kernel/sys_x86_64.c:91
> > > >do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> > > >entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > > >
> > > > -> #1 (&mm->mmap_sem){}:
> > > >__might_fault+0x155/0x1e0 mm/memory.c:4568
> > > >_copy_to_user+0x30/0x110 lib/usercopy.c:25
> > > >copy_to_user include/linux/uaccess.h:155 [inline]
> > > >filldir+0x1ea/0x3a0 fs/readdir.c:196
> > > >dir_emit_dot include/linux/fs.h:3464 [inline]
> > > >dir_emit_dots include/linux/fs.h:3475 [inline]
> > > >dcache_readdir+0x13a/0x620 fs/libfs.c:193
> > > >iterate_dir+0x48b/0x5d0 fs/readdir.c:51
> > > >__do_sys_getdents fs/readdir.c:231 [inline]
> > > >__se_sys_getdents fs/readdir.c:212 [inline]
> > > >__x64_sys_getdents+0x29f/0x510 fs/readdir.c:212
> > > >do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
> > > >entry_SYSCALL_64_after_hwframe+0x49/0xbe
> > > >
> > > > -> #0 (&sb->s_type->i_mutex_key#9){}:
> > > >lock_acquire+0x1e4/0x540 kernel/locking/lockdep.c:3924
> > > >down_write+0x8f/0x130 kernel/locking/rwsem.c:70
> > > >inode_lock include/linux/fs.h:765 [inline]
> > > >shmem_fallocate+0x18b/0x12e0 mm/shmem.c:2602
> > > >ashmem_shrink_scan+0x236/0x630
> > > > drivers/staging/android/ashmem.c:455
> > > >ashmem_ioctl+0x3ae/0x13a0 drivers/staging/andr
Re: [PATCH v6 07/16] sched/core: uclamp: Add system default clamps
On 22-Jan 16:13, Peter Zijlstra wrote:
> On Tue, Jan 22, 2019 at 02:43:29PM +, Patrick Bellasi wrote:
> > On 22-Jan 14:56, Peter Zijlstra wrote:
> > > On Tue, Jan 15, 2019 at 10:15:04AM +, Patrick Bellasi wrote:
> > >
> > > > diff --git a/include/linux/sched.h b/include/linux/sched.h
> > > > index 84294925d006..c8f391d1cdc5 100644
> > > > --- a/include/linux/sched.h
> > > > +++ b/include/linux/sched.h
> > > > @@ -625,6 +625,11 @@ struct uclamp_se {
> > > > unsigned int bucket_id : bits_per(UCLAMP_BUCKETS);
> > > > unsigned int mapped : 1;
> > > > unsigned int active : 1;
> > > > + /* Clamp bucket and value actually used by a RUNNABLE task */
> > > > + struct {
> > > > + unsigned int value :
> > > > bits_per(SCHED_CAPACITY_SCALE);
> > > > + unsigned int bucket_id : bits_per(UCLAMP_BUCKETS);
> > > > + } effective;
> > >
> > > I am confuzled by this thing.. so uclamp_se already has a value,bucket,
> > > which per the prior code is the effective one.
> > >
> > > Now; I think I see why you want another value; you need the second to
> > > store the original value for when the system limits change and we must
> > > re-evaluate.
> >
> > Yes, that's one reason, the other one being to properly support
> > CGroup when we add them in the following patches.
> >
> > Effective will always track the value/bucket in which the task has
> > been refcounted at enqueue time and it depends on the aggregated
> > value.
>
> > > Should you not update all tasks?
> >
> > That's true, but that's also an expensive operation, that's why now
> > I'm doing only lazy updates at next enqueue time.
>
> Aaah, so you refcount on the original value, which allows you to skip
> fixing up all tasks. I missed that bit.
Right, effective is always tracking the bucket we refcounted at
enqueue time.
We can still argue that, the moment we change a clamp, a task should
be updated without waiting for a dequeue/enqueue cycle.
IMO, that could be a limitation only for tasks which never sleep, but
that's a very special case.
Instead, as you'll see, in the cgroup integration we force update all
RUNNABLE tasks. Although that's expensive, since we are in the domain
of the "delegation model" and "containers resources control", there
it's probably more worth to pay than here.
> > Do you think that could be acceptable?
>
> Think so, it's a sysctl poke, 'nobody' ever does that.
Cool, so... I'll keep lazy update for system default.
--
#include
Patrick Bellasi
Re: [PATCH 3/4] dma-buf: add support for mapping with dma mapping attributes
On 1/21/19 4:18 PM, Liam Mark wrote: > On Mon, 21 Jan 2019, Andrew F. Davis wrote: > >> On 1/21/19 2:20 PM, Liam Mark wrote: >>> On Mon, 21 Jan 2019, Andrew F. Davis wrote: >>> On 1/21/19 1:44 PM, Liam Mark wrote: > On Mon, 21 Jan 2019, Christoph Hellwig wrote: > >> On Sat, Jan 19, 2019 at 08:50:41AM -0800, Laura Abbott wrote: And who is going to decide which ones to pass? And who documents which ones are safe? I'd much rather have explicit, well documented dma-buf flags that might get translated to the DMA API flags, which are not error checked, not very well documented and way to easy to get wrong. >>> >>> I'm not sure having flags in dma-buf really solves anything >>> given drivers can use the attributes directly with dma_map >>> anyway, which is what we're looking to do. The intention >>> is for the driver creating the dma_buf attachment to have >>> the knowledge of which flags to use. >> >> Well, there are very few flags that you can simply use for all calls of >> dma_map*. And given how badly these flags are defined I just don't want >> people to add more places where they indirectly use these flags, as >> it will be more than enough work to clean up the current mess. >> >> What flag(s) do you want to pass this way, btw? Maybe that is where >> the problem is. >> > > The main use case is for allowing clients to pass in > DMA_ATTR_SKIP_CPU_SYNC in order to skip the default cache maintenance > which happens in dma_buf_map_attachment and dma_buf_unmap_attachment. In > ION the buffers aren't usually accessed from the CPU so this allows > clients to often avoid doing unnecessary cache maintenance. > How can a client know that no CPU access has occurred that needs to be flushed out? >>> >>> I have left this to clients, but if they own the buffer they can have the >>> knowledge as to whether CPU access is needed in that use case (example for >>> post-processing). >>> >>> For example with the previous version of ION we left all decisions of >>> whether cache maintenance was required up to the client, they would use >>> the ION cache maintenance IOCTL to force cache maintenance only when it >>> was required. >>> In these cases almost all of the access was being done by the device and >>> in the rare cases CPU access was required clients would initiate the >>> required cache maintenance before and after the CPU access. >>> >> >> I think we have different definitions of "client", I'm talking about the >> DMA-BUF client (the importer), that is who can set this flag. It seems >> you mean the userspace application, which has no control over this flag. >> > > I am also talking about dma-buf clients, I am referring to both the > userspace and kernel component of the client. For example our Camera ION > client has both a usersapce and kernel component and they have ION > buffers, which they control the access to, which may or may not be > accessed by the CPU in certain uses cases. > I know they often work together, but for this discussion it would be good to keep kernel clients and usperspace clients separate. There are three types of actors at play here, userspace clients, kernel clients, and exporters. DMA-BUF only provides the basic sync primitive + mmap directly to userspace, both operations are fulfilled by the exporter. This patch is about adding more control to the kernel side clients. The kernel side clients cannot know what userspace or other kernel side clients have done with the buffer, *only* the exporter has the whole picture. Therefor neither type of client should be deciding if the CPU needs flushed or not, only the exporter, based on the type of buffer, the current set attachments, and previous actions (is this first attachment, CPU get access in-between, etc...) can make this decision. You goal seems to be to avoid unneeded CPU side CMOs when a device detaches and another attaches with no CPU access in-between, right? That's reasonable to me, but it must be the exporter who keeps track and skips the CMO. This patch allows the client to tell the exporter the CMO is not needed and that is not safe. Andrew > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project >
Re: [PATCH v2] ima: define ima_post_create_tmpfile() hook and add missing call
On Mon, 2019-01-21 at 14:29 +0200, Amir Goldstein wrote: > On Mon, Jan 21, 2019 at 2:00 PM Mimi Zohar wrote: > > > > On Thu, 2019-01-17 at 15:34 -0600, Goldwyn Rodrigues wrote: > > > On 13:47 18/12, Mimi Zohar wrote: > > > > If tmpfiles can be made persistent, then newly created tmpfiles need to > > > > be treated like any other new files in policy. > > > > > > > > This patch indicates which newly created tmpfiles are in policy, causing > > > > the file hash to be calculated on __fput(). > > > > > > Discussed in overlayfs, this would be better if we use this on inode > > > and called from vfs_tmpfile() instead of do_tmpfile(). This will cover > > > the overlayfs case which uses tmpfiles while performing copy_up(). > > > The patch is attached. > > > > > > Here is the updated patch which works for my cases. > > > However, it is the the failure case after setting the IMA flags > > > I am concerned about, though I don't think that should be as harmful. > > > > Right. The new IMA hook allocates memory for storing the flags, which > > needs to be cleaned up on failure. For this reason, the IMA call is > > deferred until after the transition from locally freeing memory on > > failure to relying on __fput(). In "do_last", the call to IMA is > > after "opened"; and in the original version of this patch the call to > > IMA is after finish_open(). > > > > Not sure I understand the concern. > The integrity context is associated with the inode and will be freed > on destroy_inode() no matter which error path is taken. > Am I missing something? No, as long as destroy_inode() is called, it should be fine. thanks, Mimi
Re: [PATCH] realtek: no need to check return value of debugfs_create functions
On 1/22/19 9:21 AM, Greg Kroah-Hartman wrote: When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this. Cc: Ping-Ke Shih Cc: Kalle Valo Cc: linux-wirel...@vger.kernel.org Signed-off-by: Greg Kroah-Hartman Greg, Please change the subject to read "rtlwifi: ...". Otherwise the patch is correct. Acked-by: Larry Finger Thanks, Larry
[PATCH] mm,memory_hotplug: Fix scan_movable_pages for gigantic hugepages
This is the same sort of error we saw in [1]. Gigantic hugepages crosses several memblocks, so it can be that the page we get in scan_movable_pages() is a page-tail belonging to a 1G-hugepage. If that happens, page_hstate()->size_to_hstate() will return NULL, and we will blow up in hugepage_migration_supported(). The splat is as follows: kernel: BUG: unable to handle kernel NULL pointer dereference at 0008 kernel: #PF error: [normal kernel read fault] kernel: PGD 0 P4D 0 kernel: Oops: [#1] SMP PTI kernel: CPU: 1 PID: 1350 Comm: bash Tainted: GE 5.0.0-rc1-mm1-1-default+ #27 kernel: Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.0.0-prebuilt.qemu-project.org 04/01/2014 kernel: RIP: 0010:__offline_pages+0x6ae/0x900 kernel: Code: 48 c7 c6 d0 3e a4 81 e8 44 c8 ad ff 49 8b 04 24 bf 00 10 00 00 a9 00 00 01 00 74 09 41 0f b6 4c 24 51 48 d3 e7 e8 42 2a c1 ff <8b> 40 08 83 f8 09 0f 84 b0 fc ff ff 83 f8 12 0f 84 a7 fc ff ff 83 kernel: RSP: 0018:c98e3d20 EFLAGS: 00010246 kernel: RAX: RBX: ea00 RCX: 0009 kernel: RDX: 825c64f0 RSI: 1000 RDI: 1000 kernel: RBP: c98e3d68 R08: 0020 R09: 01e4 kernel: R10: 0058 R11: 8254a854 R12: ea000420 kernel: R13: 00108000 R14: 0011 R15: kernel: FS: 7ff172339b80() GS:88803eb0() knlGS: kernel: CS: 0010 DS: ES: CR0: 80050033 kernel: CR2: 0008 CR3: 38d78006 CR4: 003606a0 kernel: DR0: DR1: DR2: kernel: DR3: DR6: fffe0ff0 DR7: 0400 kernel: Call Trace: kernel: ? klist_next+0x79/0xe0 kernel: memory_subsys_offline+0x42/0x60 kernel: device_offline+0x80/0xa0 kernel: state_store+0xab/0xc0 kernel: kernfs_fop_write+0x102/0x180 kernel: __vfs_write+0x26/0x190 kernel: ? set_close_on_exec+0x49/0x70 kernel: vfs_write+0xad/0x1b0 kernel: ksys_write+0x42/0x90 kernel: do_syscall_64+0x5b/0x180 kernel: entry_SYSCALL_64_after_hwframe+0x44/0xa9 kernel: RIP: 0033:0x7ff1719febe4 kernel: Code: 00 f7 d8 64 89 02 48 c7 c0 ff ff ff ff eb b7 0f 1f 80 00 00 00 00 8b 05 4a fc 2c 00 48 63 ff 85 c0 75 13 b8 01 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 54 f3 c3 66 90 55 53 48 89 d5 48 89 f3 48 83 kernel: RSP: 002b:7ffd50b7ddc8 EFLAGS: 0246 ORIG_RAX: 0001 kernel: RAX: ffda RBX: 0008 RCX: 7ff1719febe4 kernel: RDX: 0008 RSI: 5556e9216b20 RDI: 0001 kernel: RBP: 5556e9216b20 R08: 000a R09: kernel: R10: 000a R11: 0246 R12: 0008 kernel: R13: 0001 R14: 7ff171cca720 R15: 0008 kernel: Modules linked in: af_packet(E) xt_tcpudp(E) ipt_REJECT(E) xt_conntrack(E) nf_conntrack(E) nf_defrag_ipv4(E) ip_set(E) nfnetlink(E) ebtable_nat(E) ebtable_broute(E) bridge(E) stp(E) llc(E) iptable_mangle(E) iptable_raw(E) iptable_security(E) ebtable_filter(E) ebtables(E) iptable_filter(E) ip_tables(E) x_tables(E) kvm_intel(E) kvm(E) irqbypass(E) crct10dif_pclmul(E) crc32_pclmul(E) ghash_clmulni_intel(E) bochs_drm(E) ttm(E) aesni_intel(E) drm_kms_helper(E) aes_x86_64(E) crypto_simd(E) cryptd(E) glue_helper(E) drm(E) virtio_net(E) syscopyarea(E) sysfillrect(E) net_failover(E) sysimgblt(E) pcspkr(E) failover(E) i2c_piix4(E) fb_sys_fops(E) parport_pc(E) parport(E) button(E) btrfs(E) libcrc32c(E) xor(E) zstd_decompress(E) zstd_compress(E) xxhash(E) raid6_pq(E) sd_mod(E) ata_generic(E) ata_piix(E) ahci(E) libahci(E) libata(E) crc32c_intel(E) serio_raw(E) virtio_pci(E) virtio_ring(E) virtio(E) sg(E) scsi_mod(E) autofs4(E) kernel: CR2: 0008 kernel: ---[ end trace bdb71590872849fb ]--- kernel: RIP: 0010:__offline_pages+0x6ae/0x900 kernel: Code: 48 c7 c6 d0 3e a4 81 e8 44 c8 ad ff 49 8b 04 24 bf 00 10 00 00 a9 00 00 01 00 74 09 41 0f b6 4c 24 51 48 d3 e7 e8 42 2a c1 ff <8b> 40 08 83 f8 09 0f 84 b0 fc ff ff 83 f8 12 0f 84 a7 fc ff ff 83 kernel: RSP: 0018:c98e3d20 EFLAGS: 00010246 kernel: RAX: RBX: ea00 RCX: 0009 kernel: RDX: 825c64f0 RSI: 1000 RDI: 1000 kernel: RBP: c98e3d68 R08: 0020 R09: 01e4 kernel: R10: 0058 R11: 8254a854 R12: ea000420 kernel: R13: 00108000 R14: 0011 R15: kernel: FS: 7ff172339b80() GS:88803eb0() knlGS: kernel: CS: 0010 DS: ES: CR0: 80050033 kernel: CR2: 0008 CR3: 38d78006 CR4: 003606a0 kernel: DR0: DR1: DR2: kernel: DR3: DR6: fffe0ff0 DR7: 0400 Fix this by getting the head page and testing against
Re: [PATCH 1/6] mm: make mm->pinned_vm an atomic64 counter
On Mon, Jan 21, 2019 at 09:42:15AM -0800, Davidlohr Bueso wrote:
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 6976e17dba68..640ae8a47c73 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -59,7 +59,7 @@ void task_mem(struct seq_file *m, struct mm_struct *mm)
> SEQ_PUT_DEC("VmPeak:\t", hiwater_vm);
> SEQ_PUT_DEC(" kB\nVmSize:\t", total_vm);
> SEQ_PUT_DEC(" kB\nVmLck:\t", mm->locked_vm);
> - SEQ_PUT_DEC(" kB\nVmPin:\t", mm->pinned_vm);
> + SEQ_PUT_DEC(" kB\nVmPin:\t", atomic64_read(&mm->pinned_vm));
> SEQ_PUT_DEC(" kB\nVmHWM:\t", hiwater_rss);
> SEQ_PUT_DEC(" kB\nVmRSS:\t", total_rss);
> SEQ_PUT_DEC(" kB\nRssAnon:\t", anon);
This is signed on 64b but printed as unsigned, so if some bug made pinned_vm go
negative, it would appear as an obviously wrong, gigantic number. Seems ok.
For this patch, you can add
Reviewed-by: Daniel Jordan
Re: [PATCH v6 08/16] sched/cpufreq: uclamp: Add utilization clamping for FAIR tasks
On 22-Jan 16:21, Peter Zijlstra wrote: > On Tue, Jan 15, 2019 at 10:15:05AM +, Patrick Bellasi wrote: > > --- a/kernel/sched/cpufreq_schedutil.c > > +++ b/kernel/sched/cpufreq_schedutil.c > > @@ -218,8 +218,15 @@ unsigned long schedutil_freq_util(int cpu, unsigned > > long util_cfs, > > * CFS tasks and we use the same metric to track the effective > > * utilization (PELT windows are synchronized) we can directly add them > > * to obtain the CPU's actual utilization. > > +* > > +* CFS utilization can be boosted or capped, depending on utilization > > +* clamp constraints requested by currently RUNNABLE tasks. > > +* When there are no CFS RUNNABLE tasks, clamps are released and > > +* frequency will be gracefully reduced with the utilization decay. > > */ > > - util = util_cfs; > > + util = (type == ENERGY_UTIL) > > + ? util_cfs > > + : uclamp_util(rq, util_cfs); > > That's pretty horrible; what's wrong with: > > util = util_cfs; > if (type == FREQUENCY_UTIL) > util = uclamp_util(rq, util); > > That should generate the same code, but is (IMO) far easier to read. Yes, right... and that's also the pattern we end up with the following patch on RT integration. However, as suggested by Rafael, I'll squash these two patches together and we will get rid of the above for free ;) > > util += cpu_util_rt(rq); > > > > dl_util = cpu_util_dl(rq); -- #include Patrick Bellasi
Re: [PATCH] b43legacy: no need to check return value of debugfs_create functions
On 1/22/19 9:21 AM, Greg Kroah-Hartman wrote: When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this. Cc: Larry Finger Cc: Kalle Valo Cc: linux-wirel...@vger.kernel.org Cc: b43-...@lists.infradead.org Signed-off-by: Greg Kroah-Hartman Acked-by: Larry Finger Thanks, Larry
[PATCH 1/2] irqchip: Add driver for Loongson-1 interrupt controller
This controller appeared on Loongson-1 family MCUs
including Loongson-1B and Loongson-1C.
Signed-off-by: Jiaxun Yang
---
drivers/irqchip/Kconfig| 9 ++
drivers/irqchip/Makefile | 1 +
drivers/irqchip/irq-ls1x.c | 194 +
3 files changed, 204 insertions(+)
create mode 100644 drivers/irqchip/irq-ls1x.c
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index 3d1e60779078..5dcb5456cd14 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -406,6 +406,15 @@ config IMX_IRQSTEER
help
Support for the i.MX IRQSTEER interrupt multiplexer/remapper.
+config LS1X_IRQ
+ bool "Loongson-1 Interrupt Controller"
+ depends on MACH_LOONGSON32
+ default y
+ select IRQ_DOMAIN
+ select GENERIC_IRQ_CHIP
+ help
+ Support for the Loongson-1 platform Interrupt Controller.
+
endmenu
config SIFIVE_PLIC
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index c93713d24b86..7acd0e36d0b4 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -94,3 +94,4 @@ obj-$(CONFIG_CSKY_APB_INTC) += irq-csky-apb-intc.o
obj-$(CONFIG_SIFIVE_PLIC) += irq-sifive-plic.o
obj-$(CONFIG_IMX_IRQSTEER) += irq-imx-irqsteer.o
obj-$(CONFIG_MADERA_IRQ) += irq-madera.o
+obj-$(CONFIG_LS1X_IRQ) += irq-ls1x.o
diff --git a/drivers/irqchip/irq-ls1x.c b/drivers/irqchip/irq-ls1x.c
new file mode 100644
index ..f8ca738ed25c
--- /dev/null
+++ b/drivers/irqchip/irq-ls1x.c
@@ -0,0 +1,194 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019, Jiaxun Yang
+ * Loongson-1 platform IRQ support
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+
+struct ls_intc_data {
+ void __iomem *base;
+ unsigned int chip;
+};
+
+#define MAX_CHIPS 5
+#define LS_REG_INTC_STATUS 0x00
+#define LS_REG_INTC_EN 0x04
+#define LS_REG_INTC_SET0x08
+#define LS_REG_INTC_CLR0x0c
+#define LS_REG_INTC_POL 0x10
+#define LS_REG_INTC_EDGE 0x14
+#define CHIP_SIZE 0x18
+
+static irqreturn_t intc_cascade(int irq, void *data)
+{
+ struct ls_intc_data *intc = irq_get_handler_data(irq);
+ uint32_t irq_reg;
+
+ irq_reg = readl(intc->base + (CHIP_SIZE * intc->chip)
+ + LS_REG_INTC_STATUS);
+ generic_handle_irq(__fls(irq_reg) + (intc->chip * 32) + LS1X_IRQ_BASE);
+
+ return IRQ_HANDLED;
+}
+
+static void ls_intc_set_bit(
+ struct irq_chip_generic *gc, unsigned int offset,
+ u32 mask, bool set)
+{
+ if (set)
+ writel(readl(gc->reg_base + offset) |
+ mask, gc->reg_base + offset);
+ else
+ writel(readl(gc->reg_base + offset) &
+ ~mask, gc->reg_base + offset);
+}
+
+static int ls_intc_set_type(struct irq_data *data, unsigned int type)
+{
+ struct irq_chip_generic *gc = irq_data_get_irq_chip_data(data);
+ u32 mask = data->mask;
+
+ switch (type) {
+ case IRQ_TYPE_LEVEL_HIGH:
+ ls_intc_set_bit(gc, LS_REG_INTC_EDGE, mask, false);
+ ls_intc_set_bit(gc, LS_REG_INTC_POL, mask, true);
+ break;
+ case IRQ_TYPE_LEVEL_LOW:
+ ls_intc_set_bit(gc, LS_REG_INTC_EDGE, mask, false);
+ ls_intc_set_bit(gc, LS_REG_INTC_POL, mask, false);
+ break;
+ case IRQ_TYPE_EDGE_RISING:
+ ls_intc_set_bit(gc, LS_REG_INTC_EDGE, mask, true);
+ ls_intc_set_bit(gc, LS_REG_INTC_POL, mask, true);
+ break;
+ case IRQ_TYPE_EDGE_FALLING:
+ ls_intc_set_bit(gc, LS_REG_INTC_EDGE, mask, true);
+ ls_intc_set_bit(gc, LS_REG_INTC_POL, mask, false);
+ break;
+ case IRQ_TYPE_NONE:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+
+static struct irqaction intc_cascade_action = {
+ .handler = intc_cascade,
+ .name = "intc cascade interrupt",
+};
+
+static int __init ls_intc_of_init(struct device_node *node,
+ unsigned int num_chips)
+{
+ struct ls_intc_data *intc[MAX_CHIPS];
+ struct irq_chip_generic *gc;
+ struct irq_chip_type *ct;
+ struct irq_domain *domain;
+ void __iomem *base;
+ int parent_irq[MAX_CHIPS], err = 0;
+ unsigned int i = 0;
+
+ base = of_iomap(node, 0);
+ if (!base)
+ return -ENODEV;
+
+ for (i = 0; i < num_chips; i++) {
+ /* Mask all irqs */
+ writel(0x0, base + (i * CHIP_SIZE) +
+ LS_REG_INTC_EN);
+
+ /* Set all irqs to high level triggered */
+ writel(0x, base + (i * CHIP_SIZE) +
+ LS_REG_INTC_POL);
+
+ intc[i] = kz
Re: [PATCH] b43: no need to check return value of debugfs_create functions
On 1/22/19 9:21 AM, Greg Kroah-Hartman wrote: When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this. Cc: Kalle Valo Cc: linux-wirel...@vger.kernel.org Cc: b43-...@lists.infradead.org Signed-off-by: Greg Kroah-Hartman Acked-by: Larry Finger Thanks, Larry
Re: Plain accesses and data races in the Linux Kernel Memory Model
> @@ -131,7 +159,7 @@ let rec rcu-fence = rcu-gp | srcu-gp |
> (rcu-fence ; rcu-link ; rcu-fence)
>
> (* rb orders instructions just as pb does *)
> -let rb = prop ; po ; rcu-fence ; po? ; hb* ; pb*
> +let rb = prop ; po ; rcu-fence ; po? ; hb* ; pb* ; [marked]
Testing has revealed some subtle semantics changes for some RCU tests
_without_ unmarked memory accesses; an example is reported at the end
of this email. I suspect that the improvements you mentioned in this
thread can restore the original semantics but I'm reporting this here
for further reference.
With the above definition of 'rb', we're losing links which originate
or target RCU fences, so that this definition is in fact a relaxation
w.r.t. the current semantics (even when limiting to marked accesses).
The test below, for example, is currently forbidden by the LKMM, but
it becomes allowed with this patch.
FWIW, I checked that including the RCU fences in 'marked' can restore
the original semantics of these tests; I'm still not sure whether this
change can make sense though
Thoughts?
Oh, one last (and unrelated) nit before I forget: IIUC, we used to
upper-case set names, so I'd also suggest s/marked/Marked, s/plain/Plain
and similarly for the other sets to be introduced.
Andrea
C sync-rcu-is-not-idempotent
{ }
P0(int *x, int *y)
{
int r0;
WRITE_ONCE(*x, 1);
synchronize_rcu();
synchronize_rcu();
r0 = READ_ONCE(*y);
}
P1(int *y, int *z)
{
int r0;
rcu_read_lock();
WRITE_ONCE(*y, 1);
r0 = READ_ONCE(*z);
rcu_read_unlock();
}
P2(int *z, int *x)
{
int r0;
rcu_read_lock();
WRITE_ONCE(*z, 1);
r0 = READ_ONCE(*x);
rcu_read_unlock();
}
exists (0:r0=0 /\ 1:r0=0 /\ 2:r0=0)
>
> irreflexive rb as rcu
>
Re: [RFC PATCH v1 13/13] watchdog: bd70528: Initial support for ROHM BD70528 watchdog block
On Tue, Jan 22, 2019 at 11:48:36AM +0200, Matti Vaittinen wrote:
> Initial support for watchdog block included in ROHM BD70528
> power management IC.
>
> Configurations for low power states are still to be checked.
>
> Signed-off-by: Matti Vaittinen
> ---
> drivers/watchdog/Kconfig | 12 +++
> drivers/watchdog/Makefile | 1 +
> drivers/watchdog/bd70528_wdt.c | 161
> +
> 3 files changed, 174 insertions(+)
> create mode 100644 drivers/watchdog/bd70528_wdt.c
>
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 57f017d74a97..f30e3a3e886e 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -90,6 +90,18 @@ config SOFT_WATCHDOG_PRETIMEOUT
> watchdog. Be aware that governors might affect the watchdog because it
> is purely software, e.g. the panic governor will stall it!
>
> +config BD70528_WATCHDOG
> + tristate "ROHM BD70528 PMIC Watchdog"
> + depends on MFD_ROHM_BD70528
> + select WATCHDOG_CORE
> + help
> + Support for the watchdog in the ROHM BD70528 PMIC. Watchdog trigger
> + cause system reset.
> +
> + Say Y here to include support for the ROHM BD70528 watchdog.
> + Alternatively say M to compile the driver as a module,
> + which will be called bd70528_wdt.
> +
> config DA9052_WATCHDOG
> tristate "Dialog DA9052 Watchdog"
> depends on PMIC_DA9052 || COMPILE_TEST
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index a0917ef28e07..1ce87a3b1172 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -204,6 +204,7 @@ obj-$(CONFIG_WATCHDOG_SUN4V) += sun4v_wdt.o
> obj-$(CONFIG_XEN_WDT) += xen_wdt.o
>
> # Architecture Independent
> +obj-$(CONFIG_BD70528_WATCHDOG) += bd70528_wdt.o
> obj-$(CONFIG_DA9052_WATCHDOG) += da9052_wdt.o
> obj-$(CONFIG_DA9055_WATCHDOG) += da9055_wdt.o
> obj-$(CONFIG_DA9062_WATCHDOG) += da9062_wdt.o
> diff --git a/drivers/watchdog/bd70528_wdt.c b/drivers/watchdog/bd70528_wdt.c
> new file mode 100644
> index ..e9a32566f595
> --- /dev/null
> +++ b/drivers/watchdog/bd70528_wdt.c
> @@ -0,0 +1,161 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright (C) 2018 ROHM Semiconductors
> +// ROHM BD70528MWV watchdog driver
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +static int bd70528_wdt_set(struct bd70528 *bd70528, int enable)
> +{
> + int ret;
> +
> + if (bd70528->rtc_timer_lock)
> + mutex_lock(bd70528->rtc_timer_lock);
This looks awkward. I don't think the if() is necessary.
> +
> + ret = bd70528->wdt_set(bd70528, enable, NULL);
> +
> + if (bd70528->rtc_timer_lock)
> + mutex_unlock(bd70528->rtc_timer_lock);
> + return ret;
> +}
> +
> +static int bd70528_wdt_start(struct watchdog_device *wdt)
> +{
> + struct bd70528 *bd70528 = watchdog_get_drvdata(wdt);
> +
> + return bd70528_wdt_set(bd70528, 1);
> +}
> +
> +static int bd70528_wdt_stop(struct watchdog_device *wdt)
> +{
> + struct bd70528 *bd70528 = watchdog_get_drvdata(wdt);
> +
> + return bd70528_wdt_set(bd70528, 0);
> +}
> +
> +static int bd70528_wdt_set_timeout(struct watchdog_device *wdt,
> + unsigned int timeout)
> +{
> + unsigned int hours;
> + unsigned int minutes;
> + unsigned int seconds;
> + int ret;
> + struct bd70528 *bd70528 = watchdog_get_drvdata(wdt);
> +
> + seconds = timeout;
> + hours = timeout / (60 * 60);
> + /* Maximum timeout is 1h 59m 59s => hours is 1 or 0 */
> + if (hours)
> + seconds -= (60 * 60);
> + minutes = seconds / 60;
> + seconds = seconds % 60;
> +
> + if (bd70528->rtc_timer_lock)
> + mutex_lock(bd70528->rtc_timer_lock);
> +
> + ret = bd70528->wdt_set(bd70528, 0, NULL);
> + if (ret)
> + goto out_unlock;
> +
> + ret = regmap_update_bits(bd70528->chip.regmap, BD70528_REG_WDT_HOUR,
> + BD70528_MASK_WDT_HOUR, hours);
> + if (ret) {
> + dev_err(bd70528->chip.dev, "Failed to set WDT hours\n");
> + goto out_en_unlock;
> + }
> + ret = regmap_update_bits(bd70528->chip.regmap, BD70528_REG_WDT_MINUTE,
> + BD70528_MASK_WDT_MINUTE, bin2bcd(minutes));
> + if (ret) {
> + dev_err(bd70528->chip.dev, "Failed to set WDT minutes\n");
> + goto out_en_unlock;
> + }
> + ret = regmap_update_bits(bd70528->chip.regmap, BD70528_REG_WDT_SEC,
> + BD70528_MASK_WDT_SEC, bin2bcd(seconds));
> + if (ret) {
> + dev_err(bd70528->chip.dev, "Failed to set WDT seconds\n");
> + goto out_en_unlock;
Unnecessary goto.
> + }
> +
> +out_en_unlock:
> + ret = bd70528->wdt_set(bd70528, 1, NULL);
> +out_unlock:
> + if (bd70528->rtc_timer_lock)
> +
Re: [PATCH] realtek: no need to check return value of debugfs_create functions
Larry Finger writes: > On 1/22/19 9:21 AM, Greg Kroah-Hartman wrote: >> When calling debugfs functions, there is no need to ever check the >> return value. The function can work or not, but the code logic should >> never do something different based on this. >> >> Cc: Ping-Ke Shih >> Cc: Kalle Valo >> Cc: linux-wirel...@vger.kernel.org >> Signed-off-by: Greg Kroah-Hartman > > Greg, > > Please change the subject to read "rtlwifi: ...". Otherwise the patch is > correct. I can fix that during commit. -- Kalle Valo
[PATCH] dt-bindings: Fix dt_binding_check target for in tree builds
On in tree builds, subsequent builds will incorrectly include the intermediate file 'processed-schema.yaml' with the input schema files resulting in a build error. Update the find command to ignore processed-schema.yaml. Signed-off-by: Rob Herring --- Documentation/devicetree/bindings/Makefile | 6 +- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/Makefile b/Documentation/devicetree/bindings/Makefile index 6e5cef0ed6fb..50daa0b3b032 100644 --- a/Documentation/devicetree/bindings/Makefile +++ b/Documentation/devicetree/bindings/Makefile @@ -17,7 +17,11 @@ extra-y += $(DT_TMP_SCHEMA) quiet_cmd_mk_schema = SCHEMA $@ cmd_mk_schema = $(DT_MK_SCHEMA) $(DT_MK_SCHEMA_FLAGS) -o $@ $(filter-out FORCE, $^) -DT_DOCS = $(shell cd $(srctree)/$(src) && find * -name '*.yaml') +DT_DOCS = $(shell \ + cd $(srctree)/$(src) && \ + find * \( -name '*.yaml' ! -name $(DT_TMP_SCHEMA) \) \ + ) + DT_SCHEMA_FILES ?= $(addprefix $(src)/,$(DT_DOCS)) extra-y += $(patsubst $(src)/%.yaml,%.example.dts, $(DT_SCHEMA_FILES)) -- 2.19.1
Re: [PATCH] mm: no need to check return value of debugfs_create functions
On Tue, Jan 22, 2019 at 04:31:02PM +0100, Michal Hocko wrote:
> On Tue 22-01-19 16:21:13, Greg KH wrote:
> [...]
> > diff --git a/mm/memblock.c b/mm/memblock.c
> > index 022d4cbb3618..18ee657fb918 100644
> > --- a/mm/memblock.c
> > +++ b/mm/memblock.c
> > @@ -1998,8 +1998,7 @@ DEFINE_SHOW_ATTRIBUTE(memblock_debug);
> > static int __init memblock_init_debugfs(void)
> > {
> > struct dentry *root = debugfs_create_dir("memblock", NULL);
> > - if (!root)
> > - return -ENXIO;
> > +
> > debugfs_create_file("memory", 0444, root,
> > &memblock.memory, &memblock_debug_fops);
> > debugfs_create_file("reserved", 0444, root,
>
> I haven't really read the whole patch but this has just hit my eyes. Is
> this a correct behavior?
>
> Documentations says:
> * @parent: a pointer to the parent dentry for this file. This should be a
> * directory dentry if set. If this parameter is NULL, then the
> * file will be created in the root of the debugfs filesystem.
>
> so in case of failure we would get those debugfs files outside of their
> intended scope. I believe it is much more correct to simply not create
> anything, no?
If debugfs_create_dir() returns NULL, then something is really wrong
(you passed it an invalid pointer as the parent dentry, or free memory
is gone), so there's nothing you can do except keep moving forward and
take that result and pass it as any parent pointer you want to. Your
code logic should never care if a debugfs file is created or not, it is
"fire and forget".
And any result of a debugfs call, like this one, that is to be passed
into another debugfs call, will work just fine if the first one failed
(the second one usually will also fail, which is fine.)
Also, and this is the biggest problem, everyone gets the return value
check wrong thinking NULL will be an error, it is one type of error, but
other ones are also returned and no one checks them properly. So just
don't check at all, that is the design goal here.
hope this helps,
greg k-h
Re: [PATCH] power: qos: no need to check return value of debugfs_create functions
On Tue, Jan 22, 2019 at 04:30:38PM +0100, Rafael J. Wysocki wrote: > On Tue, Jan 22, 2019 at 4:25 PM Greg Kroah-Hartman > wrote: > > > > When calling debugfs functions, there is no need to ever check the > > return value. The function can work or not, but the code logic should > > never do something different based on this. > > > > Cc: "Rafael J. Wysocki" > > Cc: Len Brown > > Cc: Pavel Machek > > Cc: linux...@vger.kernel.org > > Signed-off-by: Greg Kroah-Hartman > > All of these changes are fine by me and I can take the patches > touching the code maintained by me, so please let me know. Please take them! thanks, greg k-h
Re: [PATCH] iommu/amd: Fix IOMMU page flush when detach all devices from a domain
Joerg, On 1/22/19 5:44 PM, j...@8bytes.org wrote: > Hi Suravee, > > On Thu, Jan 17, 2019 at 08:44:36AM +, Suthikulpanit, Suravee wrote: >> Then, in __domain_flush_pages, we issue command when the dev_iommu[] >= 0. >> This should preserve previous behavior, and only add flushing condition to >> the specific IOMMU in detached state. Please let me know what you think. > > I think the whole point why VFIO is detaching all devices first and then > goes into unmapping pages is that there are no flushes needed anymore > when devices are detached. > > But when we follow your proposal we would still do IOTLB flushes even > when no devices are attached anymore. So I'd like to avoid this, given > the implications on unmapping performance. We should just flush the > IOMMU TLB at detach time and be done with it. > > Makes sense? > > Regards, > > Joerg > Thanks for the detail. Alright then, let's just go with the version you sent on 1/16/19. Do you want me to resend V3 with that changes, or would you be taking care of that? Thanks, Suravee
Re: [PATCH] acpi: no need to check return value of debugfs_create functions
On Tue, Jan 22, 2019 at 4:40 PM Greg Kroah-Hartman wrote: > > On Tue, Jan 22, 2019 at 04:28:42PM +0100, Rafael J. Wysocki wrote: > > On Tue, Jan 22, 2019 at 4:27 PM Greg Kroah-Hartman > > wrote: > > > > > > When calling debugfs functions, there is no need to ever check the > > > return value. The function can work or not, but the code logic should > > > never do something different based on this. > > > > > > Cc: "Rafael J. Wysocki" > > > Cc: Len Brown > > > Cc: Tony Luck > > > Cc: Borislav Petkov > > > Cc: Yangtao Li > > > Cc: linux-a...@vger.kernel.org > > > Signed-off-by: Greg Kroah-Hartman > > > > Acked-by: Rafael J. Wysocki > > > > Or do you want me to take this? > > It's up to you, I can take it, or you can, either is fine with me :) OK, I'll take it in case there is a clash with a different patch or similar.
[PATCH 2/2] dt-bindings: interrupt-controller: loongson ls1x intc
Dt-bindings doc about Loongson-1 interrupt controller
Signed-off-by: Jiaxun Yang
---
.../loongson,ls1x-intc.txt| 28 +++
1 file changed, 28 insertions(+)
create mode 100644
Documentation/devicetree/bindings/interrupt-controller/loongson,ls1x-intc.txt
diff --git
a/Documentation/devicetree/bindings/interrupt-controller/loongson,ls1x-intc.txt
b/Documentation/devicetree/bindings/interrupt-controller/loongson,ls1x-intc.txt
new file mode 100644
index ..afa8fec45f88
--- /dev/null
+++
b/Documentation/devicetree/bindings/interrupt-controller/loongson,ls1x-intc.txt
@@ -0,0 +1,28 @@
+Loongson ls1x Interrupt Controller
+
+Required properties:
+
+- compatible : should be "ingenic,-intc". Valid strings are:
+loongson,ls1b-intc
+loongson,ls1c-intc
+
+- reg : Specifies base physical address and size of the registers.
+- interrupt-controller : Identifies the node as an interrupt controller
+- #interrupt-cells : Specifies the number of cells needed to encode an
+ interrupt source. The value shall be 1.
+- interrupts : Specifies the CPU interrupts the controller is connected to,
+- For ls1b, it must have 4 interrupts.
+- For ls1c, it must have 5 interrupts.
+
+Example:
+
+intc: interrupt-controller@1fd01040 {
+ compatible = "loongson,ls1c-intc";
+ reg = <0x1fd01040 0x78>;
+
+ interrupt-controller;
+ #interrupt-cells = <1>;
+
+ interrupt-parent = <&cpu_intc>;
+ interrupts = <2>, <3>, <4>, <5>, <6>;
+};
\ No newline at end of file
--
2.20.1
Re: [PATCH] PM / EM: Expose the Energy Model in debugfs
On Tue, Jan 22, 2019 at 03:34:14PM +, Quentin Perret wrote:
> On Monday 07 Jan 2019 at 12:26:08 (+), Quentin Perret wrote:
> > diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
> > index d9dc2c38764a..8ef48daa62ff 100644
> > --- a/kernel/power/energy_model.c
> > +++ b/kernel/power/energy_model.c
> > @@ -10,6 +10,7 @@
> >
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -23,6 +24,88 @@ static DEFINE_PER_CPU(struct em_perf_domain *, em_data);
> > */
> > static DEFINE_MUTEX(em_pd_mutex);
> >
> > +#ifdef CONFIG_DEBUG_FS
> > +static struct dentry *rootdir;
> > +
> > +static int em_debug_create_cs(struct em_cap_state *cs, struct dentry *pd)
> > +{
> > + struct dentry *d;
> > + char name[24];
> > +
> > + snprintf(name, sizeof(name), "cs:%lu", cs->frequency);
> > +
> > + d = debugfs_create_dir(name, pd);
> > + if (!d)
> > + return -ENOMEM;
> > +
> > + if (!debugfs_create_ulong("frequency", 0444, d, &cs->frequency))
> > + return -ENOMEM;
>
> Looking at the patches Greg just sent I assume all this is wrong.
Yes it is :)
> I'll send a v2 without the 'if' all over.
That would be wonderful, thanks.
greg k-h
Re: [PATCH] regmap: no need to check return value of debugfs_create functions
On Tue, Jan 22, 2019 at 04:21:06PM +0100, Greg Kroah-Hartman wrote: > When calling debugfs functions, there is no need to ever check the > return value. The function can work or not, but the code logic should > never do something different based on this. Given that all the rest of the function is doing is further debugfs operations and when it fails people trying to use the debugfs do welcome some diagnostics I'm not sure that's particularly helpful. signature.asc Description: PGP signature
[PATCH 0/4] powerpc/livepatch: reliable stack unwinder fixes
This patchset fixes a false negative report (ie, unreliable) from the ppc64 reliable stack unwinder, discussed here [1] when it may inadvertently trip over a stale exception marker left on the stack. The first two patches fix this bug. Nicolai's change clears the marker from the stack when an exception is finished. The next patch modifies the unwinder to only look for such on stack elements when the ABI guarantees that they will actually be initialized. The final two patches consist of code cleanups that Nicolai and I spotted during the development of the fixes. Testing included re-running the original test scenario (loading a livepatch module on ppc64le) on a 5.0.0-rc2 kernel as well as a RHEL-7 backport. I ran internal tests on the RHEL-7 backport and no new test failures were introduced. I believe that Nicolai has done the same with respect to the first patch. [1] https://lore.kernel.org/lkml/7f468285-b149-37e2-e782-c9e538b99...@redhat.com/ Joe Lawrence (3): powerpc/livepatch: relax reliable stack tracer checks for first-frame powerpc/livepatch: small cleanups in save_stack_trace_tsk_reliable() powerpc/livepatch: return -ERRNO values in save_stack_trace_tsk_reliable() Nicolai Stange (1): powerpc/64s: Clear on-stack exception marker upon exception return arch/powerpc/Kconfig | 2 +- arch/powerpc/kernel/entry_64.S | 7 +++ arch/powerpc/kernel/stacktrace.c | 74 +--- 3 files changed, 47 insertions(+), 36 deletions(-) -- 2.20.1
[PATCH 3/4] powerpc/livepatch: small cleanups in save_stack_trace_tsk_reliable()
Mostly cosmetic changes:
- Group common stack pointer code at the top
- Simplify the first frame logic
- Code stackframe iteration into for...loop construct
- Check for trace->nr_entries overflow before adding any into the array
Suggested-by: Nicolai Stange
Signed-off-by: Joe Lawrence
---
arch/powerpc/kernel/stacktrace.c | 40 +++-
1 file changed, 14 insertions(+), 26 deletions(-)
diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c
index 06688f4d557b..28c3c25755d7 100644
--- a/arch/powerpc/kernel/stacktrace.c
+++ b/arch/powerpc/kernel/stacktrace.c
@@ -95,20 +95,11 @@ save_stack_trace_tsk_reliable(struct task_struct *tsk,
struct stack_trace *trace)
{
unsigned long sp;
+ unsigned long newsp;
unsigned long stack_page = (unsigned long)task_stack_page(tsk);
unsigned long stack_end;
int graph_idx = 0;
-
- /*
-* The last frame (unwinding first) may not yet have saved
-* its LR onto the stack.
-*/
- int firstframe = 1;
-
- if (tsk == current)
- sp = current_stack_pointer();
- else
- sp = tsk->thread.ksp;
+ bool firstframe;
stack_end = stack_page + THREAD_SIZE;
if (!is_idle_task(tsk)) {
@@ -135,14 +126,20 @@ save_stack_trace_tsk_reliable(struct task_struct *tsk,
stack_end -= STACK_FRAME_OVERHEAD;
}
+ if (tsk == current)
+ sp = current_stack_pointer();
+ else
+ sp = tsk->thread.ksp;
+
if (sp < stack_page + sizeof(struct thread_struct) ||
sp > stack_end - STACK_FRAME_MIN_SIZE) {
return 1;
}
- for (;;) {
+ for (firstframe = true; sp != stack_end;
+firstframe = false, sp = newsp) {
unsigned long *stack = (unsigned long *) sp;
- unsigned long newsp, ip;
+ unsigned long ip;
/* sanity check: ABI requires SP to be aligned 16 bytes. */
if (sp & 0xF)
@@ -163,10 +160,8 @@ save_stack_trace_tsk_reliable(struct task_struct *tsk,
* rest of the frame may be uninitialized, continue to
* the next.
*/
- if (firstframe) {
- firstframe = 0;
- goto next;
- }
+ if (firstframe)
+ continue;
/* Mark stacktraces with exception frames as unreliable. */
if (sp <= stack_end - STACK_INT_FRAME_SIZE &&
@@ -193,19 +188,12 @@ save_stack_trace_tsk_reliable(struct task_struct *tsk,
return 1;
#endif
+ if (trace->nr_entries >= trace->max_entries)
+ return -E2BIG;
if (!trace->skip)
trace->entries[trace->nr_entries++] = ip;
else
trace->skip--;
-
-next:
- if (newsp == stack_end)
- break;
-
- if (trace->nr_entries >= trace->max_entries)
- return -E2BIG;
-
- sp = newsp;
}
return 0;
}
--
2.20.1
[PATCH 1/4] powerpc/64s: Clear on-stack exception marker upon exception return
From: Nicolai Stange
The ppc64 specific implementation of the reliable stacktracer,
save_stack_trace_tsk_reliable(), bails out and reports an "unreliable
trace" whenever it finds an exception frame on the stack. Stack frames
are classified as exception frames if the STACK_FRAME_REGS_MARKER magic,
as written by exception prologues, is found at a particular location.
However, as observed by Joe Lawrence, it is possible in practice that
non-exception stack frames can alias with prior exception frames and thus,
that the reliable stacktracer can find a stale STACK_FRAME_REGS_MARKER on
the stack. It in turn falsely reports an unreliable stacktrace and blocks
any live patching transition to finish. Said condition lasts until the
stack frame is overwritten/initialized by function call or other means.
In principle, we could mitigate this by making the exception frame
classification condition in save_stack_trace_tsk_reliable() stronger:
in addition to testing for STACK_FRAME_REGS_MARKER, we could also take into
account that for all exceptions executing on the kernel stack
- their stack frames's backlink pointers always match what is saved
in their pt_regs instance's ->gpr[1] slot and that
- their exception frame size equals STACK_INT_FRAME_SIZE, a value
uncommonly large for non-exception frames.
However, while these are currently true, relying on them would make the
reliable stacktrace implementation more sensitive towards future changes in
the exception entry code. Note that false negatives, i.e. not detecting
exception frames, would silently break the live patching consistency model.
Furthermore, certain other places (diagnostic stacktraces, perf, xmon)
rely on STACK_FRAME_REGS_MARKER as well.
Make the exception exit code clear the on-stack STACK_FRAME_REGS_MARKER
for those exceptions running on the "normal" kernel stack and returning
to kernelspace: because the topmost frame is ignored by the reliable stack
tracer anyway, returns to userspace don't need to take care of clearing
the marker.
Furthermore, as I don't have the ability to test this on Book 3E or
32 bits, limit the change to Book 3S and 64 bits.
Finally, make the HAVE_RELIABLE_STACKTRACE Kconfig option depend on
PPC_BOOK3S_64 for documentation purposes. Before this patch, it depended
on PPC64 && CPU_LITTLE_ENDIAN and because CPU_LITTLE_ENDIAN implies
PPC_BOOK3S_64, there's no functional change here.
Fixes: df78d3f61480 ("powerpc/livepatch: Implement reliable stack tracing for
the consistency model")
Reported-by: Joe Lawrence
Signed-off-by: Nicolai Stange
Signed-off-by: Joe Lawrence
---
arch/powerpc/Kconfig | 2 +-
arch/powerpc/kernel/entry_64.S | 7 +++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
index 2890d36eb531..73bf87b1d274 100644
--- a/arch/powerpc/Kconfig
+++ b/arch/powerpc/Kconfig
@@ -220,7 +220,7 @@ config PPC
select HAVE_PERF_USER_STACK_DUMP
select HAVE_RCU_TABLE_FREE if SMP
select HAVE_REGS_AND_STACK_ACCESS_API
- select HAVE_RELIABLE_STACKTRACE if PPC64 && CPU_LITTLE_ENDIAN
+ select HAVE_RELIABLE_STACKTRACE if PPC_BOOK3S_64 &&
CPU_LITTLE_ENDIAN
select HAVE_SYSCALL_TRACEPOINTS
select HAVE_VIRT_CPU_ACCOUNTING
select HAVE_IRQ_TIME_ACCOUNTING
diff --git a/arch/powerpc/kernel/entry_64.S b/arch/powerpc/kernel/entry_64.S
index 435927f549c4..a2c168b395d2 100644
--- a/arch/powerpc/kernel/entry_64.S
+++ b/arch/powerpc/kernel/entry_64.S
@@ -1002,6 +1002,13 @@ END_FTR_SECTION_IFSET(CPU_FTR_HAS_PPR)
ld r2,_NIP(r1)
mtspr SPRN_SRR0,r2
+ /*
+* Leaving a stale exception_marker on the stack can confuse
+* the reliable stack unwinder later on. Clear it.
+*/
+ li r2,0
+ std r2,STACK_FRAME_OVERHEAD-16(r1)
+
ld r0,GPR0(r1)
ld r2,GPR2(r1)
ld r3,GPR3(r1)
--
2.20.1
Re: [PATCH 4/4] sisusb: remove useless macros and compact the code
On Tue, 2019-01-22 at 16:12 +0100, Jiri Slaby wrote:
> Remove macros which are only wrappers around standard operations. When
> we expand them into code, we see that sisusbcon_memsetw can simply use
> memset16 and sisusbcon_putcs can just call memcpy. So make the code
> compact.
[]
> diff --git a/drivers/usb/misc/sisusbvga/sisusb_con.c
> b/drivers/usb/misc/sisusbvga/sisusb_con.c
[]
> @@ -344,13 +337,11 @@ sisusbcon_invert_region(struct vc_data *vc, u16 *p, int
> count)
>*/
>
> while (count--) {
> - u16 a = sisusbcon_readw(p);
> -
> - a = ((a) & 0x88ff)|
> - (((a) & 0x7000) >> 4) |
> - (((a) & 0x0700) << 4);
> + u16 a = *p;
>
> - sisusbcon_writew(a, p++);
> + *p++ = ((a) & 0x88ff)|
> +(((a) & 0x7000) >> 4) |
> +(((a) & 0x0700) << 4);
You might remove the unnecessary parentheses
around (a) here too
[PATCH 2/4] powerpc/livepatch: relax reliable stack tracer checks for first-frame
The bottom-most stack frame (the first to be unwound) may be largely
uninitialized, for the "Power Architecture 64-Bit ELF V2 ABI" only
requires its backchain pointer to be set.
The reliable stack tracer should be careful when verifying this frame:
skip checks on STACK_FRAME_LR_SAVE and STACK_FRAME_MARKER offsets that
may contain uninitialized residual data.
Fixes: df78d3f61480 ("powerpc/livepatch: Implement reliable stack tracing for
the consistency model")
Signed-off-by: Joe Lawrence
---
arch/powerpc/kernel/stacktrace.c | 32
1 file changed, 24 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c
index e2c50b55138f..06688f4d557b 100644
--- a/arch/powerpc/kernel/stacktrace.c
+++ b/arch/powerpc/kernel/stacktrace.c
@@ -84,6 +84,12 @@ save_stack_trace_regs(struct pt_regs *regs, struct
stack_trace *trace)
EXPORT_SYMBOL_GPL(save_stack_trace_regs);
#ifdef CONFIG_HAVE_RELIABLE_STACKTRACE
+/*
+ * This function returns an error if it detects any unreliable features of the
+ * stack. Otherwise it guarantees that the stack trace is reliable.
+ *
+ * If the task is not 'current', the caller *must* ensure the task is inactive.
+ */
int
save_stack_trace_tsk_reliable(struct task_struct *tsk,
struct stack_trace *trace)
@@ -142,12 +148,6 @@ save_stack_trace_tsk_reliable(struct task_struct *tsk,
if (sp & 0xF)
return 1;
- /* Mark stacktraces with exception frames as unreliable. */
- if (sp <= stack_end - STACK_INT_FRAME_SIZE &&
- stack[STACK_FRAME_MARKER] == STACK_FRAME_REGS_MARKER) {
- return 1;
- }
-
newsp = stack[0];
/* Stack grows downwards; unwinder may only go up. */
if (newsp <= sp)
@@ -158,11 +158,26 @@ save_stack_trace_tsk_reliable(struct task_struct *tsk,
return 1; /* invalid backlink, too far up. */
}
+ /*
+* We can only trust the bottom frame's backlink, the
+* rest of the frame may be uninitialized, continue to
+* the next.
+*/
+ if (firstframe) {
+ firstframe = 0;
+ goto next;
+ }
+
+ /* Mark stacktraces with exception frames as unreliable. */
+ if (sp <= stack_end - STACK_INT_FRAME_SIZE &&
+ stack[STACK_FRAME_MARKER] == STACK_FRAME_REGS_MARKER) {
+ return 1;
+ }
+
/* Examine the saved LR: it must point into kernel code. */
ip = stack[STACK_FRAME_LR_SAVE];
- if (!firstframe && !__kernel_text_address(ip))
+ if (!__kernel_text_address(ip))
return 1;
- firstframe = 0;
/*
* FIXME: IMHO these tests do not belong in
@@ -183,6 +198,7 @@ save_stack_trace_tsk_reliable(struct task_struct *tsk,
else
trace->skip--;
+next:
if (newsp == stack_end)
break;
--
2.20.1
[PATCH 4/4] powerpc/livepatch: return -ERRNO values in save_stack_trace_tsk_reliable()
To match its x86 counterpart, save_stack_trace_tsk_reliable() should
return -EINVAL in cases that it is currently returning 1. No caller is
currently differentiating non-zero error codes, but let's keep the
arch-specific implementations consistent.
Signed-off-by: Joe Lawrence
---
arch/powerpc/kernel/stacktrace.c | 14 +++---
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/arch/powerpc/kernel/stacktrace.c b/arch/powerpc/kernel/stacktrace.c
index 28c3c25755d7..cf31ce6c1f53 100644
--- a/arch/powerpc/kernel/stacktrace.c
+++ b/arch/powerpc/kernel/stacktrace.c
@@ -133,7 +133,7 @@ save_stack_trace_tsk_reliable(struct task_struct *tsk,
if (sp < stack_page + sizeof(struct thread_struct) ||
sp > stack_end - STACK_FRAME_MIN_SIZE) {
- return 1;
+ return -EINVAL;
}
for (firstframe = true; sp != stack_end;
@@ -143,16 +143,16 @@ save_stack_trace_tsk_reliable(struct task_struct *tsk,
/* sanity check: ABI requires SP to be aligned 16 bytes. */
if (sp & 0xF)
- return 1;
+ return -EINVAL;
newsp = stack[0];
/* Stack grows downwards; unwinder may only go up. */
if (newsp <= sp)
- return 1;
+ return -EINVAL;
if (newsp != stack_end &&
newsp > stack_end - STACK_FRAME_MIN_SIZE) {
- return 1; /* invalid backlink, too far up. */
+ return -EINVAL; /* invalid backlink, too far up. */
}
/*
@@ -166,13 +166,13 @@ save_stack_trace_tsk_reliable(struct task_struct *tsk,
/* Mark stacktraces with exception frames as unreliable. */
if (sp <= stack_end - STACK_INT_FRAME_SIZE &&
stack[STACK_FRAME_MARKER] == STACK_FRAME_REGS_MARKER) {
- return 1;
+ return -EINVAL;
}
/* Examine the saved LR: it must point into kernel code. */
ip = stack[STACK_FRAME_LR_SAVE];
if (!__kernel_text_address(ip))
- return 1;
+ return -EINVAL;
/*
* FIXME: IMHO these tests do not belong in
@@ -185,7 +185,7 @@ save_stack_trace_tsk_reliable(struct task_struct *tsk,
* as unreliable.
*/
if (ip == (unsigned long)kretprobe_trampoline)
- return 1;
+ return -EINVAL;
#endif
if (trace->nr_entries >= trace->max_entries)
--
2.20.1
Re: [PATCH v6 1/2] PM-runtime: update accounting_timestamp only when enable
On Tue, 22 Jan 2019 at 16:14, Rafael J. Wysocki wrote: > > On Tue, Jan 22, 2019 at 3:24 PM Vincent Guittot > wrote: > > > > Initializing accounting_timestamp to something different from 0 during > > pm_runtime_init() doesn't make sense and put useless ordering constraint > > between > > timekeeping_init() and pm_runtime_init(). > > PM runtime should start accounting time only when it is enable and discard > > the period when disabled. > > Set accounting_timestamp to now when enabling PM runtime. > > > > Suggested-by: "Rafael J. Wysocki" > > Signed-off-by: Vincent Guittot > > --- > > drivers/base/power/runtime.c | 6 +- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c > > index fb5e2b6..7df1d05 100644 > > --- a/drivers/base/power/runtime.c > > +++ b/drivers/base/power/runtime.c > > @@ -1306,6 +1306,10 @@ void pm_runtime_enable(struct device *dev) > > > > spin_lock_irqsave(&dev->power.lock, flags); > > > > + /* About to enable runtime pm, set accounting_timestamp to now */ > > + if (dev->power.disable_depth == 1) > > + dev->power.accounting_timestamp = jiffies; > > I would do this in the dev->power.disable_depth > 0 branch below. > Just check if disable_depth is zero after decrementing it and update > accounting_timestamp if so. My goal was to prevent nested "if" which is sometime difficult to read. I'm going to change that and send a new version Vincent > > > + > > if (dev->power.disable_depth > 0) > > dev->power.disable_depth--; > > else > > @@ -1506,7 +1510,7 @@ void pm_runtime_init(struct device *dev) > > dev->power.request_pending = false; > > dev->power.request = RPM_REQ_NONE; > > dev->power.deferred_resume = false; > > - dev->power.accounting_timestamp = jiffies; > > + dev->power.accounting_timestamp = 0; > > INIT_WORK(&dev->power.work, pm_runtime_work); > > > > dev->power.timer_expires = 0; > > -- > > 2.7.4 > >
Re: [PATCH 3/4] dma-buf: add support for mapping with dma mapping attributes
On 1/21/19 4:12 PM, Liam Mark wrote: > On Mon, 21 Jan 2019, Christoph Hellwig wrote: > >> On Mon, Jan 21, 2019 at 11:44:10AM -0800, Liam Mark wrote: >>> The main use case is for allowing clients to pass in >>> DMA_ATTR_SKIP_CPU_SYNC in order to skip the default cache maintenance >>> which happens in dma_buf_map_attachment and dma_buf_unmap_attachment. In >>> ION the buffers aren't usually accessed from the CPU so this allows >>> clients to often avoid doing unnecessary cache maintenance. >> >> This can't work. The cpu can still easily speculate into this area. > > Can you provide more detail on your concern here. > The use case I am thinking about here is a cached buffer which is accessed > by a non IO-coherent device (quite a common use case for ION). > > Guessing on your concern: > The speculative access can be an issue if you are going to access the > buffer from the CPU after the device has written to it, however if you > know you aren't going to do any CPU access before the buffer is again > returned to the device then I don't think the speculative access is a > concern. > >> Moreover in general these operations should be cheap if the addresses >> aren't cached. >> > > I am thinking of use cases with cached buffers here, so CMO isn't cheap. > These buffers are cacheable, not cached, if you haven't written anything the data wont actually be in cache. And in the case of speculative cache filling the lines are marked clean. In either case the only cost is the little 7 instruction loop calling the clean/invalidate instruction (dc civac for ARMv8) for the cache-lines. Unless that is the cost you are trying to avoid? In that case if you are mapping and unmapping so much that the little CMO here is hurting performance then I would argue your usage is broken and needs to be re-worked a bit. Andrew > > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project >
Re: [PATCHv4 1/4] arm64: dts: qcom: sdm845: Add Coresight support
Hi Sai,
On 01/22/2019 03:02 PM, Sai Prakash Ranjan wrote:
Hi Suzuki,
Thanks for looking into this. Please find my response inline.
On 1/22/2019 7:30 PM, Suzuki K Poulose wrote:
Hi Sai,
On 01/22/2019 01:37 PM, Sai Prakash Ranjan wrote:
Add coresight components found on Qualcomm SDM845 SoC.
Signed-off-by: Sai Prakash Ranjan
Sorry, but I hadn't noticed the PID override strings below. Please
find the question.
[..]
+ /*
+ * On QCOM SDM845, we bypass the normal AMBA bus discovery
+ * method by forcing the peripheral ID because of the wrong
+ * value read from ETM PID registers.
+ */
What is the value read back from the ETM PIDx registers ? Do they
provide inconsistent or incompatible value w.r.t the ETM/Coresight
architecture ? If it is an unsupported CPU with proper values,
you must add them to the table in etm4x driver.
The values read from ETM PIDx registers are actually inconsistent
and is different for some ETMs.
By inconsistent, I meant the registers provides values which are not
the same on two different CPUs of the *same type*. And it is expected
that two different CPU/ETM implementations will have different PIDs.
Below are the PIDs read for SDM845:
[ 5.996448] resname=etm@704 pid=001bb803
[ 6.052891] resname=etm@714 pid=001bb803
.. (Same pid=001bb803 for etm@724 to etm@754 but differs
for other 2 cpus as shown below)
[ 6.266687] resname=etm@764 pid=001bb802
[ 6.329171] resname=etm@774 pid=001bb802
This is the case for MSM8996 also as shown below where PID
value is not correct and has to be hardcoded.
They differ because they are two different types of CPU cores (and thus
different ETM PIDs). What does the Register descriptions say for
the Cores ?
To me it looks like there are two different types of Qualcomm
Cores with their respective ETMs which are missing in the ETM4x
driver and we are trying to "make the ETM" work by faking it as
something else, which is not nice. I would rather prefer
to add the appropriate masks and the expected value for these
two different ETM implementations and be done with it, instead
of faking it in all the DTs where these cores appear.
For MSM8996:
resname=etm@3b4 pid=102f0205
I don't know what CPUs the MSM8996 have. If they don't have A53, you
must add the actual PIDs to the table once and for all.
+ etm@704 {
+ compatible = "arm,coresight-etm4x", "arm,primecell";
+ arm,primecell-periphid = <0x000bb95d> > + reg
= <0 0x0704 0 0x1000>;
+
+ cpu = <&CPU0>;
+
You seem to be specifying the PID of A53 ETM all over, while at least
one of your cores is ETMv4.2 (from the other patch) and A53 is not
ETMv4.2. As above, it would be good to add the PID to the table.
As explained in above comment, PID values read are not correct. Please
let me know if I am not clear.
There is no measure for "correctness" here. If the ETM exposes different
PID than what you expect from the TRM, then we could think of overriding
it. Otherwise please add the PIDs to the table.
Cheers
Suzuki
Re: [PATCH] mm: no need to check return value of debugfs_create functions
On Tue 22-01-19 16:52:55, Greg KH wrote:
> On Tue, Jan 22, 2019 at 04:31:02PM +0100, Michal Hocko wrote:
> > On Tue 22-01-19 16:21:13, Greg KH wrote:
> > [...]
> > > diff --git a/mm/memblock.c b/mm/memblock.c
> > > index 022d4cbb3618..18ee657fb918 100644
> > > --- a/mm/memblock.c
> > > +++ b/mm/memblock.c
> > > @@ -1998,8 +1998,7 @@ DEFINE_SHOW_ATTRIBUTE(memblock_debug);
> > > static int __init memblock_init_debugfs(void)
> > > {
> > > struct dentry *root = debugfs_create_dir("memblock", NULL);
> > > - if (!root)
> > > - return -ENXIO;
> > > +
> > > debugfs_create_file("memory", 0444, root,
> > > &memblock.memory, &memblock_debug_fops);
> > > debugfs_create_file("reserved", 0444, root,
> >
> > I haven't really read the whole patch but this has just hit my eyes. Is
> > this a correct behavior?
> >
> > Documentations says:
> > * @parent: a pointer to the parent dentry for this file. This should be a
> > * directory dentry if set. If this parameter is NULL, then the
> > * file will be created in the root of the debugfs filesystem.
> >
> > so in case of failure we would get those debugfs files outside of their
> > intended scope. I believe it is much more correct to simply not create
> > anything, no?
>
> If debugfs_create_dir() returns NULL, then something is really wrong
> (you passed it an invalid pointer as the parent dentry, or free memory
> is gone), so there's nothing you can do except keep moving forward and
> take that result and pass it as any parent pointer you want to. Your
> code logic should never care if a debugfs file is created or not, it is
> "fire and forget".
OK, but does it make any sense to continue creating files when you know
that the parent directory has failed to create? What kind of advantage
does this have?
> And any result of a debugfs call, like this one, that is to be passed
> into another debugfs call, will work just fine if the first one failed
> (the second one usually will also fail, which is fine.)
>
> Also, and this is the biggest problem, everyone gets the return value
> check wrong thinking NULL will be an error, it is one type of error, but
> other ones are also returned and no one checks them properly. So just
> don't check at all, that is the design goal here.
sounds like a poor design goal to me but not mine code to maintain so...
--
Michal Hocko
SUSE Labs
Re: [PATCH 4/6] dt-bindings: display: armada: Add display subsystem binding
On Tue, Jan 22, 2019 at 4:58 AM Russell King - ARM Linux admin
wrote:
>
> On Mon, Jan 21, 2019 at 05:58:50PM -0600, Rob Herring wrote:
> > On Mon, Jan 21, 2019 at 11:53 AM Russell King - ARM Linux admin
> > wrote:
> > >
> > > On Mon, Jan 21, 2019 at 10:07:11AM -0600, Rob Herring wrote:
> > > > On Mon, Jan 21, 2019 at 9:46 AM Lubomir Rintel wrote:
> > > > >
> > > > > On Mon, 2019-01-21 at 09:35 -0600, Rob Herring wrote:
> > > > > > On Sun, Jan 20, 2019 at 11:26 AM Lubomir Rintel
> > > > > > wrote:
> > > > > > > The Marvell Armada DRM master device is a virtual device needed
> > > > > > > to list all
> > > > > > > nodes that comprise the graphics subsystem.
> > > > > > >
> > > > > > > Signed-off-by: Lubomir Rintel
> > > > > > > ---
> > > > > > > .../display/armada/marvell-armada-drm.txt | 24
> > > > > > > +++
> > > > > > > 1 file changed, 24 insertions(+)
> > > > > > >
> > > > > > > diff --git
> > > > > > > a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
> > > > > > >
> > > > > > > b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
> > > > > > > index de4cca9432c8..3dbfa8047f0b 100644
> > > > > > > ---
> > > > > > > a/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
> > > > > > > +++
> > > > > > > b/Documentation/devicetree/bindings/display/armada/marvell-armada-drm.txt
> > > > > > > @@ -1,3 +1,27 @@
> > > > > > > +Marvell Armada DRM master device
> > > > > > > +
> > > > > > > +
> > > > > > > +The Marvell Armada DRM master device is a virtual device needed
> > > > > > > to list all
> > > > > > > +nodes that comprise the graphics subsystem.
> > > > > > > +
> > > > > > > +Required properties:
> > > > > > > +
> > > > > > > + - compatible: value should be "marvell,dove-display-subsystem",
> > > > > > > + "marvell,armada-display-subsystem"
> > > > > > > + - ports: a list of phandles pointing to display interface ports
> > > > > > > of CRTC
> > > > > > > + devices
> > > > > > > + - memory-region: phandle to a node describing memory to be used
> > > > > > > for the
> > > > > > > + framebuffer
> > > > > > > +
> > > > > > > +Example:
> > > > > > > +
> > > > > > > + display-subsystem {
> > > > > > > + compatible = "marvell,dove-display-subsystem",
> > > > > > > +"marvell,armada-display-subsystem";
> > > > > > > + memory-region = <&display_reserved>;
> > > > > > > + ports = <&lcd0_port>;
> > > > > >
> > > > > > If there is only one device, you don't need this virtual node.
> > > > >
> > > > > By "one device" you mean one LCD controller (CRTC)?
> > > >
> > > > Yes.
> > >
> > > How does that work (as far as the Linux implementation) ? I can't see
> > > a way that could work, while allowing the flexibility that Armada DRM
> > > allows (two completely independent LCD controllers as two separate DRM
> > > devices vs one DRM device containing both LCD controllers.)
> > >
> > > > > I suppose in the (single CRTC) example case, the display-subsystem
> > > > > node
> > > > > used to associate it with the memory region reserved for allocating
> > > > > the
> > > > > frame buffers from. Could that be done differently?
> > > >
> > > > Move memory-region to the LCD controller node.
> > >
> > > That doesn't work - it would appear in the wrong part of the driver.
> >
> > Why? You can fetch properties from other nodes.
> >
> > If you have 2 CRTCs, do you have 1 or 2 reserved memory regions? I'd
> > think 2 with each one in the corresponding LCDC that uses them would
> > be more flexible.
>
> There would still be one reserved memory region, since it is shared
> between both LCDCs.
>
> > Or just get the data out of the /reserved-memory node directly. Surely
> > it has a compatible that you can find it with.
>
> I see that the DT reserved memory support has progressed since I wrote
> the armada code to deal with it, and it's now possible to make use of
> reserved memory via of_reserved_mem_lookup() rather than using the
> RESERVEDMEM_OF_DECLARE() and so forth.
>
> > > > > Also, if the node is indeed made optional, then it's going to
> > > > > complicate things on the DRM side. Currently the driver that binds to
> > > > > the node creates the DRM device once it sees all the components
> > > > > connected to the ports appear. If we loose it, then the LCD controller
> > > > > driver would somehow need to find out that it's alone and create the
> > > > > DRM device itself.
> > > >
> > > > DT is not the only way to create devices. The DRM driver can bind to
> > > > the LCDC node and then create a child CRTC device (or even multiple
> > > > ones for h/w with multiple pipelines).
> > >
> > > That seems completely upside down and rediculous to me - are you
> > > really suggesting that we should have some kind of virtual device
> > > in DT, and omit the _real_ physical devices for that, having the
> > > driver create the dev
Re: [PATCH] backing-dev: no need to check return value of debugfs_create functions
On 2019-01-22 16:21:07 [+0100], Greg Kroah-Hartman wrote:
> diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> index 8a8bb8796c6c..85ef344a9c67 100644
> --- a/mm/backing-dev.c
> +++ b/mm/backing-dev.c
> @@ -102,39 +102,25 @@ static int bdi_debug_stats_show(struct seq_file *m,
> void *v)
> }
> DEFINE_SHOW_ATTRIBUTE(bdi_debug_stats);
>
> -static int bdi_debug_register(struct backing_dev_info *bdi, const char *name)
> +static void bdi_debug_register(struct backing_dev_info *bdi, const char
> *name)
> {
> - if (!bdi_debug_root)
> - return -ENOMEM;
> -
> bdi->debug_dir = debugfs_create_dir(name, bdi_debug_root);
If this fails then ->debug_dir is NULL
> - if (!bdi->debug_dir)
> - return -ENOMEM;
> -
> - bdi->debug_stats = debugfs_create_file("stats", 0444, bdi->debug_dir,
> -bdi, &bdi_debug_stats_fops);
> - if (!bdi->debug_stats) {
> - debugfs_remove(bdi->debug_dir);
> - bdi->debug_dir = NULL;
> - return -ENOMEM;
> - }
>
> - return 0;
> + debugfs_create_file("stats", 0444, bdi->debug_dir, bdi,
> + &bdi_debug_stats_fops);
then this creates the stats file in the root folder and
> }
>
> static void bdi_debug_unregister(struct backing_dev_info *bdi)
> {
> - debugfs_remove(bdi->debug_stats);
> - debugfs_remove(bdi->debug_dir);
> + debugfs_remove_recursive(bdi->debug_dir);
this won't remove it.
If you return for "debug_dir == NULL" then it is a nice cleanup.
Sebastian
Re: [PATCH 4.19 00/99] 4.19.17-stable review
On Mon, 21 Jan 2019 at 19:30, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 4.19.17 release. > There are 99 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 Jan 23 13:48:56 UTC 2019. > 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.17-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 Results from Linaro’s test farm. No regressions on arm64, arm, x86_64, and i386. Summary kernel: 4.19.17-rc1 git repo: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git git branch: linux-4.19.y git commit: 23a84dfade5b350d25ab6afd6a96244a46572511 git describe: v4.19.16-102-g23a84dfade5b Test details: https://qa-reports.linaro.org/lkft/linux-stable-rc-4.19-oe/build/v4.19.16-102-g23a84dfade5b No regressions (compared to build v4.19.16) No fixes (compared to build v4.19.16) Ran 14175 total tests in the following environments and test suites. Environments -- - dragonboard-410c - arm64 - hi6220-hikey - arm64 - i386 - juno-r2 - arm64 - qemu_arm - qemu_arm64 - qemu_i386 - qemu_x86_64 - x15 - arm Test Suites --- * boot * install-android-platform-tools-r2600 * kselftest * libhugetlbfs * ltp-cap_bounds-tests * ltp-containers-tests * ltp-cpuhotplug-tests * ltp-cve-tests * ltp-fcntl-locktests-tests * ltp-filecaps-tests * ltp-fs-tests * ltp-fs_bind-tests * ltp-fs_perms_simple-tests * ltp-fsx-tests * ltp-hugetlb-tests * ltp-io-tests * ltp-ipc-tests * ltp-math-tests * ltp-nptl-tests * ltp-pty-tests * ltp-sched-tests * ltp-securebits-tests * ltp-syscalls-tests * ltp-timers-tests * spectre-meltdown-checker-test * ltp-open-posix-tests -- Linaro LKFT https://lkft.linaro.org
Re: [PATCH] ARM: dts: imx: Add support for Logic PD i.MX6QD EVM
On Tue, Jan 22, 2019 at 9:19 AM Fabio Estevam wrote:
>
> Hi Adam,
>
> On Tue, Jan 22, 2019 at 12:07 PM Adam Ford wrote:
>
> > + reg_audio: regulator-audio {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_reg_audio>;
> > + compatible = "regulator-fixed";
> > + regulator-name = "3v3_aud";
> > + regulator-min-microvolt = <330>;
> > + regulator-max-microvolt = <330>;
> > + gpio = <&gpio1 29 GPIO_ACTIVE_HIGH>;
> > + enable-active-high;
> > + regulator-always-on;
>
> It seems that the 'regulator-always-on' property is not needed in this case.
>
> > +&i2c1 {
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_i2c1>;
> > + clock-frequency = <40>;
> > + status = "okay";
> > +
> > + codec: wm8962@1a {
>
> Node name should be generic and label name should be specific, so:
>
> wm8962: codec@1a {
>
> > + chosen {
> > + stdout-path = &uart1;
> > + };
> > +
> > + memory@1000 {
>
> Need to pass device_type = "memory";
>
> > + reg = <0x1000 0x8000>;
> > + };
> > +
> > + reg_wl18xx_vmmc: regulator-wl18xx {
> > + compatible = "regulator-fixed";
> > + regulator-name = "vwl1837";
> > + regulator-min-microvolt = <330>;
> > + regulator-max-microvolt = <330>;
> > + gpio = <&gpio7 0 GPIO_ACTIVE_HIGH>;
> > + startup-delay-us = <7>;
> > + enable-active-high;
> > + };
> > +
>
> No need for this blank line.
>
> > +};
>
> > +&i2c3 {
> > + clock-frequency = <10>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_i2c3>;
> > + status = "okay";
> > +
> > + pmic: pfuze100@8 {
>
> pfuze100: pmic@8
>
> > + tmp102_0: tmp102@4a {
>
> generic node name, please.
I have two temperature sensors. Should I just call them tempsense0
and tempsense1?
>
> > + compatible = "ti,tmp102";
> > + reg = <0x4a>;
> > + pinctrl-names = "default";
> > + pinctrl-0 = <&pinctrl_tempsense>;
> > + interrupt-parent = <&gpio6>;
> > + interrupts = <15 IRQ_TYPE_LEVEL_LOW>;
> > + #thermal-sensor-cells = <1>;
> > + };
> > +
> > + tmp102_1: tmp102@49 {
>
> Ditto
>
> > + compatible = "ti,tmp102";
> > + reg = <0x49>;
> > + interrupt-parent = <&gpio6>;
> > + interrupts = <15 IRQ_TYPE_LEVEL_LOW>;
> > + #thermal-sensor-cells = <1>;
> > + };
> > +
> > + at24c64_mfg: at24@51 {
I have two eeproms. One is a programmed by the factory with info like
serial number, model name, etc. The other is blank and available for
users.
Should I name them something like 'eeprom_mfg' and 'eeprom_usr'?
>
> Ditto
>
> > + compatible = "atmel,24c64";
> > + pagesize = <32>;
> > + read-only; /* Manufacturing EEPROM programmed at
> > factory */
> > + reg = <0x51>;
> > + };
> > +
> > + at24c64_usr: at24@52 {
>
> Ditto
>
> > + panel-lvds0 {
> > + compatible = "ampire,am800480b3tmqw";
>
> Could not found this compatible documented.
I'll pull this out in V2. I pushed a patch a while ago for this
display, but it's not been accepted yet, and I need to follow up on
this.
>
> > +&i2c1 {
> > + ili_touch: ilitouch@26 {
>
> Generic node name, please.
>
> > + compatible = "ili,ili2117a";
>
> Could not found this compatible documented.
I'll pull this out in V2. I have a working driver, but I haven't
pushed it yet. I'll add them back in once the driver's been pushed
and accepted.
thanks for reviewing this so quickly.
adam
Re: [PATCH V2] wlcore: sdio: Fixup power on/off sequence
On 21.01.19 15:40, Ulf Hansson wrote:
On Fri, 18 Jan 2019 at 16:09, Ulf Hansson wrote:
On Fri, 18 Jan 2019 at 13:09, Jan Kiszka wrote:
On 17.01.19 10:54, Ulf Hansson wrote:
On Wed, 16 Jan 2019 at 21:26, Jan Kiszka wrote:
On 16.01.19 12:37, Ulf Hansson wrote:
During "wlan-up", we are programming the FW into the WiFi-chip. However,
re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
is made in-between the programmings.
To conform to this requirement and to fix the regression in a simple way,
let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
(runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
current code is to treat this scenario as an error, but unfortunate this
doesn't work as expected, so let's fix this.
The other part is to guarantee that a power cycle of the SDIO card has been
completed when wl12xx_sdio_power_on() returns, as to allow the FW
programming to succeed. However, relying solely on runtime PM to deal with
this isn't sufficient. For example, userspace may prevent runtime suspend
via sysfs for the device that represents the SDIO card, leading to that the
mmc core also keeps it powered on. For this reason, let's instead do a
brute force power cycle in wl12xx_sdio_power_on().
Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
Signed-off-by: Ulf Hansson
---
Changes in v2:
- Keep the SDIO host claimed when calling mmc_hw_reset().
- Add a fixes tag.
---
drivers/net/wireless/ti/wlcore/sdio.c | 15 +++
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ti/wlcore/sdio.c
b/drivers/net/wireless/ti/wlcore/sdio.c
index bd10165d7eec..4d4b07701149 100644
--- a/drivers/net/wireless/ti/wlcore/sdio.c
+++ b/drivers/net/wireless/ti/wlcore/sdio.c
@@ -164,6 +164,12 @@ static int wl12xx_sdio_power_on(struct wl12xx_sdio_glue
*glue)
}
sdio_claim_host(func);
+ /*
+ * To guarantee that the SDIO card is power cycled, as required to make
+ * the FW programming to succeed, let's do a brute force HW reset.
+ */
+ mmc_hw_reset(card->host);
+
sdio_enable_func(func);
sdio_release_host(func);
@@ -174,20 +180,13 @@ static int wl12xx_sdio_power_off(struct wl12xx_sdio_glue
*glue)
{
struct sdio_func *func = dev_to_sdio_func(glue->dev);
struct mmc_card *card = func->card;
- int error;
sdio_claim_host(func);
sdio_disable_func(func);
sdio_release_host(func);
/* Let runtime PM know the card is powered off */
- error = pm_runtime_put(&card->dev);
- if (error < 0 && error != -EBUSY) {
- dev_err(&card->dev, "%s failed: %i\n", __func__, error);
-
- return error;
- }
-
+ pm_runtime_put(&card->dev);
return 0;
}
Just tested on both HiKey (620) and Ultra96 but it fails to fix the issue on
both. I'm getting
wl1271_sdio: probe of mmc2:0001:1 failed with error -16
during boot again, and the interface is not available.
Okay, sounds like this may be a different problem then. Can you share
the complete log and the kernel config?
You can find the config here [1], log from the HiKey boot attached.
I can prepare a debug patch as well, if you are willing to re-run the test?
Sure, send it over, I can run it.
Alright, sounds great. However, I need to defer that to Monday/Tuesday
next week.
Adding a post-power-on-delay-ms of 1 ms as you suggested [1], doesn't
sounds like the correct solution to me, unless I am overlooking some
things. The point is, since the mmc core succeeds to detect and
initialize the SDIO card, the power sequence seems to be correct.
Yeah, I'm not claiming at all I know what I'm doing there, just that it happens
to work.
I see. Good to know, thanks!
Jan
[1]
https://github.com/siemens/jailhouse-images/blob/next/recipes-kernel/linux/files/arm64_defconfig_4.19
I have looked through the log and the defconfig. No obvious things
found at this point. Thanks for sharing them!
So, I have put together a debug patch, mostly to verify that things
seems to be correct in regards to runtime PM. It should produce some
prints to the log, particular during power on/off of the SDIO card and
during probe of the wifi driver. Please re-run the test on top of the
v2 version of the $subject patch.
Log attached.
Jan
[0.00] Booting Linux on physical CPU 0x00 [0x410fd033]
[0.00] Linux version 4.19.16 (builder@870f50503134) (gcc version 6.3.0 20170516 (Debian 6.3.0-18)) #1 SMP PREEMPT Tue Jan 22 15:50:59 UTC 2019
[0.00] Machine model: HiKey Development Board
[0.00] Memory limited to 1820MB
[0.00] efi: Getting EFI parameters from FDT:
[0.00] efi: EFI v2.60 by EDK II
[0.00] efi: MEMATTR=0x3cd48a98
[0.00] Reserved memory: created CMA memory pool at 0x6bc0, size 128 MiB
[0.00] OF: reserved mem: in
Re: [PATCH] regmap: no need to check return value of debugfs_create functions
On Tue, Jan 22, 2019 at 03:56:06PM +, Mark Brown wrote: > On Tue, Jan 22, 2019 at 04:21:06PM +0100, Greg Kroah-Hartman wrote: > > When calling debugfs functions, there is no need to ever check the > > return value. The function can work or not, but the code logic should > > never do something different based on this. > > Given that all the rest of the function is doing is further debugfs > operations and when it fails people trying to use the debugfs do welcome > some diagnostics I'm not sure that's particularly helpful. The only way it will fail is if we are out of memory. And you are in a bigger mess then, no one cares about debugfs calls, just make them and move on, you should never care about the result of such a call. thanks, greg k-h
[BUG] "access dev->iommu_fwspec" cause crash on BPI-R2
Hi, the following Patch breaks hdmi (at least) on Bananapi R2 (mt7623): a9bf2eec5a6fc01a0a5250eaf0bf61dfd382a78a "iommu/mediatek: Use helper functions to access dev->iommu_fwspec" https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/drivers/iommu/mtk_iommu_v1.c?id=a9bf2eec5a6fc01a0a5250eaf0bf61dfd382a78a [5.363509] Backtrace: [5.365946] [] (mtk_iommu_domain_free) from [] (iommu_domain_free+0x20/) [5.374670] r4:decd57a0 [5.377192] [] (iommu_domain_free) from [] (release_iommu_mapping+0x24/) [5.385922] [] (release_iommu_mapping) from [] (arm_iommu_release_mappi) [5.395943] r7: r6:decd5780 r5: r4:decd57a0 [5.401567] [] (arm_iommu_release_mapping.part.0) from [] (arch_setup_d) [5.411412] r5: r4:dead1410 [5.414968] [] (arch_setup_dma_ops) from [] (of_dma_configure+0x27c/0x3) [5.423521] r6:c0b58e20 r5: r4: [5.428109] [] (of_dma_configure) from [] (platform_dma_configure+0x28/) [5.436838] r10:c107efdc r9: r8:c10c0008 r7: r6:c1117b34 r5:dead1410 [5.444612] r4:c1117b30 [5.447131] [] (platform_dma_configure) from [] (really_probe+0xc4/0x42) [5.455602] [] (really_probe) from [] (driver_probe_device+0x88/0x1e0) [5.463814] r10: r9:c060a25c r8: r7:c1008c48 r6:c107efdc r5:c107efdc [5.471588] r4:dead1410 [5.474107] [] (driver_probe_device) from [] (__driver_attach+0x134/0x1) [5.482663] r9:c060a25c r8: r7:c1008c48 r6:c107efdc r5:dead1444 r4:dead1410 [5.490358] [] (__driver_attach) from [] (bus_for_each_dev+0x84/0xc4) [5.498480] r7:c1008c48 r6:c06080c8 r5:c107efdc r4: [5.504102] [] (bus_for_each_dev) from [] (driver_attach+0x2c/0x30) [5.512052] r7:c10bff30 r6:c107fad8 r5:decde780 r4:c107efdc [5.517675] [] (driver_attach) from [] (bus_add_driver+0x1d0/0x274) [5.525629] [] (bus_add_driver) from [] (driver_register+0x84/0x118) [5.533666] r8:c060a20c r7:c0609c60 r6:c10c0230 r5:0002 r4:c107efdc [5.540323] [] (driver_register) from [] (__platform_register_drivers+0) after reverting it i can boot without crash and start x-server my repo just for reference (revert not pushed yet): https://github.com/frank-w/BPI-R2-4.14/tree/5.0-hdmi i hope i had chosen the right way to report this... regards Frank
[PATCH 0/2] Fix crash in cper_estatus_check()
I recently encountered a crash in cper_estatus_check() when called by bert_init(). Patches follow to fix the problem. Note that I cannot fully test the patches since the hardware error record on that machine has been cleared. The crash log: [ 125.666350] BUG: unable to handle kernel paging request at c9004046d02c [ 125.666503] PGD 1f6dce067 P4D 1f6dce067 PUD 1e6532067 PMD 1e3d11067 PTE 0 [ 125.96] Oops: [#1] SMP KASAN NOPTI [ 125.666837] CPU: 7 PID: 1 Comm: swapper/0 Not tainted 4.19.0+0 #1 [ 125.666983] Hardware name: Dell Inc. PowerEdge M520/0DW6GX, BIOS 1.8.6 08/30/2013 [ 125.667171] RIP: e030:cper_estatus_check+0x7e/0xf0 [ 125.667315] Code: 41 29 c5 48 98 48 01 c3 48 89 d8 4c 29 e0 48 39 e8 7d 4a 48 8d 7b 18 be 04 00 00 00 e8 bb 6f 9f ff 48 8d 7b 14 be 02 00 00 00 <44> 8b 73 18 e8 a9 6f 9f ff 0f b6 4b 15 44 89 ee 66 83 f9 03 19 d2 [ 125.667554] RSP: e02b:8881e65efce0 EFLAGS: 00010246 [ 125.667699] RAX: f5200808da06 RBX: c9004046d014 RCX: 8192bf25 [ 125.667849] RDX: RSI: 0002 RDI: c9004046d028 [ 125.668009] RBP: 0700 R08: f5200808da06 R09: f5200808da06 [ 125.668207] R10: 0001 R11: f5200808da05 R12: c9004046cc14 [ 125.668358] R13: 0300 R14: 00c0 R15: c9004046cc00 [ 125.668519] FS: () GS:8881e77c() knlGS: [ 125.668698] CS: e033 DS: ES: CR0: 80050033 [ 125.668844] CR2: c9004046d02c CR3: 0260c000 CR4: 00042660 [ 125.668999] Call Trace: [ 125.669139] bert_init+0x21c/0x362 [ 125.669279] ? setup_bert_disable+0x12/0x12 [ 125.669420] ? pci_get_dev_by_id+0x57/0x70 [ 125.669560] ? pci_get_device+0x86/0xc0 [ 125.669738] ? pci_create_sysfs_dev_files+0x1a6/0x330 [ 125.669883] ? setup_bert_disable+0x12/0x12 [ 125.670026] ? set_debug_rodata+0x11/0x11 [ 125.670166] ? do_one_initcall+0x8b/0x253 [ 125.670306] do_one_initcall+0x8b/0x253 [ 125.670447] ? perf_trace_initcall_level+0x250/0x250 [ 125.670592] ? __wake_up_common+0x140/0x1d0 [ 125.670736] ? kasan_unpoison_shadow+0x30/0x40 [ 125.670879] ? kasan_unpoison_shadow+0x30/0x40 [ 125.671023] ? set_debug_rodata+0x11/0x11 [ 125.671164] kernel_init_freeable+0x269/0x304 [ 125.671346] ? rest_init+0xc0/0xc0 [ 125.671485] kernel_init+0xf/0x130 [ 125.671623] ? rest_init+0xc0/0xc0 [ 125.671761] ? rest_init+0xc0/0xc0 [ 125.671901] ret_from_fork+0x35/0x40 [ 125.672063] Modules linked in: [ 125.672201] CR2: c9004046d02c [ 125.672349] ---[ end trace a17cd87742b2c49e ]--- [ 125.683693] RIP: e030:cper_estatus_check+0x7e/0xf0 [ 125.683840] Code: 41 29 c5 48 98 48 01 c3 48 89 d8 4c 29 e0 48 39 e8 7d 4a 48 8d 7b 18 be 04 00 00 00 e8 bb 6f 9f ff 48 8d 7b 14 be 02 00 00 00 <44> 8b 73 18 e8 a9 6f 9f ff 0f b6 4b 15 44 89 ee 66 83 f9 03 19 d2 [ 125.684103] RSP: e02b:8881e65efce0 EFLAGS: 00010246 [ 125.684247] RAX: f5200808da06 RBX: c9004046d014 RCX: 8192bf25 [ 125.684397] RDX: RSI: 0002 RDI: c9004046d028 [ 125.684548] RBP: 0700 R08: f5200808da06 R09: f5200808da06 [ 125.684699] R10: 0001 R11: f5200808da05 R12: c9004046cc14 [ 125.684850] R13: 0300 R14: 00c0 R15: c9004046cc00 [ 125.685009] FS: () GS:8881e77c() knlGS: [ 125.685224] CS: e033 DS: ES: CR0: 80050033 [ 125.685371] CR2: c9004046d02c CR3: 0260c000 CR4: 00042660 [ 125.685566] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0009 Thanks, Ross Lagerwall (2): acpi/apei: Avoid possible OOB when accessing BERT region efi/cper: Avoid possible OOB when checking generic data block drivers/acpi/apei/bert.c| 23 ++- drivers/firmware/efi/cper.c | 10 ++ 2 files changed, 16 insertions(+), 17 deletions(-) -- 2.17.2
[PATCH 1/2] acpi/apei: Avoid possible OOB when accessing BERT region
Check that the length recorded in the generic error status block is
within the region before checking the contents of the region itself.
Otherwise it may result in an OOB access if the system firmware has
generated a status block with an invalid length (larger than the mapped
region). Also move the block_status check so that it only happens after
the block has been verified to be within the mapped region.
Signed-off-by: Ross Lagerwall
---
drivers/acpi/apei/bert.c | 23 ++-
1 file changed, 10 insertions(+), 13 deletions(-)
diff --git a/drivers/acpi/apei/bert.c b/drivers/acpi/apei/bert.c
index 12771fcf0417..0d948d0a41af 100644
--- a/drivers/acpi/apei/bert.c
+++ b/drivers/acpi/apei/bert.c
@@ -42,15 +42,7 @@ static void __init bert_print_all(struct acpi_bert_region
*region,
int remain = region_len;
u32 estatus_len;
- if (!estatus->block_status)
- return;
-
- while (remain > sizeof(struct acpi_bert_region)) {
- if (cper_estatus_check(estatus)) {
- pr_err(FW_BUG "Invalid error record.\n");
- return;
- }
-
+ while (remain >= sizeof(struct acpi_bert_region)) {
estatus_len = cper_estatus_len(estatus);
if (remain < estatus_len) {
pr_err(FW_BUG "Truncated status block (length: %u).\n",
@@ -58,6 +50,15 @@ static void __init bert_print_all(struct acpi_bert_region
*region,
return;
}
+ /* No more error records. */
+ if (!estatus->block_status)
+ return;
+
+ if (cper_estatus_check(estatus)) {
+ pr_err(FW_BUG "Invalid error record.\n");
+ return;
+ }
+
pr_info_once("Error records from previous boot:\n");
cper_estatus_print(KERN_INFO HW_ERR, estatus);
@@ -70,10 +71,6 @@ static void __init bert_print_all(struct acpi_bert_region
*region,
estatus->block_status = 0;
estatus = (void *)estatus + estatus_len;
- /* No more error records. */
- if (!estatus->block_status)
- return;
-
remain -= estatus_len;
}
}
--
2.17.2
[PATCH 2/2] efi/cper: Avoid possible OOB when checking generic data block
When checking a generic status block, we iterate over all the generic
data blocks. The loop condition only checks that the start of the
generic data block is valid (within estatus->data_length) but not the
whole block. Because the size of data blocks (excluding error data) may
vary depending on the revision and the revision is contained within the
data block, ensure that enough of the current data block is valid before
dereferencing any members otherwise an OOB access may occur if
estatus->data_length is invalid. This relies on the fact that
struct acpi_hest_generic_data_v300 is a superset of the earlier version.
Also rework the other checks to avoid potential underflow.
Signed-off-by: Ross Lagerwall
---
drivers/firmware/efi/cper.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/firmware/efi/cper.c b/drivers/firmware/efi/cper.c
index a7902fccdcfa..7cc18874b9d0 100644
--- a/drivers/firmware/efi/cper.c
+++ b/drivers/firmware/efi/cper.c
@@ -546,7 +546,7 @@ EXPORT_SYMBOL_GPL(cper_estatus_check_header);
int cper_estatus_check(const struct acpi_hest_generic_status *estatus)
{
struct acpi_hest_generic_data *gdata;
- unsigned int data_len, gedata_len;
+ unsigned int data_len, record_len;
int rc;
rc = cper_estatus_check_header(estatus);
@@ -555,10 +555,12 @@ int cper_estatus_check(const struct
acpi_hest_generic_status *estatus)
data_len = estatus->data_length;
apei_estatus_for_each_section(estatus, gdata) {
- gedata_len = acpi_hest_get_error_length(gdata);
- if (gedata_len > data_len - acpi_hest_get_size(gdata))
+ if (sizeof(struct acpi_hest_generic_data) > data_len)
return -EINVAL;
- data_len -= acpi_hest_get_record_size(gdata);
+ record_len = acpi_hest_get_record_size(gdata);
+ if (record_len > data_len)
+ return -EINVAL;
+ data_len -= record_len;
}
if (data_len)
return -EINVAL;
--
2.17.2
Re: [RFC] irq-gic-v3-its: fix occasional VLPI drop
On Tue, 22 Jan 2019 12:44:18 +,
Heyi Guo wrote:
>
> Hi Marc,
>
> Thanks for your feedback. Please see my comments below.
>
>
> On 2019/1/22 17:53, Marc Zyngier wrote:
> > Hi Heyi,
> >
> > On Mon, 21 Jan 2019 11:51:48 +,
> > Heyi Guo wrote:
> >> Every VLPI will temporarily be mapped to the first CPU in system
> >> (normally CPU0) and then moved to the real scheduled CPU later. There
> >> is a time window so a VLPI may be sent to CPU0 instead of the real
> >> scheduled vCPU, in a multi-CPU virtual machine. However, CPU0 may have
> >> not been scheduled as a virtual CPU after system boots up, so the
> >> value of GICR_VPROPBASER still be the reset value. According to GIC
> >> spec, the reset value of IDbits in GICR_VPROPBASER is architecturally
> >> UNKNOWN, and the GIC will behave as if all virtual LPIs are out of
> >> range if it is less than 0b1101. On our platform the GICR will simply
> >> drop the incoming VLPI, which results in interrupt missing in Guest.
> > OK, it took me some time to page this horror back in. So let's see if
> > I can sum-up the issue correctly:
>
> Sorry for not explaining the whole thing clearly...
>
No worries, that's mostly for me to make sure I understood the issue
correctly. I've paged out the GICv4 driver a while ago, and it takes
some effort to reload it. ;-)
> >
> > - When a VM gets created, all the vPEs are mapped to CPU0's
> >redistributor.
>
> Not exactly on VM geting created, but when the passthru PCI device
> driver in Guest tries to enable MSI interrupts. The specific code is
> in its_map_vm().
What I meant is that as far as the GIC is concerned, that's equivalent
to a VM creation. its_map_vm() is what makes the GIC aware of it.
> >
> > - If a device starts emitting VLPIs targeting a vPE that has not run
> >yet, these VLPIs are forwarded to CPU0's redistributor.
> >
> > - If CPU0 has itself never run any vPE, its GICR_PROPBASER is not
> >initialised, meaning that the IDbits field may contain a value that
> >makes the redistributor drop the interrupt on the floor.
> Yes.
> >
> > Is that a correct assessment of the issue you're seeing? If so, I
> > think you have a very good point here, and this looks like a hole in
> > the driver.
> >
> > Comments below:
> >
> >> As no code will clear GICR_VPROPBASER at runtime, we can safely
> >> initialize the IDbits field at boot time for each CPU to get rid of
> >> this issue.
> >>
> >> Signed-off-by: Heyi Guo
> >> Signed-off-by: Heyi Guo
> >> ---
> >> drivers/irqchip/irq-gic-v3-its.c | 14 ++
> >> 1 file changed, 14 insertions(+)
> >>
> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c
> >> b/drivers/irqchip/irq-gic-v3-its.c
> >> index db20e99..6116215 100644
> >> --- a/drivers/irqchip/irq-gic-v3-its.c
> >> +++ b/drivers/irqchip/irq-gic-v3-its.c
> >> @@ -2144,6 +2144,20 @@ static void its_cpu_init_lpis(void)
> >>val |= GICR_CTLR_ENABLE_LPIS;
> >>writel_relaxed(val, rbase + GICR_CTLR);
> >> +/*
> >> + * Temporary workaround for vlpi drop on Hi1620.
> > Why is this specific to this implementation? Isn't this an issue that
> > affects every GICv4 implementations?
> This was an internal patch and I forgot to modify the comment before
> sending out, either not 100% sure that it is the common behavior of
> GICv4 to drop VLPI if IDbits is not correctly configured. I can
> change it in V2.
Dropping VLPIs that are out of range is the expected behaviour.
>
> >
> >> + * IDbits must be set before any VLPI is sent to this CPU, or else the
> >> + * VLPI will be considered as out of range and dropped.
> >> + */
> >> + if (gic_rdists->has_vlpis) {
> >> + void __iomem *vlpi_base = gic_data_rdist_vlpi_base();
> >> +
> >> + val = (LPI_NRBITS - 1) & GICR_VPROPBASER_IDBITS_MASK;
> >> + pr_info("GICv4: CPU%d: Init IDbits to 0x%llx for
> >> GICR_VPROPBASER\n",
> >> + smp_processor_id(), val);
> > I don't think this pr_info is useful to a normal user, as it is only
> > debug information. I'm actually minded to demote a bunch of the GICv3
> > prints to pr_debug.
> OK.
> >> + gits_write_vpropbaser(val, vlpi_base + GICR_VPROPBASER);
> >> + }
> >> +
> > I think we need to clear GICR_VPENDBASER.Valid too (you can probably
> > reuse part of its_vpe_deschedule for that), so that we don't get into
> > a bizarre situation where CPU0's redistributor has some ancient
> > programming left in, and could start corrupting memory.
> I can do that for safety. But is it possible of corrupting memory?
> Even if GICR_VPENDBASER.Valid==1, I don't think it is possible that
> GICR_VPENDBASER.Physical_Address equals to VPT_addr, so kernel
> should consider vPE is not sheculed on CPU0 and only memory pointed
> by VPT_addr will be modified. Please let me know if I'm wrong :)
Well, we must still make sure that no VLPI will be propagated to
memory before KVM actually run a vcpu on this CPU. And yes, this is
for safety, we'd be very unl
Re: [for next][PATCH] iwlwifi: Fix unmet dependency error for IWLWIFI_LEDS
On 1/22/2019 5:15 AM, Luciano Coelho wrote: On Mon, 2019-01-21 at 23:31 +, Sinan Kaya wrote: There is an unresolved dependency as follows: IWLWIFI_LEDS selects MAC80211_LEDS. MAC80211_LEDS depends on MAC80211. It is possible to choose MAC80211_LEDS (y) but not choose MAC80211 (n) WARNING: unmet direct dependencies detected for MAC80211_LEDS Depends on [n]: NET [=y] && WIRELESS [=y] && MAC80211 [=n] && LEDS_CLASS [=y] Selected by [y]: - IWLWIFI_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] && WLAN_VENDOR_INTEL [=y] && IWLWIFI [=y] && (LEDS_CLASS [=y]=y || LEDS_CLASS [=y]=IWLWIFI [=y]) Move the MAC80211 dependency into IWLWIFI_LEDS so that we avoid this configuration. Signed-off-by: Sinan Kaya --- Thanks for your patch! But we already have another patch to fix this issued queued for 5.0-rc4 (it's currently in wireless-drivers.git): https://patchwork.kernel.org/patch/10762079/ Is it possible to queue this up soon? There is an effort to clean up linux-next against randconfig failures and this issue showed up there. -- Cheers, Luca.
Re: [PATCH 05/22] x86/fpu: Remove fpu->initialized usage in copy_fpstate_to_sigframe()
On 01/22, Borislav Petkov wrote: > > On Mon, Jan 21, 2019 at 12:21:17PM +0100, Oleg Nesterov wrote: > > And in any case I do not understand the idea to use the second > > in-kernel struct fpu. A signal handler can be interrupted by another > > signal, this will need to save/restore the FPU state again. > > Well, we were just speculating whether doing that would simplify the > code around get_sigframe() et al. But if that is an ABI, then we can't > really touch it. > > Btw, where is that whole ABI deal about saving FPU regs on the user > signal stack documented? I don't know... tried to google, found nothing. the comment in /usr/include/sys/ucontext.h mentions SysV/i386 ABI + historical reasons, this didn't help. Oleg.
Re: [for next][PATCH] iwlwifi: Fix unmet dependency error for IWLWIFI_LEDS
Sinan Kaya writes: > On 1/22/2019 5:15 AM, Luciano Coelho wrote: >> On Mon, 2019-01-21 at 23:31 +, Sinan Kaya wrote: >>> There is an unresolved dependency as follows: >>> >>> IWLWIFI_LEDS selects MAC80211_LEDS. >>> MAC80211_LEDS depends on MAC80211. >>> >>> It is possible to choose MAC80211_LEDS (y) but not choose MAC80211 >>> (n) >>> >>> WARNING: unmet direct dependencies detected for MAC80211_LEDS >>>Depends on [n]: NET [=y] && WIRELESS [=y] && MAC80211 [=n] && >>> LEDS_CLASS [=y] >>>Selected by [y]: >>>- IWLWIFI_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] && >>> WLAN_VENDOR_INTEL [=y] && IWLWIFI [=y] && (LEDS_CLASS [=y]=y || >>> LEDS_CLASS [=y]=IWLWIFI [=y]) >>> >>> Move the MAC80211 dependency into IWLWIFI_LEDS so that we avoid this >>> configuration. >>> >>> Signed-off-by: Sinan Kaya >>> --- >> >> Thanks for your patch! But we already have another patch to fix this >> issued queued for 5.0-rc4 (it's currently in wireless-drivers.git): >> >> https://patchwork.kernel.org/patch/10762079/ > > Is it possible to queue this up soon? There is an effort to clean up > linux-next against randconfig failures and this issue showed up there. It already should be in linux-next: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git/commit/?id=ec5aecc0b227f5509d25853537f989ca303e2be1 But it's not in Linus' tree, yet. -- Kalle Valo
Re: Plain accesses and data races in the Linux Kernel Memory Model
On Tue, 22 Jan 2019, Andrea Parri wrote: > > @@ -131,7 +159,7 @@ let rec rcu-fence = rcu-gp | srcu-gp | > > (rcu-fence ; rcu-link ; rcu-fence) > > > > (* rb orders instructions just as pb does *) > > -let rb = prop ; po ; rcu-fence ; po? ; hb* ; pb* > > +let rb = prop ; po ; rcu-fence ; po? ; hb* ; pb* ; [marked] > > Testing has revealed some subtle semantics changes for some RCU tests > _without_ unmarked memory accesses; an example is reported at the end > of this email. I suspect that the improvements you mentioned in this > thread can restore the original semantics but I'm reporting this here > for further reference. > > With the above definition of 'rb', we're losing links which originate > or target RCU fences, so that this definition is in fact a relaxation > w.r.t. the current semantics (even when limiting to marked accesses). > The test below, for example, is currently forbidden by the LKMM, but > it becomes allowed with this patch. > > FWIW, I checked that including the RCU fences in 'marked' can restore > the original semantics of these tests; I'm still not sure whether this > change can make sense though > > Thoughts? Ah, a very good discovery. I think changing marked to ~plain in a few places would be a better solution. Or maybe allowing plain accesses in those places will also be okay -- it's hard to judge at this point. > Oh, one last (and unrelated) nit before I forget: IIUC, we used to > upper-case set names, so I'd also suggest s/marked/Marked, s/plain/Plain > and similarly for the other sets to be introduced. Okay, I'll follow that convention. Alan
Re: [for next][PATCH] iwlwifi: Fix unmet dependency error for IWLWIFI_LEDS
On 1/22/2019 11:17 AM, Kalle Valo wrote: Sinan Kaya writes: On 1/22/2019 5:15 AM, Luciano Coelho wrote: On Mon, 2019-01-21 at 23:31 +, Sinan Kaya wrote: There is an unresolved dependency as follows: IWLWIFI_LEDS selects MAC80211_LEDS. MAC80211_LEDS depends on MAC80211. It is possible to choose MAC80211_LEDS (y) but not choose MAC80211 (n) WARNING: unmet direct dependencies detected for MAC80211_LEDS Depends on [n]: NET [=y] && WIRELESS [=y] && MAC80211 [=n] && LEDS_CLASS [=y] Selected by [y]: - IWLWIFI_LEDS [=y] && NETDEVICES [=y] && WLAN [=y] && WLAN_VENDOR_INTEL [=y] && IWLWIFI [=y] && (LEDS_CLASS [=y]=y || LEDS_CLASS [=y]=IWLWIFI [=y]) Move the MAC80211 dependency into IWLWIFI_LEDS so that we avoid this configuration. Signed-off-by: Sinan Kaya --- Thanks for your patch! But we already have another patch to fix this issued queued for 5.0-rc4 (it's currently in wireless-drivers.git): https://patchwork.kernel.org/patch/10762079/ Is it possible to queue this up soon? There is an effort to clean up linux-next against randconfig failures and this issue showed up there. It already should be in linux-next: https://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git/commit/?id=ec5aecc0b227f5509d25853537f989ca303e2be1 But it's not in Linus' tree, yet. Thanks, let me grab a recent linux-next tag.
Re: [RFC PATCH v1 05/13] mfd: bd70528: Support ROHM bd70528 PMIC - core
Hello Guenter,
Thanks for taking the time and doing review!
On Tue, Jan 22, 2019 at 06:51:26AM -0800, Guenter Roeck wrote:
> On 1/22/19 1:44 AM, Matti Vaittinen wrote:
> > +static DEFINE_MUTEX(rtc_timer_mutex);
// snip
> > +static int bd70528_i2c_probe(struct i2c_client *i2c,
> > + const struct i2c_device_id *id)
> > +{
> > + struct bd70528 *bd70528;
> > + struct regmap_irq_chip_data *irq_data;
> > + int ret, i;
> > +
> > + if (!i2c->irq) {
> > + dev_err(&i2c->dev, "No IRQ configured\n");
> > + return -EINVAL;
> > + }
> > + bd70528 = devm_kzalloc(&i2c->dev, sizeof(*bd70528), GFP_KERNEL);
> > +
> > + if (!bd70528)
> > + return -ENOMEM;
> > +
> > + dev_set_drvdata(&i2c->dev, bd70528);
> > + bd70528->rtc_timer_lock = &rtc_timer_mutex;
>
> One global mutex for all instances of this driver is odd.
> Why isn't this just part of struct bd70528 ?
>
You are right. This was a brainfart from my side. I don't think there
is many cases where this would har though. I don't expect to see
many instances of PMIC drivers load. But you are correct nevertheless.
I will fix this in future version. Thanks for pointing it out!
Br,
Matti Vaittinen
--
Matti Vaittinen
ROHM Semiconductors
~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
Re: linux-next: Tree for Jan 22 (drivers/reset/reset-brcmstb.c)
On 1/21/19 8:36 PM, Stephen Rothwell wrote: > Hi all, > > Changes since 20190121: > on i386: ld: drivers/reset/reset-brcmstb.o: in function `brcmstb_reset_probe': reset-brcmstb.c:(.text+0xd7): undefined reference to `__umoddi3' ld: reset-brcmstb.c:(.text+0x120): undefined reference to `__udivdi3' Full randconfig file is attached. -- ~Randy # # Automatically generated file; DO NOT EDIT. # Linux/i386 5.0.0-rc3 Kernel Configuration # # # Compiler: gcc (SUSE Linux) 4.8.5 # CONFIG_CC_IS_GCC=y CONFIG_GCC_VERSION=40805 CONFIG_CLANG_VERSION=0 CONFIG_CC_HAS_ASM_GOTO=y CONFIG_CONSTRUCTORS=y CONFIG_IRQ_WORK=y CONFIG_BUILDTIME_EXTABLE_SORT=y CONFIG_THREAD_INFO_IN_TASK=y # # General setup # CONFIG_BROKEN_ON_SMP=y CONFIG_INIT_ENV_ARG_LIMIT=32 CONFIG_COMPILE_TEST=y CONFIG_LOCALVERSION="" CONFIG_BUILD_SALT="" CONFIG_HAVE_KERNEL_GZIP=y CONFIG_HAVE_KERNEL_BZIP2=y CONFIG_HAVE_KERNEL_LZMA=y CONFIG_HAVE_KERNEL_XZ=y CONFIG_HAVE_KERNEL_LZO=y CONFIG_HAVE_KERNEL_LZ4=y # CONFIG_KERNEL_GZIP is not set CONFIG_KERNEL_BZIP2=y # CONFIG_KERNEL_LZMA is not set # CONFIG_KERNEL_XZ is not set # CONFIG_KERNEL_LZO is not set # CONFIG_KERNEL_LZ4 is not set CONFIG_DEFAULT_HOSTNAME="(none)" CONFIG_SWAP=y # CONFIG_SYSVIPC is not set # CONFIG_POSIX_MQUEUE is not set # CONFIG_CROSS_MEMORY_ATTACH is not set CONFIG_USELIB=y CONFIG_AUDIT=y CONFIG_HAVE_ARCH_AUDITSYSCALL=y CONFIG_AUDITSYSCALL=y # # IRQ subsystem # CONFIG_GENERIC_IRQ_PROBE=y CONFIG_GENERIC_IRQ_SHOW=y CONFIG_GENERIC_IRQ_CHIP=y CONFIG_IRQ_DOMAIN=y CONFIG_IRQ_SIM=y CONFIG_IRQ_DOMAIN_HIERARCHY=y CONFIG_GENERIC_IRQ_MATRIX_ALLOCATOR=y CONFIG_GENERIC_IRQ_RESERVATION_MODE=y CONFIG_IRQ_FORCED_THREADING=y CONFIG_SPARSE_IRQ=y # CONFIG_GENERIC_IRQ_DEBUGFS is not set CONFIG_CLOCKSOURCE_WATCHDOG=y CONFIG_ARCH_CLOCKSOURCE_DATA=y CONFIG_ARCH_CLOCKSOURCE_INIT=y CONFIG_CLOCKSOURCE_VALIDATE_LAST_CYCLE=y CONFIG_GENERIC_TIME_VSYSCALL=y CONFIG_GENERIC_CLOCKEVENTS=y CONFIG_GENERIC_CLOCKEVENTS_BROADCAST=y CONFIG_GENERIC_CLOCKEVENTS_MIN_ADJUST=y CONFIG_GENERIC_CMOS_UPDATE=y # # Timers subsystem # CONFIG_TICK_ONESHOT=y CONFIG_NO_HZ_COMMON=y # CONFIG_HZ_PERIODIC is not set CONFIG_NO_HZ_IDLE=y CONFIG_NO_HZ=y CONFIG_HIGH_RES_TIMERS=y # CONFIG_PREEMPT_NONE is not set CONFIG_PREEMPT_VOLUNTARY=y # CONFIG_PREEMPT is not set # # CPU/Task time and stats accounting # CONFIG_TICK_CPU_ACCOUNTING=y # CONFIG_IRQ_TIME_ACCOUNTING is not set CONFIG_PSI=y CONFIG_PSI_DEFAULT_DISABLED=y CONFIG_CPU_ISOLATION=y # # RCU Subsystem # CONFIG_TINY_RCU=y CONFIG_RCU_EXPERT=y CONFIG_SRCU=y CONFIG_TINY_SRCU=y CONFIG_TASKS_RCU=y CONFIG_BUILD_BIN2C=y CONFIG_IKCONFIG=y CONFIG_IKCONFIG_PROC=y CONFIG_HAVE_UNSTABLE_SCHED_CLOCK=y CONFIG_ARCH_WANT_BATCHED_UNMAP_TLB_FLUSH=y CONFIG_CGROUPS=y CONFIG_PAGE_COUNTER=y CONFIG_MEMCG=y # CONFIG_MEMCG_SWAP is not set CONFIG_MEMCG_KMEM=y # CONFIG_BLK_CGROUP is not set CONFIG_CGROUP_SCHED=y CONFIG_FAIR_GROUP_SCHED=y CONFIG_CFS_BANDWIDTH=y # CONFIG_RT_GROUP_SCHED is not set # CONFIG_CGROUP_PIDS is not set CONFIG_CGROUP_RDMA=y # CONFIG_CGROUP_FREEZER is not set CONFIG_CGROUP_HUGETLB=y # CONFIG_CGROUP_DEVICE is not set # CONFIG_CGROUP_CPUACCT is not set CONFIG_CGROUP_PERF=y # CONFIG_CGROUP_DEBUG is not set CONFIG_SOCK_CGROUP_DATA=y # CONFIG_CHECKPOINT_RESTORE is not set CONFIG_SCHED_AUTOGROUP=y CONFIG_SYSFS_DEPRECATED=y # CONFIG_SYSFS_DEPRECATED_V2 is not set # CONFIG_RELAY is not set # CONFIG_BLK_DEV_INITRD is not set CONFIG_CC_OPTIMIZE_FOR_PERFORMANCE=y # CONFIG_CC_OPTIMIZE_FOR_SIZE is not set CONFIG_SYSCTL=y CONFIG_ANON_INODES=y CONFIG_HAVE_UID16=y CONFIG_SYSCTL_EXCEPTION_TRACE=y CONFIG_HAVE_PCSPKR_PLATFORM=y CONFIG_BPF=y CONFIG_EXPERT=y # CONFIG_MULTIUSER is not set # CONFIG_SGETMASK_SYSCALL is not set CONFIG_SYSFS_SYSCALL=y # CONFIG_SYSCTL_SYSCALL is not set # CONFIG_FHANDLE is not set # CONFIG_POSIX_TIMERS is not set # CONFIG_PRINTK is not set CONFIG_BUG=y # CONFIG_PCSPKR_PLATFORM is not set CONFIG_BASE_FULL=y CONFIG_FUTEX=y CONFIG_FUTEX_PI=y # CONFIG_EPOLL is not set # CONFIG_SIGNALFD is not set # CONFIG_TIMERFD is not set CONFIG_EVENTFD=y # CONFIG_SHMEM is not set # CONFIG_AIO is not set # CONFIG_ADVISE_SYSCALLS is not set CONFIG_MEMBARRIER=y CONFIG_KALLSYMS=y CONFIG_KALLSYMS_ALL=y CONFIG_KALLSYMS_BASE_RELATIVE=y # CONFIG_BPF_SYSCALL is not set CONFIG_USERFAULTFD=y CONFIG_ARCH_HAS_MEMBARRIER_SYNC_CORE=y CONFIG_RSEQ=y CONFIG_DEBUG_RSEQ=y # CONFIG_EMBEDDED is not set CONFIG_HAVE_PERF_EVENTS=y # CONFIG_PC104 is not set # # Kernel Performance Events And Counters # CONFIG_PERF_EVENTS=y # CONFIG_DEBUG_PERF_USE_VMALLOC is not set # CONFIG_VM_EVENT_COUNTERS is not set # CONFIG_COMPAT_BRK is not set CONFIG_SLAB=y # CONFIG_SLUB is not set # CONFIG_SLOB is not set # CONFIG_SLAB_MERGE_DEFAULT is not set CONFIG_SLAB_FREELIST_RANDOM=y CONFIG_SYSTEM_DATA_VERIFICATION=y CONFIG_PROFILING=y CONFIG_TRACEPOINTS=y CONFIG_GENERIC_CMDLINE=y # CONFIG_CMDLINE_BOOL is not set CONFIG_X86_32=y CONFIG_X86=y CONFIG_INSTRUCTION_DECODER=y CONFIG_OUTPUT_FORMAT="elf32-i386" CONFIG_ARCH_DEFCONFIG="arch/x86/configs/i386_defconfig"
Re: [PATCH 1/8] infiniband: cxgb4: no need to check return value of debugfs_create functions
Hey Greg,
On 1/22/2019 9:17 AM, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic should
> never do something different based on this.
>
> Cc: Steve Wise
> Cc: Doug Ledford
> Cc: Jason Gunthorpe
> Cc: linux-r...@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman
> ---
> drivers/infiniband/hw/cxgb4/device.c | 8 +---
> 1 file changed, 1 insertion(+), 7 deletions(-)
>
> diff --git a/drivers/infiniband/hw/cxgb4/device.c
> b/drivers/infiniband/hw/cxgb4/device.c
> index c13c0ba30f63..9c10fff6dcfb 100644
> --- a/drivers/infiniband/hw/cxgb4/device.c
> +++ b/drivers/infiniband/hw/cxgb4/device.c
> @@ -720,11 +720,8 @@ static const struct file_operations ep_debugfs_fops = {
> .read= debugfs_read,
> };
>
> -static int setup_debugfs(struct c4iw_dev *devp)
> +static void setup_debugfs(struct c4iw_dev *devp)
> {
> - if (!devp->debugfs_root)
> - return -1;
> -
> debugfs_create_file_size("qps", S_IWUSR, devp->debugfs_root,
>(void *)devp, &qp_debugfs_fops, 4096);
>
> @@ -740,7 +737,6 @@ static int setup_debugfs(struct c4iw_dev *devp)
> if (c4iw_wr_log)
> debugfs_create_file_size("wr_log", S_IWUSR, devp->debugfs_root,
>(void *)devp, &wr_log_debugfs_fops,
> 4096);
> - return 0;
> }
>
> void c4iw_release_dev_ucontext(struct c4iw_rdev *rdev,
> @@ -1553,8 +1549,6 @@ static int __init c4iw_init_module(void)
> return err;
>
> c4iw_debugfs_root = debugfs_create_dir(DRV_NAME, NULL);
> - if (!c4iw_debugfs_root)
> - pr_warn("could not create debugfs entry, continuing\n");
>
> reg_workq = create_singlethread_workqueue("Register_iWARP_device");
> if (!reg_workq) {
>
So it is not a problem to call debugfs_create_file_size() when
devp->debugfs_root is NULL?
Acked-by: Steve Wise
Re: [PATCH V2] wlcore: sdio: Fixup power on/off sequence
Ulf Hansson wrote:
> During "wlan-up", we are programming the FW into the WiFi-chip. However,
> re-programming the FW doesn't work, unless a power cycle of the WiFi-chip
> is made in-between the programmings.
>
> To conform to this requirement and to fix the regression in a simple way,
> let's start by allowing that the SDIO card (WiFi-chip) may stay powered on
> (runtime resumed) when wl12xx_sdio_power_off() returns. The intent with the
> current code is to treat this scenario as an error, but unfortunate this
> doesn't work as expected, so let's fix this.
>
> The other part is to guarantee that a power cycle of the SDIO card has been
> completed when wl12xx_sdio_power_on() returns, as to allow the FW
> programming to succeed. However, relying solely on runtime PM to deal with
> this isn't sufficient. For example, userspace may prevent runtime suspend
> via sysfs for the device that represents the SDIO card, leading to that the
> mmc core also keeps it powered on. For this reason, let's instead do a
> brute force power cycle in wl12xx_sdio_power_on().
>
> Fixes: 728a9dc61f13 ("wlcore: sdio: Fix flakey SDIO runtime PM handling")
> Signed-off-by: Ulf Hansson
> Tested-by: Tony Lindgren
> Tested-by: Anders Roxell
> Signed-off-by: Ulf Hansson
Patch applied to wireless-drivers.git, thanks.
13e62626c578 wlcore: sdio: Fixup power on/off sequence
--
https://patchwork.kernel.org/patch/10765781/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v5 2/4] perf: add arm64 smmuv3 pmu driver
On Fri, Nov 30, 2018 at 03:47:49PM +, Shameer Kolothum wrote: > From: Neil Leeder > > Adds a new driver to support the SMMUv3 PMU and add it into the > perf events framework. > > Each SMMU node may have multiple PMUs associated with it, each of > which may support different events. > > SMMUv3 PMCG devices are named as smmuv3_pmcg_ where > is the physical page address of the SMMU PMCG > wrapped to 4K boundary. For example, the PMCG at 0xff8884 is > named smmuv3_pmcg_ff88840 > > Filtering by stream id is done by specifying filtering parameters > with the event. options are: >filter_enable- 0 = no filtering, 1 = filtering enabled >filter_span - 0 = exact match, 1 = pattern match >filter_stream_id - pattern to filter against > > Example: perf stat -e smmuv3_pmcg_ff88840/transaction,filter_enable=1, >filter_span=1,filter_stream_id=0x42/ -a netperf > > Applies filter pattern 0x42 to transaction events, which means events > matching stream ids 0x42 & 0x43 are counted as only upper StreamID > bits are required to match the given filter. Further filtering > information is available in the SMMU documentation. > > SMMU events are not attributable to a CPU, so task mode and sampling > are not supported. > > Signed-off-by: Neil Leeder > Signed-off-by: Shameer Kolothum > --- > drivers/perf/Kconfig | 9 + > drivers/perf/Makefile | 1 + > drivers/perf/arm_smmuv3_pmu.c | 778 > ++ > 3 files changed, 788 insertions(+) > create mode 100644 drivers/perf/arm_smmuv3_pmu.c > > diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig > index 08ebaf7..92be6a3 100644 > --- a/drivers/perf/Kconfig > +++ b/drivers/perf/Kconfig > @@ -52,6 +52,15 @@ config ARM_PMU_ACPI > depends on ARM_PMU && ACPI > def_bool y > > +config ARM_SMMU_V3_PMU > + bool "ARM SMMUv3 Performance Monitors Extension" > + depends on ARM64 && ACPI && ARM_SMMU_V3 > +help > +Provides support for the SMMU version 3 performance monitor unit > (PMU) > +on ARM-based systems. > +Adds the SMMU PMU into the perf events subsystem for > +monitoring SMMU performance events. > + > config ARM_DSU_PMU > tristate "ARM DynamIQ Shared Unit (DSU) PMU" > depends on ARM64 > diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile > index b3902bd..f10a932 100644 > --- a/drivers/perf/Makefile > +++ b/drivers/perf/Makefile > @@ -4,6 +4,7 @@ obj-$(CONFIG_ARM_CCN) += arm-ccn.o > obj-$(CONFIG_ARM_DSU_PMU) += arm_dsu_pmu.o > obj-$(CONFIG_ARM_PMU) += arm_pmu.o arm_pmu_platform.o > obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o > +obj-$(CONFIG_ARM_SMMU_V3_PMU) += arm_smmuv3_pmu.o > obj-$(CONFIG_HISI_PMU) += hisilicon/ > obj-$(CONFIG_QCOM_L2_PMU)+= qcom_l2_pmu.o > obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o > diff --git a/drivers/perf/arm_smmuv3_pmu.c b/drivers/perf/arm_smmuv3_pmu.c > new file mode 100644 > index 000..fb9dcd8 > --- /dev/null > +++ b/drivers/perf/arm_smmuv3_pmu.c > @@ -0,0 +1,778 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* > + * This driver adds support for perf events to use the Performance > + * Monitor Counter Groups (PMCG) associated with an SMMUv3 node > + * to monitor that node. > + * > + * SMMUv3 PMCG devices are named as smmuv3_pmcg_ where > + * is the physical page address of the SMMU PMCG wrapped > + * to 4K boundary. For example, the PMCG at 0xff8884 is named > + * smmuv3_pmcg_ff88840 > + * > + * Filtering by stream id is done by specifying filtering parameters > + * with the event. options are: > + * filter_enable- 0 = no filtering, 1 = filtering enabled > + * filter_span - 0 = exact match, 1 = pattern match > + * filter_stream_id - pattern to filter against > + * > + * To match a partial StreamID where the X most-significant bits must match > + * but the Y least-significant bits might differ, STREAMID is programmed > + * with a value that contains: > + * STREAMID[Y - 1] == 0. > + * STREAMID[Y - 2:0] == 1 (where Y > 1). > + * The remainder of implemented bits of STREAMID (X bits, from bit Y upwards) > + * contain a value to match from the corresponding bits of event StreamID. > + * > + * Example: perf stat -e smmuv3_pmcg_ff88840/transaction,filter_enable=1, > + *filter_span=1,filter_stream_id=0x42/ -a netperf > + * Applies filter pattern 0x42 to transaction events, which means events > + * matching stream ids 0x42 and 0x43 are counted. Further filtering > + * information is available in the SMMU documentation. > + * > + * SMMU events are not attributable to a CPU, so task mode and sampling > + * are not supported. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define SMMU_PMCG_EVCNTR0 0x0 > +#define
Re: [PATCH] backing-dev: no need to check return value of debugfs_create functions
On Tue, Jan 22, 2019 at 05:07:59PM +0100, Sebastian Andrzej Siewior wrote:
> On 2019-01-22 16:21:07 [+0100], Greg Kroah-Hartman wrote:
> > diff --git a/mm/backing-dev.c b/mm/backing-dev.c
> > index 8a8bb8796c6c..85ef344a9c67 100644
> > --- a/mm/backing-dev.c
> > +++ b/mm/backing-dev.c
> > @@ -102,39 +102,25 @@ static int bdi_debug_stats_show(struct seq_file *m,
> > void *v)
> > }
> > DEFINE_SHOW_ATTRIBUTE(bdi_debug_stats);
> >
> > -static int bdi_debug_register(struct backing_dev_info *bdi, const char
> > *name)
> > +static void bdi_debug_register(struct backing_dev_info *bdi, const char
> > *name)
> > {
> > - if (!bdi_debug_root)
> > - return -ENOMEM;
> > -
> > bdi->debug_dir = debugfs_create_dir(name, bdi_debug_root);
>
> If this fails then ->debug_dir is NULL
Wonderful, who cares :)
> > - if (!bdi->debug_dir)
> > - return -ENOMEM;
> > -
> > - bdi->debug_stats = debugfs_create_file("stats", 0444, bdi->debug_dir,
> > - bdi, &bdi_debug_stats_fops);
> > - if (!bdi->debug_stats) {
> > - debugfs_remove(bdi->debug_dir);
> > - bdi->debug_dir = NULL;
> > - return -ENOMEM;
> > - }
> >
> > - return 0;
> > + debugfs_create_file("stats", 0444, bdi->debug_dir, bdi,
> > + &bdi_debug_stats_fops);
>
> then this creates the stats file in the root folder and
True.
> > }
> >
> > static void bdi_debug_unregister(struct backing_dev_info *bdi)
> > {
> > - debugfs_remove(bdi->debug_stats);
> > - debugfs_remove(bdi->debug_dir);
> > + debugfs_remove_recursive(bdi->debug_dir);
>
> this won't remove it.
Which is fine, you don't care.
But step back, how could that original call be NULL? That only happens
if you pass it a bad parent dentry (which you didn't), or the system is
totally out of memory (in which case you don't care as everything else
is on fire).
> If you return for "debug_dir == NULL" then it is a nice cleanup.
No, that's not a valid thing to check for, you should not care as it
will not happen. And if it does happen, it's ok, it's only debugfs, no
one can rely on it, it is only for debugging.
thanks,
greg k-h
Re: [PATCH] s390: kernel: no need to check return value of debugfs_create functions
On Tue, Jan 22, 2019 at 04:21:02PM +0100, Greg Kroah-Hartman wrote:
> When calling debugfs functions, there is no need to ever check the
> return value. The function can work or not, but the code logic should
> never do something different based on this.
>
> Cc: Martin Schwidefsky
> Cc: Heiko Carstens
> Cc: Kees Cook
> Cc: Christian Borntraeger
> Cc: Hendrik Brueckner
> Cc: linux-s...@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman
> ---
> arch/s390/kernel/debug.c| 6 --
> arch/s390/kernel/kdebugfs.c | 2 --
> arch/s390/kernel/sysinfo.c | 2 --
> 3 files changed, 10 deletions(-)
> diff --git a/arch/s390/kernel/sysinfo.c b/arch/s390/kernel/sysinfo.c
> index 12f80d1f0415..2ac3c9b56a13 100644
> --- a/arch/s390/kernel/sysinfo.c
> +++ b/arch/s390/kernel/sysinfo.c
> @@ -545,8 +545,6 @@ static __init int stsi_init_debugfs(void)
> int lvl, i;
>
> stsi_root = debugfs_create_dir("stsi", arch_debugfs_dir);
> - if (IS_ERR_OR_NULL(stsi_root))
> - return 0;
No objections, however will you also change the odd behaviour that
e.g. debugfs_create_file() returns -ENODEV instead of (the expected)
NULL pointer if CONFIG_DEBUGFS is disabled?
I do remember this since it caused at least one crash ;)
19cdd08ba155 ("[S390] qdio: fix broken pointer in case of CONFIG_DEBUG_FS is
disabled").
[PATCH] sched/fair: Call post_init_entity_util_avg() with task_struct pointer argument
Since commit d03266910a53 ("sched/fair: Fix task group initialization")
the utilization of a sched entity representing a task group is no longer
initialized to any other value than 0. So post_init_entity_util_avg() is
only used for tasks.
Make this clear by calling it with a task_struct pointer argument which
also eliminates the entity_is_task(se) if condition in the fork path and
get rid of the stale comment in remove_entity_load_avg() accordingly.
Signed-off-by: Dietmar Eggemann
---
kernel/sched/core.c | 2 +-
kernel/sched/fair.c | 38 --
kernel/sched/sched.h | 2 +-
3 files changed, 18 insertions(+), 24 deletions(-)
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index d4d3514c4fe9..20eab0d96a8d 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -2418,7 +2418,7 @@ void wake_up_new_task(struct task_struct *p)
#endif
rq = __task_rq_lock(p, &rf);
update_rq_clock(rq);
- post_init_entity_util_avg(&p->se);
+ post_init_entity_util_avg(p);
activate_task(rq, p, ENQUEUE_NOCLOCK);
p->on_rq = TASK_ON_RQ_QUEUED;
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index e2ff4b69dcf6..732711ed3cf3 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -744,8 +744,9 @@ static void attach_entity_cfs_rq(struct sched_entity *se);
* Finally, that extrapolated util_avg is clamped to the cap (util_avg_cap)
* if util_avg > util_avg_cap.
*/
-void post_init_entity_util_avg(struct sched_entity *se)
+void post_init_entity_util_avg(struct task_struct *p)
{
+ struct sched_entity *se = &p->se;
struct cfs_rq *cfs_rq = cfs_rq_of(se);
struct sched_avg *sa = &se->avg;
long cpu_scale = arch_scale_cpu_capacity(NULL, cpu_of(rq_of(cfs_rq)));
@@ -763,22 +764,19 @@ void post_init_entity_util_avg(struct sched_entity *se)
}
}
- if (entity_is_task(se)) {
- struct task_struct *p = task_of(se);
- if (p->sched_class != &fair_sched_class) {
- /*
-* For !fair tasks do:
-*
- update_cfs_rq_load_avg(now, cfs_rq);
- attach_entity_load_avg(cfs_rq, se, 0);
- switched_from_fair(rq, p);
-*
-* such that the next switched_to_fair() has the
-* expected state.
-*/
- se->avg.last_update_time = cfs_rq_clock_task(cfs_rq);
- return;
- }
+ if (p->sched_class != &fair_sched_class) {
+ /*
+* For !fair tasks do:
+*
+ update_cfs_rq_load_avg(now, cfs_rq);
+ attach_entity_load_avg(cfs_rq, se, 0);
+ switched_from_fair(rq, p);
+*
+* such that the next switched_to_fair() has the
+* expected state.
+*/
+ se->avg.last_update_time = cfs_rq_clock_task(cfs_rq);
+ return;
}
attach_entity_cfs_rq(se);
@@ -788,7 +786,7 @@ void post_init_entity_util_avg(struct sched_entity *se)
void init_entity_runnable_average(struct sched_entity *se)
{
}
-void post_init_entity_util_avg(struct sched_entity *se)
+void post_init_entity_util_avg(struct task_struct *p)
{
}
static void update_tg_load_avg(struct cfs_rq *cfs_rq, int force)
@@ -3577,10 +3575,6 @@ void remove_entity_load_avg(struct sched_entity *se)
* tasks cannot exit without having gone through wake_up_new_task() ->
* post_init_entity_util_avg() which will have added things to the
* cfs_rq, so we can remove unconditionally.
-*
-* Similarly for groups, they will have passed through
-* post_init_entity_util_avg() before unregister_sched_fair_group()
-* calls this.
*/
sync_entity_load_avg(se);
diff --git a/kernel/sched/sched.h b/kernel/sched/sched.h
index d27c1a5d4e25..ac0a2c0cd33e 100644
--- a/kernel/sched/sched.h
+++ b/kernel/sched/sched.h
@@ -1781,7 +1781,7 @@ extern void init_dl_rq_bw_ratio(struct dl_rq *dl_rq);
unsigned long to_ratio(u64 period, u64 runtime);
extern void init_entity_runnable_average(struct sched_entity *se);
-extern void post_init_entity_util_avg(struct sched_entity *se);
+extern void post_init_entity_util_avg(struct task_struct *p);
#ifdef CONFIG_NO_HZ_FULL
extern bool sched_can_stop_tick(struct rq *rq);
--
2.17.1
Re: [PATCH v2 2/4] mtd: rawnand: Support bad block markers in first, second or last page
On Tue, 22 Jan 2019 11:23:31 +
Schrempf Frieder wrote:
> From: Frieder Schrempf
>
> Currently supported bad block marker positions within the block are:
> * in first page only
> * in last page only
> * in first or second page
>
> Some ESMT NANDs are known to have been shipped by the manufacturer
> with bad block markers in the first or last page, instead of the
> first or second page.
>
> Also the datasheets for Cypress/Spansion/AMD NANDs claim that the
> first, second *and* last page needs to be checked.
>
> Therefore we make it possible to set NAND_BBM_FIRSTPAGE,
> NAND_BBM_SECONDPAGE and NAND_BBM_LASTPAGE independently in any
> combination.
>
> To simplify the code, the logic to evaluate the flags is moved to a
> a new function nand_bbm_get_next_page().
>
> Signed-off-by: Frieder Schrempf
Reviewed-by: Boris Brezillon
> ---
> drivers/mtd/nand/raw/internals.h | 1 +
> drivers/mtd/nand/raw/nand_base.c | 62 ---
> drivers/mtd/nand/raw/nand_bbt.c | 29 +++-
> 3 files changed, 55 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/internals.h
> b/drivers/mtd/nand/raw/internals.h
> index fbf6ca015cd7..97ae67e009d5 100644
> --- a/drivers/mtd/nand/raw/internals.h
> +++ b/drivers/mtd/nand/raw/internals.h
> @@ -76,6 +76,7 @@ extern const struct nand_manufacturer_ops
> toshiba_nand_manuf_ops;
>
> /* Core functions */
> const struct nand_manufacturer *nand_get_manufacturer(u8 id);
> +int nand_bbm_get_next_page(struct nand_chip *chip, int page);
> int nand_markbad_bbm(struct nand_chip *chip, loff_t ofs);
> int nand_erase_nand(struct nand_chip *chip, struct erase_info *instr,
> int allowbbt);
> diff --git a/drivers/mtd/nand/raw/nand_base.c
> b/drivers/mtd/nand/raw/nand_base.c
> index 9ef7b86cdc42..7bc20c1fe23c 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -285,6 +285,31 @@ static void nand_release_device(struct nand_chip *chip)
> spin_unlock(&chip->controller->lock);
> }
>
> +/**
> + * nand_bbm_get_next_page - Get the next page for bad block markers
> + * @chip: NAND chip object
> + * @index: Current page, only pages beyond this will be considered
> + *
> + * Returns an integer that corresponds to the page offset within a block, for
> + * a page that is used to store bad block markers. If no more pages are
> + * available, -1 is returned.
> + */
> +int nand_bbm_get_next_page(struct nand_chip *chip, int page)
> +{
> + struct mtd_info *mtd = nand_to_mtd(chip);
> + int last_page = ((mtd->erasesize - mtd->writesize) >>
> + chip->page_shift) & chip->pagemask;
> +
> + if (page < 0 && chip->options & NAND_BBM_FIRSTPAGE)
> + return 0;
> + else if (page < 1 && chip->options & NAND_BBM_SECONDPAGE)
> + return 1;
> + else if (page < last_page && chip->options & NAND_BBM_LASTPAGE)
> + return last_page;
> +
> + return -1;
> +}
> +
> /**
> * nand_block_bad - [DEFAULT] Read bad block marker from the chip
> * @chip: NAND chip object
> @@ -294,19 +319,14 @@ static void nand_release_device(struct nand_chip *chip)
> */
> static int nand_block_bad(struct nand_chip *chip, loff_t ofs)
> {
> - struct mtd_info *mtd = nand_to_mtd(chip);
> - int page, page_end, res;
> + int page_offset;
> + int res, first_page = (int)(ofs >> chip->page_shift) & chip->pagemask;
> u8 bad;
>
> - if (chip->options & NAND_BBM_LASTPAGE)
> - ofs += mtd->erasesize - mtd->writesize;
> + page_offset = nand_bbm_get_next_page(chip, -1);
>
> - page = (int)(ofs >> chip->page_shift) & chip->pagemask;
> - page_end = page + (((chip->options & NAND_BBM_FIRSTPAGE) &&
> - (chip->options & NAND_BBM_SECONDPAGE)) ? 2 : 1);
> -
> - for (; page < page_end; page++) {
> - res = chip->ecc.read_oob(chip, page);
> + while (page_offset != -1) {
> + res = chip->ecc.read_oob(chip, first_page + page_offset);
> if (res < 0)
> return res;
>
> @@ -318,6 +338,8 @@ static int nand_block_bad(struct nand_chip *chip, loff_t
> ofs)
> res = hweight8(bad) < chip->badblockbits;
> if (res)
> return res;
> +
> + page_offset = nand_bbm_get_next_page(chip, page_offset);
> }
>
> return 0;
> @@ -528,7 +550,7 @@ static int nand_default_block_markbad(struct nand_chip
> *chip, loff_t ofs)
> struct mtd_info *mtd = nand_to_mtd(chip);
> struct mtd_oob_ops ops;
> uint8_t buf[2] = { 0, 0 };
> - int ret = 0, res, i = 0;
> + int ret = 0, res, page_offset;
>
> memset(&ops, 0, sizeof(ops));
> ops.oobbuf = buf;
> @@ -541,18 +563,18 @@ static int nand_default_block_markbad(struct nand_chip
> *chip, loff_t ofs)
> }
> ops.mode = MTD_OPS_PLACE_OOB;
>
> - /* Write to first/last page(s) if ne
Re: [PATCH] mm: no need to check return value of debugfs_create functions
On Tue, Jan 22, 2019 at 05:07:01PM +0100, Michal Hocko wrote:
> On Tue 22-01-19 16:52:55, Greg KH wrote:
> > On Tue, Jan 22, 2019 at 04:31:02PM +0100, Michal Hocko wrote:
> > > On Tue 22-01-19 16:21:13, Greg KH wrote:
> > > [...]
> > > > diff --git a/mm/memblock.c b/mm/memblock.c
> > > > index 022d4cbb3618..18ee657fb918 100644
> > > > --- a/mm/memblock.c
> > > > +++ b/mm/memblock.c
> > > > @@ -1998,8 +1998,7 @@ DEFINE_SHOW_ATTRIBUTE(memblock_debug);
> > > > static int __init memblock_init_debugfs(void)
> > > > {
> > > > struct dentry *root = debugfs_create_dir("memblock", NULL);
> > > > - if (!root)
> > > > - return -ENXIO;
> > > > +
> > > > debugfs_create_file("memory", 0444, root,
> > > > &memblock.memory, &memblock_debug_fops);
> > > > debugfs_create_file("reserved", 0444, root,
> > >
> > > I haven't really read the whole patch but this has just hit my eyes. Is
> > > this a correct behavior?
> > >
> > > Documentations says:
> > > * @parent: a pointer to the parent dentry for this file. This should be
> > > a
> > > * directory dentry if set. If this parameter is NULL, then the
> > > * file will be created in the root of the debugfs filesystem.
> > >
> > > so in case of failure we would get those debugfs files outside of their
> > > intended scope. I believe it is much more correct to simply not create
> > > anything, no?
> >
> > If debugfs_create_dir() returns NULL, then something is really wrong
> > (you passed it an invalid pointer as the parent dentry, or free memory
> > is gone), so there's nothing you can do except keep moving forward and
> > take that result and pass it as any parent pointer you want to. Your
> > code logic should never care if a debugfs file is created or not, it is
> > "fire and forget".
>
> OK, but does it make any sense to continue creating files when you know
> that the parent directory has failed to create? What kind of advantage
> does this have?
It has no advantage or disadvantage. And again, it can't really happen
unless the system is out of memory and in that case, everything else
just crashed anyway.
> > And any result of a debugfs call, like this one, that is to be passed
> > into another debugfs call, will work just fine if the first one failed
> > (the second one usually will also fail, which is fine.)
> >
> > Also, and this is the biggest problem, everyone gets the return value
> > check wrong thinking NULL will be an error, it is one type of error, but
> > other ones are also returned and no one checks them properly. So just
> > don't check at all, that is the design goal here.
>
> sounds like a poor design goal to me but not mine code to maintain so...
The design goal was to make it as simple as possible to use, and that
includes "you do not care about the return value". Now we do have to
return a value because some people need that for when they want to make
a subdirectory, or remove the file later on, otherwise I would have just
had everything be a void return function :)
thanks,
greg k-h
Re: [PATCH v2 3/4] mtd: rawnand: ESMT: Also use the last page for bad block markers
On Tue, 22 Jan 2019 11:23:32 +
Schrempf Frieder wrote:
> From: Frieder Schrempf
>
> It is known that some ESMT SLC NANDs have been shipped
> with the factory bad block markers in the first or last page
> of the block, instead of the first or second page. To be on
> the safe side, let's check all three locations.
>
> Signed-off-by: Frieder Schrempf
Reviewed-by: Boris Brezillon
> ---
> drivers/mtd/nand/raw/nand_esmt.c | 9 -
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_esmt.c
> b/drivers/mtd/nand/raw/nand_esmt.c
> index 99a8092969a7..b37e0b5af5ae 100644
> --- a/drivers/mtd/nand/raw/nand_esmt.c
> +++ b/drivers/mtd/nand/raw/nand_esmt.c
> @@ -36,7 +36,14 @@ static void esmt_nand_decode_id(struct nand_chip *chip)
> static int esmt_nand_init(struct nand_chip *chip)
> {
> if (nand_is_slc(chip))
> - chip->options |= NAND_BBM_FIRSTPAGE | NAND_BBM_SECONDPAGE;
> + /*
> + * It is known that some ESMT SLC NANDs have been shipped
> + * with the factory bad block markers in the first or last page
> + * of the block, instead of the first or second page. To be on
> + * the safe side, let's check all three locations.
> + */
> + chip->options |= NAND_BBM_FIRSTPAGE | NAND_BBM_SECONDPAGE |
> + NAND_BBM_LASTPAGE;
>
> return 0;
> }
Re: [PATCH v2 4/4] mtd: rawnand: AMD: Also use the last page for bad block markers
On Tue, 22 Jan 2019 11:23:33 +
Schrempf Frieder wrote:
> From: Frieder Schrempf
>
> According to the datasheet of some Cypress SLC NANDs, the bad
> block markers can be in the first, second or last page of a block.
> So let's check all three locations.
>
> Signed-off-by: Frieder Schrempf
Reviewed-by: Boris Brezillon
> ---
> drivers/mtd/nand/raw/nand_amd.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_amd.c b/drivers/mtd/nand/raw/nand_amd.c
> index 6202cbf7ee8d..2ffcddff3e27 100644
> --- a/drivers/mtd/nand/raw/nand_amd.c
> +++ b/drivers/mtd/nand/raw/nand_amd.c
> @@ -40,7 +40,13 @@ static void amd_nand_decode_id(struct nand_chip *chip)
> static int amd_nand_init(struct nand_chip *chip)
> {
> if (nand_is_slc(chip))
> - chip->options |= NAND_BBM_FIRSTPAGE | NAND_BBM_SECONDPAGE;
> + /*
> + * According to the datasheet of some Cypress SLC NANDs,
> + * the bad block markers can be in the first, second or last
> + * page of a block. So let's check all three locations.
> + */
> + chip->options |= NAND_BBM_FIRSTPAGE | NAND_BBM_SECONDPAGE |
> + NAND_BBM_LASTPAGE;
>
> return 0;
> }
Re: [RFC PATCH v1 11/13] rtc: bd70528: Initial support for ROHM bd70528 RTC
On Tue, Jan 22, 2019 at 06:48:21AM -0800, Guenter Roeck wrote:
> On 1/22/19 1:47 AM, Matti Vaittinen wrote:
> > +
> > +static int bd70528_set_rtc_based_timers(struct bd70528 *bd70528, int
> > new_state,
> > + int *old_state)
>
> Passed parameter is an int, not int *. I'd be quite surprised if this compiles
> without warning.
>
> > +static int bd70528_re_enable_rtc_based_timers(struct bd70528 *bd70528,
> > + int old_state)
// snip
> > + return bd70528_set_rtc_based_timers(bd70528, old_state, NULL);
and
> > +static int bd70528_disable_rtc_based_timers(struct bd70528 *bd70528,
> > + int *old_state)
// snip
> > + return bd70528_set_rtc_based_timers(bd70528, 0, old_state);
I'm not quite sure I understand what you mean by that. Second parameter is int,
third one is is int *.
> > +static int bd70528_re_enable_rtc_based_timers(struct bd70528 *bd70528,
> > + int old_state)
> > +{
> > + if (bd70528->rtc_timer_lock)
> > + mutex_unlock(bd70528->rtc_timer_lock);
> > +
> Unlock before calling bd70528_set_rtc_based_timers is odd, especially since it
> is called after locking below.
>
Yet another brainfart. Thanks for pointing this out! Will be fixed as
well.
Br,
Matti Vaittinen
--
Matti Vaittinen
ROHM Semiconductors
~~~ "I don't think so," said Rene Descartes. Just then, he vanished ~~~
Re: [PATCH V2] livepatch: fix non-static warnings
On 12/18/18 10:18 AM, Joe Lawrence wrote: On 12/18/2018 03:49 AM, Miroslav Benes wrote: On Mon, 17 Dec 2018, Joe Lawrence wrote: I'm just being picky about its documentation and how we should note its usage in the v3 patch. Think that s/__noclone/used/g of the v2 commit message would be sufficient? We could rephrase it. After all it is not only about symbol names in the symbol table. The traceable/patchable code has to be present... "Sparse reported warnings about non-static symbols. For the variables a simple static attribute is fine - for the functions referenced by livepatch via klp_func the symbol-names must be unmodified in the symbol table and the patchable code has to be emitted. Attach __used attribute to the shared statically declared functions." ? That works for me. Hi Nicholas, Did you still want to post a v3 for this fix? I think there were only a few v3 suggestions (link tag, tag order, __used attribute, and commit msg phrasing.) The context has been clipped in the quoting above, so here's an archive link if you need it: https://lore.kernel.org/lkml/1544965657-26804-1-git-send-email-hof...@osadl.org/ Thanks, -- Joe
Re: [PATCH 1/8] infiniband: cxgb4: no need to check return value of debugfs_create functions
On Tue, Jan 22, 2019 at 10:22:59AM -0600, Steve Wise wrote:
> Hey Greg,
>
> On 1/22/2019 9:17 AM, Greg Kroah-Hartman wrote:
> > When calling debugfs functions, there is no need to ever check the
> > return value. The function can work or not, but the code logic should
> > never do something different based on this.
> >
> > Cc: Steve Wise
> > Cc: Doug Ledford
> > Cc: Jason Gunthorpe
> > Cc: linux-r...@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman
> > ---
> > drivers/infiniband/hw/cxgb4/device.c | 8 +---
> > 1 file changed, 1 insertion(+), 7 deletions(-)
> >
> > diff --git a/drivers/infiniband/hw/cxgb4/device.c
> > b/drivers/infiniband/hw/cxgb4/device.c
> > index c13c0ba30f63..9c10fff6dcfb 100644
> > --- a/drivers/infiniband/hw/cxgb4/device.c
> > +++ b/drivers/infiniband/hw/cxgb4/device.c
> > @@ -720,11 +720,8 @@ static const struct file_operations ep_debugfs_fops = {
> > .read= debugfs_read,
> > };
> >
> > -static int setup_debugfs(struct c4iw_dev *devp)
> > +static void setup_debugfs(struct c4iw_dev *devp)
> > {
> > - if (!devp->debugfs_root)
> > - return -1;
> > -
> > debugfs_create_file_size("qps", S_IWUSR, devp->debugfs_root,
> > (void *)devp, &qp_debugfs_fops, 4096);
> >
> > @@ -740,7 +737,6 @@ static int setup_debugfs(struct c4iw_dev *devp)
> > if (c4iw_wr_log)
> > debugfs_create_file_size("wr_log", S_IWUSR, devp->debugfs_root,
> > (void *)devp, &wr_log_debugfs_fops,
> > 4096);
> > - return 0;
> > }
> >
> > void c4iw_release_dev_ucontext(struct c4iw_rdev *rdev,
> > @@ -1553,8 +1549,6 @@ static int __init c4iw_init_module(void)
> > return err;
> >
> > c4iw_debugfs_root = debugfs_create_dir(DRV_NAME, NULL);
> > - if (!c4iw_debugfs_root)
> > - pr_warn("could not create debugfs entry, continuing\n");
> >
> > reg_workq = create_singlethread_workqueue("Register_iWARP_device");
> > if (!reg_workq) {
> >
>
> So it is not a problem to call debugfs_create_file_size() when
> devp->debugfs_root is NULL?
Nope!
>
> Acked-by: Steve Wise
Thanks,
greg k-h
[PATCH] ath: move spin_lock_bh to spin_lock in tasklet
as you are already in a tasklet, it is unnecessary to call
spin_lock_bh, because softirq already disable BH.
Signed-off-by: Zhiwei Jiang
---
drivers/net/wireless/ath/ath9k/htc_drv_txrx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
index 799010ed04e0..4e8e80ac8341 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
@@ -574,12 +574,12 @@ void ath9k_tx_failed_tasklet(unsigned long data)
{
struct ath9k_htc_priv *priv = (struct ath9k_htc_priv *)data;
- spin_lock_bh(&priv->tx.tx_lock);
+ spin_lock(&priv->tx.tx_lock);
if (priv->tx.flags & ATH9K_HTC_OP_TX_DRAIN) {
- spin_unlock_bh(&priv->tx.tx_lock);
+ spin_unlock(&priv->tx.tx_lock);
return;
}
- spin_unlock_bh(&priv->tx.tx_lock);
+ spin_unlock(&priv->tx.tx_lock);
ath9k_htc_tx_drainq(priv, &priv->tx.tx_failed);
}
--
2.19.0
Re: [PATCH] s390: kernel: no need to check return value of debugfs_create functions
On Tue, Jan 22, 2019 at 05:24:54PM +0100, Heiko Carstens wrote:
> On Tue, Jan 22, 2019 at 04:21:02PM +0100, Greg Kroah-Hartman wrote:
> > When calling debugfs functions, there is no need to ever check the
> > return value. The function can work or not, but the code logic should
> > never do something different based on this.
> >
> > Cc: Martin Schwidefsky
> > Cc: Heiko Carstens
> > Cc: Kees Cook
> > Cc: Christian Borntraeger
> > Cc: Hendrik Brueckner
> > Cc: linux-s...@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman
> > ---
> > arch/s390/kernel/debug.c| 6 --
> > arch/s390/kernel/kdebugfs.c | 2 --
> > arch/s390/kernel/sysinfo.c | 2 --
> > 3 files changed, 10 deletions(-)
> > diff --git a/arch/s390/kernel/sysinfo.c b/arch/s390/kernel/sysinfo.c
> > index 12f80d1f0415..2ac3c9b56a13 100644
> > --- a/arch/s390/kernel/sysinfo.c
> > +++ b/arch/s390/kernel/sysinfo.c
> > @@ -545,8 +545,6 @@ static __init int stsi_init_debugfs(void)
> > int lvl, i;
> >
> > stsi_root = debugfs_create_dir("stsi", arch_debugfs_dir);
> > - if (IS_ERR_OR_NULL(stsi_root))
> > - return 0;
>
> No objections, however will you also change the odd behaviour that
> e.g. debugfs_create_file() returns -ENODEV instead of (the expected)
> NULL pointer if CONFIG_DEBUGFS is disabled?
Nope. That is intentional.
> I do remember this since it caused at least one crash ;)
Which is why you shouldn't care about the return value of these
functions :)
> 19cdd08ba155 ("[S390] qdio: fix broken pointer in case of CONFIG_DEBUG_FS is
> disabled").
Odd, what crashes when passed an error pointer? What was someone trying
to do with those pointers? The only thing you can do with a return
value from a debugfs function is to pass it back into another debugfs
call. Sounds like someone wasn't doing that :(
Given that that patch was from 2.6.29, I think we are safe...
thanks,
greg k-h
Just working
Want to retouch your photos? we can help you for that. For your photos we can do white background, sharpen, retouching included all for your photos. Send us photos for testing, we will start it. Thanks, Laura
Just working
Want to retouch your photos? we can help you for that. For your photos we can do white background, sharpen, retouching included all for your photos. Send us photos for testing, we will start it. Thanks, Laura
Just working
Want to retouch your photos? we can help you for that. For your photos we can do white background, sharpen, retouching included all for your photos. Send us photos for testing, we will start it. Thanks, Laura
Re: [PATCHv4 05/13] Documentation/ABI: Add new node sysfs attributes
On Sun, Jan 20, 2019 at 05:16:05PM +0100, Rafael J. Wysocki wrote: > On Sat, Jan 19, 2019 at 10:01 AM Greg Kroah-Hartman > wrote: > > > > If you do a subdirectory "correctly" (i.e. a name for an attribute > > group), that's fine. > > Yes, that's what I was thinking about: along the lines of the "power" > group under device kobjects. We can't append symlinks to an attribute group, though. I'd need to create a lot of struct devices just to get the desired directory hiearchy. And then each of those "devices" will have their own "power" group, which really doesn't make any sense for what we're trying to show. Is that really the right way to do this, or something else I'm missing?
Re: [PATCH] regmap: no need to check return value of debugfs_create functions
On Tue, Jan 22, 2019 at 05:08:57PM +0100, Greg Kroah-Hartman wrote: > On Tue, Jan 22, 2019 at 03:56:06PM +, Mark Brown wrote: > > Given that all the rest of the function is doing is further debugfs > > operations and when it fails people trying to use the debugfs do welcome > > some diagnostics I'm not sure that's particularly helpful. > The only way it will fail is if we are out of memory. And you are in a > bigger mess then, no one cares about debugfs calls, just make them and > move on, you should never care about the result of such a call. No, it also fails if there's already something with the same name in debugfs which can happen as as a result of configuration. This gets confusing for users, they see the debugfs files they're expecting but the contents don't match up at all. signature.asc Description: PGP signature
Re: [PATCH v2] ima: define ima_post_create_tmpfile() hook and add missing call
On 10:43 22/01, Mimi Zohar wrote: > On Mon, 2019-01-21 at 14:29 +0200, Amir Goldstein wrote: > > On Mon, Jan 21, 2019 at 2:00 PM Mimi Zohar wrote: > > > > > > On Thu, 2019-01-17 at 15:34 -0600, Goldwyn Rodrigues wrote: > > > > On 13:47 18/12, Mimi Zohar wrote: > > > > > If tmpfiles can be made persistent, then newly created tmpfiles need > > > > > to > > > > > be treated like any other new files in policy. > > > > > > > > > > This patch indicates which newly created tmpfiles are in policy, > > > > > causing > > > > > the file hash to be calculated on __fput(). > > > > > > > > Discussed in overlayfs, this would be better if we use this on inode > > > > and called from vfs_tmpfile() instead of do_tmpfile(). This will cover > > > > the overlayfs case which uses tmpfiles while performing copy_up(). > > > > The patch is attached. > > > > > > > > Here is the updated patch which works for my cases. > > > > However, it is the the failure case after setting the IMA flags > > > > I am concerned about, though I don't think that should be as harmful. > > > > > > Right. The new IMA hook allocates memory for storing the flags, which > > > needs to be cleaned up on failure. For this reason, the IMA call is > > > deferred until after the transition from locally freeing memory on > > > failure to relying on __fput(). In "do_last", the call to IMA is > > > after "opened"; and in the original version of this patch the call to > > > IMA is after finish_open(). > > > > > > > Not sure I understand the concern. > > The integrity context is associated with the inode and will be freed > > on destroy_inode() no matter which error path is taken. > > Am I missing something? > > No, as long as destroy_inode() is called, it should be fine. > Excellent. I will resend the patch as v3. Thanks! -- Goldwyn
[PATCH net-next 00/12] code optimizations & bugfixes for HNS3 driver
This patchset includes bugfixes and code optimizations for the HNS3 ethernet controller driver Huazhong Tan (1): net: hns3: fix bug of ethtool_ops.get_channels for VF Jian Shen (2): net: hns3: add rx multicast packets statistic net: hns3: refactor the statistics updating for netdev Peng Li (2): net: hns3: add calling roce callback function when link status change net: hns3: clear param in ring when free ring Yunsheng Lin (6): net: hns3: fix rss configuration lost problem when setting channel net: hns3: fix for shaper not setting when TC num changes net: hns3: Change fw error code NOT_EXEC to NOT_SUPPORTED net: hns3: do not return GE PFC setting err when initializing net: hns3: add ETS TC weight setting in SSU module net: hns3: fix PFC not setting problem for DCB module liuzhongzhu (1): net: hns3: add statistics for PFC frames and MAC control frames drivers/net/ethernet/hisilicon/hns3/hnae3.h | 3 +- .../net/ethernet/hisilicon/hns3/hns3_enet.c | 47 -- .../net/ethernet/hisilicon/hns3/hns3_enet.h | 8 + .../ethernet/hisilicon/hns3/hns3_ethtool.c| 1 + .../hisilicon/hns3/hns3pf/hclge_cmd.c | 12 +- .../hisilicon/hns3/hns3pf/hclge_cmd.h | 4 +- .../hisilicon/hns3/hns3pf/hclge_dcb.c | 19 +-- .../hisilicon/hns3/hns3pf/hclge_main.c| 138 ++ .../hisilicon/hns3/hns3pf/hclge_main.h| 8 + .../ethernet/hisilicon/hns3/hns3pf/hclge_tm.c | 70 +++-- .../ethernet/hisilicon/hns3/hns3pf/hclge_tm.h | 7 +- .../hisilicon/hns3/hns3vf/hclgevf_main.c | 10 +- 12 files changed, 254 insertions(+), 73 deletions(-) -- 2.20.1
[PATCH net-next 11/12] net: hns3: add statistics for PFC frames and MAC control frames
From: liuzhongzhu
In the old firmware version, statistics acquisition of
PFC frames and MAC control frames is not supported.
Add command retrieves statistics for PFC frames and
MAC control frames from the firmware.
Signed-off-by: liuzhongzhu
Signed-off-by: Peng Li
Signed-off-by: Huazhong Tan
---
.../hisilicon/hns3/hns3pf/hclge_cmd.c | 10 +-
.../hisilicon/hns3/hns3pf/hclge_cmd.h | 2 +
.../hisilicon/hns3/hns3pf/hclge_main.c| 97 ++-
.../hisilicon/hns3/hns3pf/hclge_main.h| 7 ++
4 files changed, 109 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
index b0ee070884eb..81dbe1b6abb0 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
@@ -170,8 +170,12 @@ static bool hclge_is_special_opcode(u16 opcode)
/* these commands have several descriptors,
* and use the first one to save opcode and return value
*/
- u16 spec_opcode[3] = {HCLGE_OPC_STATS_64_BIT,
- HCLGE_OPC_STATS_32_BIT, HCLGE_OPC_STATS_MAC};
+ u16 spec_opcode[] = {HCLGE_OPC_STATS_64_BIT,
+HCLGE_OPC_STATS_32_BIT,
+HCLGE_OPC_STATS_MAC,
+HCLGE_OPC_STATS_MAC_ALL,
+HCLGE_OPC_QUERY_32_BIT_REG,
+HCLGE_OPC_QUERY_64_BIT_REG};
int i;
for (i = 0; i < ARRAY_SIZE(spec_opcode); i++) {
@@ -259,6 +263,8 @@ int hclge_cmd_send(struct hclge_hw *hw, struct hclge_desc
*desc, int num)
if (desc_ret == HCLGE_CMD_EXEC_SUCCESS)
retval = 0;
+ else if (desc_ret == HCLGE_CMD_NO_AUTH)
+ retval = -EPERM;
else if (desc_ret == HCLGE_CMD_NOT_SUPPORTED)
retval = -EOPNOTSUPP;
else
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
index 9f07279513b7..e26a25128693 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
@@ -82,6 +82,8 @@ enum hclge_opcode_type {
HCLGE_OPC_STATS_64_BIT = 0x0030,
HCLGE_OPC_STATS_32_BIT = 0x0031,
HCLGE_OPC_STATS_MAC = 0x0032,
+ HCLGE_OPC_QUERY_MAC_REG_NUM = 0x0033,
+ HCLGE_OPC_STATS_MAC_ALL = 0x0034,
HCLGE_OPC_QUERY_REG_NUM = 0x0040,
HCLGE_OPC_QUERY_32_BIT_REG = 0x0041,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 6fb3144eb79d..64b1589e549f 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -118,6 +118,12 @@ static const struct hclge_comm_stats_str
g_mac_stats_string[] = {
HCLGE_MAC_STATS_FIELD_OFF(mac_tx_mac_pause_num)},
{"mac_rx_mac_pause_num",
HCLGE_MAC_STATS_FIELD_OFF(mac_rx_mac_pause_num)},
+ {"mac_tx_control_pkt_num",
+ HCLGE_MAC_STATS_FIELD_OFF(mac_tx_ctrl_pkt_num)},
+ {"mac_rx_control_pkt_num",
+ HCLGE_MAC_STATS_FIELD_OFF(mac_rx_ctrl_pkt_num)},
+ {"mac_tx_pfc_pkt_num",
+ HCLGE_MAC_STATS_FIELD_OFF(mac_tx_pfc_pause_pkt_num)},
{"mac_tx_pfc_pri0_pkt_num",
HCLGE_MAC_STATS_FIELD_OFF(mac_tx_pfc_pri0_pkt_num)},
{"mac_tx_pfc_pri1_pkt_num",
@@ -134,6 +140,8 @@ static const struct hclge_comm_stats_str
g_mac_stats_string[] = {
HCLGE_MAC_STATS_FIELD_OFF(mac_tx_pfc_pri6_pkt_num)},
{"mac_tx_pfc_pri7_pkt_num",
HCLGE_MAC_STATS_FIELD_OFF(mac_tx_pfc_pri7_pkt_num)},
+ {"mac_rx_pfc_pkt_num",
+ HCLGE_MAC_STATS_FIELD_OFF(mac_rx_pfc_pause_pkt_num)},
{"mac_rx_pfc_pri0_pkt_num",
HCLGE_MAC_STATS_FIELD_OFF(mac_rx_pfc_pri0_pkt_num)},
{"mac_rx_pfc_pri1_pkt_num",
@@ -287,10 +295,9 @@ static const struct hclge_mac_mgr_tbl_entry_cmd
hclge_mgr_table[] = {
},
};
-static int hclge_mac_update_stats(struct hclge_dev *hdev)
+static int hclge_mac_update_stats_defective(struct hclge_dev *hdev)
{
#define HCLGE_MAC_CMD_NUM 21
-#define HCLGE_RTN_DATA_NUM 4
u64 *data = (u64 *)(&hdev->hw_stats.mac_stats);
struct hclge_desc desc[HCLGE_MAC_CMD_NUM];
@@ -308,22 +315,102 @@ static int hclge_mac_update_stats(struct hclge_dev *hdev)
}
for (i = 0; i < HCLGE_MAC_CMD_NUM; i++) {
+ /* for special opcode 0032, only the first desc has the head */
if (unlikely(i == 0)) {
desc_data = (__le64 *)(&desc[i].data[0]);
- n
[PATCH net-next 09/12] net: hns3: do not return GE PFC setting err when initializing
From: Yunsheng Lin
GE MAC does not support PFC, when driver is initializing and MAC
is in GE Mode, ignore the fw not supported error, otherwise
initialization will fail.
Signed-off-by: Yunsheng Lin
Signed-off-by: Peng Li
Signed-off-by: Huazhong Tan
---
.../hisilicon/hns3/hns3pf/hclge_dcb.c | 6 +++---
.../hisilicon/hns3/hns3pf/hclge_main.c| 2 +-
.../ethernet/hisilicon/hns3/hns3pf/hclge_tm.c | 19 ---
.../ethernet/hisilicon/hns3/hns3pf/hclge_tm.h | 4 ++--
4 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
index 5f7ac63707b8..7db491086fea 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
@@ -166,7 +166,7 @@ static int hclge_map_update(struct hnae3_handle *h)
if (ret)
return ret;
- ret = hclge_pause_setup_hw(hdev);
+ ret = hclge_pause_setup_hw(hdev, false);
if (ret)
return ret;
@@ -313,7 +313,7 @@ static int hclge_ieee_setpfc(struct hnae3_handle *h, struct
ieee_pfc *pfc)
hdev->tm_info.hw_pfc_map = pfc_map;
- return hclge_pause_setup_hw(hdev);
+ return hclge_pause_setup_hw(hdev, false);
}
/* DCBX configuration */
@@ -361,7 +361,7 @@ static int hclge_setup_tc(struct hnae3_handle *h, u8 tc, u8
*prio_tc)
hclge_tm_schd_info_update(hdev, tc);
hclge_tm_prio_tc_info_update(hdev, prio_tc);
- ret = hclge_tm_init_hw(hdev);
+ ret = hclge_tm_init_hw(hdev, false);
if (ret)
return ret;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 3ba8de93bc3d..6fb3144eb79d 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -7428,7 +7428,7 @@ static int hclge_reset_ae_dev(struct hnae3_ae_dev *ae_dev)
return ret;
}
- ret = hclge_tm_init_hw(hdev);
+ ret = hclge_tm_init_hw(hdev, true);
if (ret) {
dev_err(&pdev->dev, "tm init hw fail, ret =%d\n", ret);
return ret;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
index d057c9f03175..44316602c2fc 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
@@ -1255,7 +1255,7 @@ static int hclge_tm_bp_setup(struct hclge_dev *hdev)
return ret;
}
-int hclge_pause_setup_hw(struct hclge_dev *hdev)
+int hclge_pause_setup_hw(struct hclge_dev *hdev, bool init)
{
int ret;
@@ -1271,10 +1271,15 @@ int hclge_pause_setup_hw(struct hclge_dev *hdev)
if (!hnae3_dev_dcb_supported(hdev))
return 0;
- /* When MAC is GE Mode, hdev does not support pfc setting */
+ /* GE MAC does not support PFC, when driver is initializing and MAC
+* is in GE Mode, ignore the error here, otherwise initialization
+* will fail.
+*/
ret = hclge_pfc_setup_hw(hdev);
- if (ret)
- dev_warn(&hdev->pdev->dev, "set pfc pause failed:%d\n", ret);
+ if (init && ret == -EOPNOTSUPP)
+ dev_warn(&hdev->pdev->dev, "GE MAC does not support pfc\n");
+ else
+ return ret;
return hclge_tm_bp_setup(hdev);
}
@@ -1314,7 +1319,7 @@ void hclge_tm_schd_info_update(struct hclge_dev *hdev, u8
num_tc)
hclge_tm_schd_info_init(hdev);
}
-int hclge_tm_init_hw(struct hclge_dev *hdev)
+int hclge_tm_init_hw(struct hclge_dev *hdev, bool init)
{
int ret;
@@ -1326,7 +1331,7 @@ int hclge_tm_init_hw(struct hclge_dev *hdev)
if (ret)
return ret;
- ret = hclge_pause_setup_hw(hdev);
+ ret = hclge_pause_setup_hw(hdev, init);
if (ret)
return ret;
@@ -1345,7 +1350,7 @@ int hclge_tm_schd_init(struct hclge_dev *hdev)
if (ret)
return ret;
- return hclge_tm_init_hw(hdev);
+ return hclge_tm_init_hw(hdev, true);
}
int hclge_tm_vport_map_update(struct hclge_dev *hdev)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h
index ef3f93b70c22..f60e540c7a62 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h
@@ -143,12 +143,12 @@ struct hclge_port_shapping_cmd {
int hclge_tm_schd_init(struct hclge_dev *hdev);
int hclge_tm_vport_map_update(struct hclge_dev *hdev);
-int hclge_pause_setup_hw(struct hclge_dev *hdev);
+int hclge_pause_setup_hw(struct hclge_dev *hdev, bool init);
int hclge_tm_schd_setup_hw(struct hclge_dev *hdev);
void hclge_tm_prio_tc_info_update(struct hclge_dev *hdev, u8
[PATCH net-next 10/12] net: hns3: add ETS TC weight setting in SSU module
From: Yunsheng Lin
This patch sets the TC weight in SSU module according to
info in tm_info.
Also, zero weight of TC weight in SSU ETS module means enabling
strict priority, so do not allow zero weight when in ETS mode.
Signed-off-by: Yunsheng Lin
Signed-off-by: Peng Li
Signed-off-by: Huazhong Tan
---
.../ethernet/hisilicon/hns3/hns3pf/hclge_tm.c | 43 +++
1 file changed, 43 insertions(+)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
index 44316602c2fc..bad975ebe137 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
@@ -947,6 +947,36 @@ static int hclge_tm_pri_tc_base_dwrr_cfg(struct hclge_dev
*hdev)
return 0;
}
+static int hclge_tm_ets_tc_dwrr_cfg(struct hclge_dev *hdev)
+{
+#define DEFAULT_TC_WEIGHT 1
+#define DEFAULT_TC_OFFSET 14
+
+ struct hclge_ets_tc_weight_cmd *ets_weight;
+ struct hclge_desc desc;
+ int i;
+
+ hclge_cmd_setup_basic_desc(&desc, HCLGE_OPC_ETS_TC_WEIGHT, false);
+ ets_weight = (struct hclge_ets_tc_weight_cmd *)desc.data;
+
+ for (i = 0; i < HNAE3_MAX_TC; i++) {
+ struct hclge_pg_info *pg_info;
+
+ ets_weight->tc_weight[i] = DEFAULT_TC_WEIGHT;
+
+ if (!(hdev->hw_tc_map & BIT(i)))
+ continue;
+
+ pg_info =
+ &hdev->tm_info.pg_info[hdev->tm_info.tc_info[i].pgid];
+ ets_weight->tc_weight[i] = pg_info->tc_dwrr[i];
+ }
+
+ ets_weight->weight_offset = DEFAULT_TC_OFFSET;
+
+ return hclge_cmd_send(&hdev->hw, &desc, 1);
+}
+
static int hclge_tm_pri_vnet_base_dwrr_pri_cfg(struct hclge_vport *vport)
{
struct hnae3_knic_private_info *kinfo = &vport->nic.kinfo;
@@ -996,6 +1026,19 @@ static int hclge_tm_pri_dwrr_cfg(struct hclge_dev *hdev)
ret = hclge_tm_pri_tc_base_dwrr_cfg(hdev);
if (ret)
return ret;
+
+ if (!hnae3_dev_dcb_supported(hdev))
+ return 0;
+
+ ret = hclge_tm_ets_tc_dwrr_cfg(hdev);
+ if (ret == -EOPNOTSUPP) {
+ dev_warn(&hdev->pdev->dev,
+"fw %08x does't support ets tc weight cmd\n",
+hdev->fw_version);
+ ret = 0;
+ }
+
+ return ret;
} else {
ret = hclge_tm_pri_vnet_base_dwrr_cfg(hdev);
if (ret)
--
2.20.1
[PATCH net-next 08/12] net: hns3: Change fw error code NOT_EXEC to NOT_SUPPORTED
From: Yunsheng Lin
According to firmware error code definition, the error code of 2
means NOT_SUPPORTED, this patch changes it to NOT_SUPPORTED.
Signed-off-by: Yunsheng Lin
Signed-off-by: Peng Li
Signed-off-by: Huazhong Tan
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c | 2 ++
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
index e483a6e730e6..b0ee070884eb 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.c
@@ -259,6 +259,8 @@ int hclge_cmd_send(struct hclge_hw *hw, struct hclge_desc
*desc, int num)
if (desc_ret == HCLGE_CMD_EXEC_SUCCESS)
retval = 0;
+ else if (desc_ret == HCLGE_CMD_NOT_SUPPORTED)
+ retval = -EOPNOTSUPP;
else
retval = -EIO;
hw->cmq.last_status = desc_ret;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
index f23042b24c09..9f07279513b7 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_cmd.h
@@ -39,7 +39,7 @@ struct hclge_cmq_ring {
enum hclge_cmd_return_status {
HCLGE_CMD_EXEC_SUCCESS = 0,
HCLGE_CMD_NO_AUTH = 1,
- HCLGE_CMD_NOT_EXEC = 2,
+ HCLGE_CMD_NOT_SUPPORTED = 2,
HCLGE_CMD_QUEUE_FULL= 3,
};
--
2.20.1
[PATCH net-next 01/12] net: hns3: add calling roce callback function when link status change
From: Peng Li
This patch adds calling roce callback function when link status
change.
Signed-off-by: Wei Hu (Xavier)
Signed-off-by: Peng Li
Signed-off-by: Huazhong Tan
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 6 ++
drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 5 +
2 files changed, 11 insertions(+)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 00d7acb4d45a..35fb0c54b986 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -2105,7 +2105,9 @@ static int hclge_get_mac_phy_link(struct hclge_dev *hdev)
static void hclge_update_link_status(struct hclge_dev *hdev)
{
+ struct hnae3_client *rclient = hdev->roce_client;
struct hnae3_client *client = hdev->nic_client;
+ struct hnae3_handle *rhandle;
struct hnae3_handle *handle;
int state;
int i;
@@ -2117,6 +2119,10 @@ static void hclge_update_link_status(struct hclge_dev
*hdev)
for (i = 0; i < hdev->num_vmdq_vport + 1; i++) {
handle = &hdev->vport[i].nic;
client->ops->link_status_change(handle, state);
+ rhandle = &hdev->vport[i].roce;
+ if (rclient && rclient->ops->link_status_change)
+ rclient->ops->link_status_change(rhandle,
+state);
}
hdev->hw.mac.link = state;
}
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index bb9f45200ef5..989f08377d58 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -349,16 +349,21 @@ static void hclgevf_request_link_info(struct hclgevf_dev
*hdev)
void hclgevf_update_link_status(struct hclgevf_dev *hdev, int link_state)
{
+ struct hnae3_handle *rhandle = &hdev->roce;
struct hnae3_handle *handle = &hdev->nic;
+ struct hnae3_client *rclient;
struct hnae3_client *client;
client = handle->client;
+ rclient = hdev->roce_client;
link_state =
test_bit(HCLGEVF_STATE_DOWN, &hdev->state) ? 0 : link_state;
if (link_state != hdev->hw.mac.link) {
client->ops->link_status_change(handle, !!link_state);
+ if (rclient && rclient->ops->link_status_change)
+ rclient->ops->link_status_change(rhandle, !!link_state);
hdev->hw.mac.link = link_state;
}
}
--
2.20.1
Re: [PATCH v25 0/6] Add io{read|write}64 to io-64-atomic headers
On 2019-01-22 5:40 a.m., Greg Kroah-Hartman wrote: > On Wed, Jan 16, 2019 at 11:25:17AM -0700, Logan Gunthorpe wrote: >> This is resend number 6 since the last change to this series. >> >> This cleanup was requested by Greg KH back in June of 2017. I've resent the >> series >> a couple times a cycle since then, updating and fixing as feedback was slowly >> recieved some patches were alread accepted by specific arches. In June 2018, >> Andrew picked the remainder of this up and it was in linux-next for a >> couple weeks. There were a couple problems that were identified and addressed >> back then and I'd really like to get the ball rolling again. A year >> and a half of sending this without much feedback is far too long. >> >> @Andrew, can you please pick this set up again so it can get into >> linux-next? Or let me know if there's something else I should be doing. > > version 25? That's crazy, this has gone on too long. I've taken this > into my char/misc driver tree now, so sorry for the long suffering you > have gone through for this. Thanks a lot! Logan
[PATCH net-next 12/12] net: hns3: fix PFC not setting problem for DCB module
From: Yunsheng Lin
The PFC enabling is based on user priority, currently it is
based on TC, which may cause PFC not setting correctly when pri
to TC mapping is not one to one relation.
This patch adds pfc_en in tm_info to fix it.
Fixes: cacde272dd00 ("net: hns3: Add hclge_dcb module for the support of DCB
feature")
Signed-off-by: Yunsheng Lin
Signed-off-by: Peng Li
Signed-off-by: Huazhong Tan
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c | 7 ---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h | 1 +
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c | 2 +-
3 files changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
index 7db491086fea..3a4a54ee5204 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
@@ -296,6 +296,9 @@ static int hclge_ieee_setpfc(struct hnae3_handle *h, struct
ieee_pfc *pfc)
hdev->flag & HCLGE_FLAG_MQPRIO_ENABLE)
return -EINVAL;
+ if (pfc->pfc_en == hdev->tm_info.pfc_en)
+ return 0;
+
prio_tc = hdev->tm_info.prio_tc;
pfc_map = 0;
@@ -308,10 +311,8 @@ static int hclge_ieee_setpfc(struct hnae3_handle *h,
struct ieee_pfc *pfc)
}
}
- if (pfc_map == hdev->tm_info.hw_pfc_map)
- return 0;
-
hdev->tm_info.hw_pfc_map = pfc_map;
+ hdev->tm_info.pfc_en = pfc->pfc_en;
return hclge_pause_setup_hw(hdev, false);
}
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
index b5a38fc1af91..2c413c63c6c9 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h
@@ -325,6 +325,7 @@ struct hclge_tm_info {
struct hclge_tc_info tc_info[HNAE3_MAX_TC];
enum hclge_fc_mode fc_mode;
u8 hw_pfc_map; /* Allow for packet drop or not on this TC */
+ u8 pfc_en; /* PFC enabled or not for user priority */
};
struct hclge_comm_stats_str {
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
index bad975ebe137..9f4069fb786b 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
@@ -1215,7 +1215,7 @@ static int hclge_pfc_setup_hw(struct hclge_dev *hdev)
HCLGE_RX_MAC_PAUSE_EN_MSK;
return hclge_pfc_pause_en_cfg(hdev, enable_bitmap,
- hdev->tm_info.hw_pfc_map);
+ hdev->tm_info.pfc_en);
}
/* Each Tc has a 1024 queue sets to backpress, it divides to
--
2.20.1
[PATCH net-next 06/12] net: hns3: fix bug of ethtool_ops.get_channels for VF
The current code returns the number of all queues that can be used and
the number of queues that have been allocated, which is incorrect.
What should be returned is the number of queues allocated for each enabled
TC and the number of queues that can be allocated.
This patch fixes it.
Fixes: 849e46077689 ("net: hns3: add ethtool_ops.get_channels support for VF")
Signed-off-by: Huazhong Tan
Signed-off-by: Peng Li
---
drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
index 989f08377d58..24b54083b5f9 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_main.c
@@ -2466,7 +2466,8 @@ static u32 hclgevf_get_max_channels(struct hclgevf_dev
*hdev)
struct hnae3_handle *nic = &hdev->nic;
struct hnae3_knic_private_info *kinfo = &nic->kinfo;
- return min_t(u32, hdev->rss_size_max * kinfo->num_tc, hdev->num_tqps);
+ return min_t(u32, hdev->rss_size_max,
+hdev->num_tqps / kinfo->num_tc);
}
/**
@@ -2487,7 +2488,7 @@ static void hclgevf_get_channels(struct hnae3_handle
*handle,
ch->max_combined = hclgevf_get_max_channels(hdev);
ch->other_count = 0;
ch->max_other = 0;
- ch->combined_count = hdev->num_tqps;
+ ch->combined_count = handle->kinfo.rss_size;
}
static void hclgevf_get_tqps_and_rss_info(struct hnae3_handle *handle,
--
2.20.1
[PATCH net-next 02/12] net: hns3: add rx multicast packets statistic
From: Jian Shen
This patch adds rx multicast packets statistic for each ring.
Signed-off-by: Jian Shen
Signed-off-by: Peng Li
Signed-off-by: Huazhong Tan
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c| 9 +
drivers/net/ethernet/hisilicon/hns3/hns3_enet.h| 8
drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c | 1 +
3 files changed, 18 insertions(+)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 9dd8949381bc..f1ab2e4ca49e 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -2572,6 +2572,7 @@ static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
struct sk_buff **out_skb)
{
struct net_device *netdev = ring->tqp->handle->kinfo.netdev;
+ enum hns3_pkt_l2t_type l2_frame_type;
struct sk_buff *skb = ring->skb;
struct hns3_desc_cb *desc_cb;
struct hns3_desc *desc;
@@ -2680,6 +2681,14 @@ static int hns3_handle_rx_bd(struct hns3_enet_ring *ring,
return -EFAULT;
}
+ l2_frame_type = hnae3_get_field(l234info, HNS3_RXD_DMAC_M,
+ HNS3_RXD_DMAC_S);
+ if (l2_frame_type == HNS3_L2_TYPE_MULTICAST) {
+ u64_stats_update_begin(&ring->syncp);
+ ring->stats.rx_multicast++;
+ u64_stats_update_end(&ring->syncp);
+ }
+
u64_stats_update_begin(&ring->syncp);
ring->stats.rx_pkts++;
ring->stats.rx_bytes += skb->len;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
index f59ab7387b1f..f3d248626ab3 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.h
@@ -202,6 +202,13 @@ enum hns3_nic_state {
#define HNS3_RING_EN_B 0
+enum hns3_pkt_l2t_type {
+ HNS3_L2_TYPE_UNICAST,
+ HNS3_L2_TYPE_MULTICAST,
+ HNS3_L2_TYPE_BROADCAST,
+ HNS3_L2_TYPE_INVALID,
+};
+
enum hns3_pkt_l3t_type {
HNS3_L3T_NONE,
HNS3_L3T_IPV6,
@@ -376,6 +383,7 @@ struct ring_stats {
u64 err_bd_num;
u64 l2_err;
u64 l3l4_csum_err;
+ u64 rx_multicast;
};
};
};
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
index e678b6939da3..abb78696d7ce 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_ethtool.c
@@ -47,6 +47,7 @@ static const struct hns3_stats hns3_rxq_stats[] = {
HNS3_TQP_STAT("err_bd_num", err_bd_num),
HNS3_TQP_STAT("l2_err", l2_err),
HNS3_TQP_STAT("l3l4_csum_err", l3l4_csum_err),
+ HNS3_TQP_STAT("multicast", rx_multicast),
};
#define HNS3_RXQ_STATS_COUNT ARRAY_SIZE(hns3_rxq_stats)
--
2.20.1
[PATCH net-next 07/12] net: hns3: clear param in ring when free ring
From: Peng Li
Param pending_buf and skb may be not NULL when free ring.
This patch clears them when free ring.
Signed-off-by: Peng Li
Signed-off-by: Huazhong Tan
---
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 5 +
1 file changed, 5 insertions(+)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 0a546ee03f92..1d414f016cfb 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -3400,6 +3400,11 @@ static void hns3_fini_ring(struct hns3_enet_ring *ring)
ring->desc_cb = NULL;
ring->next_to_clean = 0;
ring->next_to_use = 0;
+ ring->pending_buf = 0;
+ if (ring->skb) {
+ dev_kfree_skb_any(ring->skb);
+ ring->skb = NULL;
+ }
}
static int hns3_buf_size2type(u32 buf_size)
--
2.20.1
Just working
Want to retouch your photos? we can help you for that. For your photos we can do white background, sharpen, retouching included all for your photos. Send us photos for testing, we will start it. Thanks, Laura
[PATCH net-next 04/12] net: hns3: fix rss configuration lost problem when setting channel
From: Yunsheng Lin
Currently rss configuration set by user will be lost when setting
channel.
This patch fixes it by not setting rss configuration to default
if user has configured the rss.
Fixes: 09f2af6405b8 ("net: hns3: add support to modify tqps number")
Signed-off-by: Yunsheng Lin
Signed-off-by: Peng Li
Signed-off-by: Huazhong Tan
---
drivers/net/ethernet/hisilicon/hns3/hnae3.h | 3 ++-
drivers/net/ethernet/hisilicon/hns3/hns3_enet.c | 6 --
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 8 +++-
3 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
index d486748d5883..dc3db45361d3 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hnae3.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hnae3.h
@@ -433,7 +433,8 @@ struct hnae3_ae_ops {
struct ethtool_channels *ch);
void (*get_tqps_and_rss_info)(struct hnae3_handle *h,
u16 *alloc_tqps, u16 *max_rss_size);
- int (*set_channels)(struct hnae3_handle *handle, u32 new_tqps_num);
+ int (*set_channels)(struct hnae3_handle *handle, u32 new_tqps_num,
+ bool rxfh_configured);
void (*get_flowctrl_adv)(struct hnae3_handle *handle,
u32 *flowctrl_adv);
int (*set_led_id)(struct hnae3_handle *handle,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index 2b4199fe0e57..0a546ee03f92 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -4166,6 +4166,7 @@ int hns3_set_channels(struct net_device *netdev,
{
struct hnae3_handle *h = hns3_get_handle(netdev);
struct hnae3_knic_private_info *kinfo = &h->kinfo;
+ bool rxfh_configured = netif_is_rxfh_configured(netdev);
u32 new_tqp_num = ch->combined_count;
u16 org_tqp_num;
int ret;
@@ -4193,9 +4194,10 @@ int hns3_set_channels(struct net_device *netdev,
return ret;
org_tqp_num = h->kinfo.num_tqps;
- ret = h->ae_algo->ops->set_channels(h, new_tqp_num);
+ ret = h->ae_algo->ops->set_channels(h, new_tqp_num, rxfh_configured);
if (ret) {
- ret = h->ae_algo->ops->set_channels(h, org_tqp_num);
+ ret = h->ae_algo->ops->set_channels(h, org_tqp_num,
+ rxfh_configured);
if (ret) {
/* If revert to old tqp failed, fatal error occurred */
dev_err(&netdev->dev,
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index bf0931c6764f..3ba8de93bc3d 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -7518,7 +7518,8 @@ static void hclge_get_tqps_and_rss_info(struct
hnae3_handle *handle,
*max_rss_size = hdev->rss_size_max;
}
-static int hclge_set_channels(struct hnae3_handle *handle, u32 new_tqps_num)
+static int hclge_set_channels(struct hnae3_handle *handle, u32 new_tqps_num,
+ bool rxfh_configured)
{
struct hclge_vport *vport = hclge_get_vport(handle);
struct hnae3_knic_private_info *kinfo = &vport->nic.kinfo;
@@ -7557,6 +7558,10 @@ static int hclge_set_channels(struct hnae3_handle
*handle, u32 new_tqps_num)
if (ret)
return ret;
+ /* RSS indirection table has been configuared by user */
+ if (rxfh_configured)
+ goto out;
+
/* Reinitializes the rss indirect table according to the new RSS size */
rss_indir = kcalloc(HCLGE_RSS_IND_TBL_SIZE, sizeof(u32), GFP_KERNEL);
if (!rss_indir)
@@ -7572,6 +7577,7 @@ static int hclge_set_channels(struct hnae3_handle
*handle, u32 new_tqps_num)
kfree(rss_indir);
+out:
if (!ret)
dev_info(&hdev->pdev->dev,
"Channels changed, rss_size from %d to %d, tqps from
%d to %d",
--
2.20.1
[PATCH net-next 03/12] net: hns3: refactor the statistics updating for netdev
From: Jian Shen
In origin codes, there are some statistics item are got from mac, which
also include the packets statistics of VF. It is unreasonable. This
patch fixes it by counting them in the rx/tx processing flow.
Signed-off-by: Jian Shen
Signed-off-by: Peng Li
Signed-off-by: Huazhong Tan
---
.../net/ethernet/hisilicon/hns3/hns3_enet.c | 27 ++-
.../hisilicon/hns3/hns3pf/hclge_main.c| 25 -
2 files changed, 20 insertions(+), 32 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
index f1ab2e4ca49e..2b4199fe0e57 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3_enet.c
@@ -1399,7 +1399,12 @@ static void hns3_nic_get_stats64(struct net_device
*netdev,
int queue_num = priv->ae_handle->kinfo.num_tqps;
struct hnae3_handle *handle = priv->ae_handle;
struct hns3_enet_ring *ring;
+ u64 rx_length_errors = 0;
+ u64 rx_crc_errors = 0;
+ u64 rx_multicast = 0;
unsigned int start;
+ u64 tx_errors = 0;
+ u64 rx_errors = 0;
unsigned int idx;
u64 tx_bytes = 0;
u64 rx_bytes = 0;
@@ -1422,6 +1427,8 @@ static void hns3_nic_get_stats64(struct net_device
*netdev,
tx_pkts += ring->stats.tx_pkts;
tx_drop += ring->stats.tx_busy;
tx_drop += ring->stats.sw_err_cnt;
+ tx_errors += ring->stats.tx_busy;
+ tx_errors += ring->stats.sw_err_cnt;
} while (u64_stats_fetch_retry_irq(&ring->syncp, start));
/* fetch the rx stats */
@@ -1433,6 +1440,12 @@ static void hns3_nic_get_stats64(struct net_device
*netdev,
rx_drop += ring->stats.non_vld_descs;
rx_drop += ring->stats.err_pkt_len;
rx_drop += ring->stats.l2_err;
+ rx_errors += ring->stats.non_vld_descs;
+ rx_errors += ring->stats.l2_err;
+ rx_crc_errors += ring->stats.l2_err;
+ rx_crc_errors += ring->stats.l3l4_csum_err;
+ rx_multicast += ring->stats.rx_multicast;
+ rx_length_errors += ring->stats.err_pkt_len;
} while (u64_stats_fetch_retry_irq(&ring->syncp, start));
}
@@ -1441,15 +1454,15 @@ static void hns3_nic_get_stats64(struct net_device
*netdev,
stats->rx_bytes = rx_bytes;
stats->rx_packets = rx_pkts;
- stats->rx_errors = netdev->stats.rx_errors;
- stats->multicast = netdev->stats.multicast;
- stats->rx_length_errors = netdev->stats.rx_length_errors;
- stats->rx_crc_errors = netdev->stats.rx_crc_errors;
+ stats->rx_errors = rx_errors;
+ stats->multicast = rx_multicast;
+ stats->rx_length_errors = rx_length_errors;
+ stats->rx_crc_errors = rx_crc_errors;
stats->rx_missed_errors = netdev->stats.rx_missed_errors;
- stats->tx_errors = netdev->stats.tx_errors;
- stats->rx_dropped = rx_drop + netdev->stats.rx_dropped;
- stats->tx_dropped = tx_drop + netdev->stats.tx_dropped;
+ stats->tx_errors = tx_errors;
+ stats->rx_dropped = rx_drop;
+ stats->tx_dropped = tx_drop;
stats->collisions = netdev->stats.collisions;
stats->rx_over_errors = netdev->stats.rx_over_errors;
stats->rx_frame_errors = netdev->stats.rx_frame_errors;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index 35fb0c54b986..bf0931c6764f 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -461,26 +461,6 @@ static u8 *hclge_comm_get_strings(u32 stringset,
return (u8 *)buff;
}
-static void hclge_update_netstat(struct hclge_hw_stats *hw_stats,
-struct net_device_stats *net_stats)
-{
- net_stats->tx_dropped = 0;
- net_stats->rx_errors = hw_stats->mac_stats.mac_rx_oversize_pkt_num;
- net_stats->rx_errors += hw_stats->mac_stats.mac_rx_undersize_pkt_num;
- net_stats->rx_errors += hw_stats->mac_stats.mac_rx_fcs_err_pkt_num;
-
- net_stats->multicast = hw_stats->mac_stats.mac_tx_multi_pkt_num;
- net_stats->multicast += hw_stats->mac_stats.mac_rx_multi_pkt_num;
-
- net_stats->rx_crc_errors = hw_stats->mac_stats.mac_rx_fcs_err_pkt_num;
- net_stats->rx_length_errors =
- hw_stats->mac_stats.mac_rx_undersize_pkt_num;
- net_stats->rx_length_errors +=
- hw_stats->mac_stats.mac_rx_oversize_pkt_num;
- net_stats->rx_over_errors =
- hw_stats->mac_stats.mac_rx_oversize_pkt_num;
-}
-
static void hclge_update_stats_for_all(struct hclge_dev *hdev)
{
struct hnae3_handl
[PATCH net-next 05/12] net: hns3: fix for shaper not setting when TC num changes
From: Yunsheng Lin
Shaper setting does not change currently, when TC num changes,
which may cause shaper parameter not setting problem.
This patch fixes it by setting the shaper parameter when TC num
changes.
Fixes: cacde272dd00 ("net: hns3: Add hclge_dcb module for the support of DCB
feature")
Signed-off-by: Yunsheng Lin
Signed-off-by: Peng Li
Signed-off-by: Huazhong Tan
---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c | 6 +-
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c | 6 +++---
drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h | 3 +--
3 files changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
index 4ec0b9cd15ae..5f7ac63707b8 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_dcb.c
@@ -162,11 +162,7 @@ static int hclge_map_update(struct hnae3_handle *h)
struct hclge_dev *hdev = vport->back;
int ret;
- ret = hclge_tm_map_cfg(hdev);
- if (ret)
- return ret;
-
- ret = hclge_tm_schd_mode_hw(hdev);
+ ret = hclge_tm_schd_setup_hw(hdev);
if (ret)
return ret;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
index fb8596a3e5e4..d057c9f03175 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.c
@@ -1005,7 +1005,7 @@ static int hclge_tm_pri_dwrr_cfg(struct hclge_dev *hdev)
return 0;
}
-int hclge_tm_map_cfg(struct hclge_dev *hdev)
+static int hclge_tm_map_cfg(struct hclge_dev *hdev)
{
int ret;
@@ -1120,7 +1120,7 @@ static int hclge_tm_lvl34_schd_mode_cfg(struct hclge_dev
*hdev)
return 0;
}
-int hclge_tm_schd_mode_hw(struct hclge_dev *hdev)
+static int hclge_tm_schd_mode_hw(struct hclge_dev *hdev)
{
int ret;
@@ -1131,7 +1131,7 @@ int hclge_tm_schd_mode_hw(struct hclge_dev *hdev)
return hclge_tm_lvl34_schd_mode_cfg(hdev);
}
-static int hclge_tm_schd_setup_hw(struct hclge_dev *hdev)
+int hclge_tm_schd_setup_hw(struct hclge_dev *hdev)
{
int ret;
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h
b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h
index 898163c4d400..ef3f93b70c22 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_tm.h
@@ -144,11 +144,10 @@ struct hclge_port_shapping_cmd {
int hclge_tm_schd_init(struct hclge_dev *hdev);
int hclge_tm_vport_map_update(struct hclge_dev *hdev);
int hclge_pause_setup_hw(struct hclge_dev *hdev);
-int hclge_tm_schd_mode_hw(struct hclge_dev *hdev);
+int hclge_tm_schd_setup_hw(struct hclge_dev *hdev);
void hclge_tm_prio_tc_info_update(struct hclge_dev *hdev, u8 *prio_tc);
void hclge_tm_schd_info_update(struct hclge_dev *hdev, u8 num_tc);
int hclge_tm_dwrr_cfg(struct hclge_dev *hdev);
-int hclge_tm_map_cfg(struct hclge_dev *hdev);
int hclge_tm_init_hw(struct hclge_dev *hdev);
int hclge_mac_pause_en_cfg(struct hclge_dev *hdev, bool tx, bool rx);
int hclge_pause_addr_cfg(struct hclge_dev *hdev, const u8 *mac_addr);
--
2.20.1
Re: [PATCH] ath: move spin_lock_bh to spin_lock in tasklet
Zhiwei Jiang writes: > as you are already in a tasklet, it is unnecessary to call > spin_lock_bh, because softirq already disable BH. > > Signed-off-by: Zhiwei Jiang If you post a new version you should mark it as "v2": https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches#patch_version_missing No need to resend now but please keep in mind in the future. -- Kalle Valo
[PATCH v2] PM / EM: Expose the Energy Model in debugfs
The recently introduced Energy Model (EM) framework manages power cost
tables of CPUs. These tables are currently only visible from kernel
space. However, in order to debug the behaviour of subsystems that use
the EM (EAS for example), it is often required to know what the power
costs are from userspace.
For this reason, introduce under /sys/kernel/debug/energy_model a set of
directories representing the performance domains of the system. Each
performance domain contains a set of sub-directories representing the
different capacity states (cs) and their attributes, as well as a file
exposing the related CPUs.
The resulting hierarchy is as follows on Arm juno r0 for example:
/sys/kernel/debug/energy_model
├── pd0
│ ├── cpus
│ ├── cs:45
│ │ ├── cost
│ │ ├── frequency
│ │ └── power
│ ├── cs:575000
│ │ ├── cost
│ │ ├── frequency
│ │ └── power
│ ├── cs:70
│ │ ├── cost
│ │ ├── frequency
│ │ └── power
│ ├── cs:775000
│ │ ├── cost
│ │ ├── frequency
│ │ └── power
│ └── cs:85
│ ├── cost
│ ├── frequency
│ └── power
└── pd1
├── cpus
├── cs:110
│ ├── cost
│ ├── frequency
│ └── power
├── cs:45
│ ├── cost
│ ├── frequency
│ └── power
├── cs:625000
│ ├── cost
│ ├── frequency
│ └── power
├── cs:80
│ ├── cost
│ ├── frequency
│ └── power
└── cs:95
├── cost
├── frequency
└── power
Signed-off-by: Quentin Perret
---
V2: removed check on return value of debugfs_create_* (Greg KH)
---
kernel/power/energy_model.c | 57 +
1 file changed, 57 insertions(+)
diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
index d9dc2c38764a..7d66ee68aaaf 100644
--- a/kernel/power/energy_model.c
+++ b/kernel/power/energy_model.c
@@ -10,6 +10,7 @@
#include
#include
+#include
#include
#include
#include
@@ -23,6 +24,60 @@ static DEFINE_PER_CPU(struct em_perf_domain *, em_data);
*/
static DEFINE_MUTEX(em_pd_mutex);
+#ifdef CONFIG_DEBUG_FS
+static struct dentry *rootdir;
+
+static void em_debug_create_cs(struct em_cap_state *cs, struct dentry *pd)
+{
+ struct dentry *d;
+ char name[24];
+
+ snprintf(name, sizeof(name), "cs:%lu", cs->frequency);
+
+ /* Create per-cs directory */
+ d = debugfs_create_dir(name, pd);
+ debugfs_create_ulong("frequency", 0444, d, &cs->frequency);
+ debugfs_create_ulong("power", 0444, d, &cs->power);
+ debugfs_create_ulong("cost", 0444, d, &cs->cost);
+}
+
+static int em_debug_cpus_show(struct seq_file *s, void *unused)
+{
+ seq_printf(s, "%*pbl\n", cpumask_pr_args(to_cpumask(s->private)));
+
+ return 0;
+}
+DEFINE_SHOW_ATTRIBUTE(em_debug_cpus);
+
+static void em_debug_create_pd(struct em_perf_domain *pd, int cpu)
+{
+ struct dentry *d;
+ char name[8];
+ int i;
+
+ snprintf(name, sizeof(name), "pd%d", cpu);
+
+ /* Create the directory of the performance domain */
+ d = debugfs_create_dir(name, rootdir);
+
+ debugfs_create_file("cpus", 0444, d, pd->cpus, &em_debug_cpus_fops);
+
+ /* Create a sub-directory for each capacity state */
+ for (i = 0; i < pd->nr_cap_states; i++)
+ em_debug_create_cs(&pd->table[i], d);
+}
+
+static int __init em_debug_init(void)
+{
+ /* Create /sys/kernel/debug/energy_model directory */
+ rootdir = debugfs_create_dir("energy_model", NULL);
+
+ return 0;
+}
+core_initcall(em_debug_init);
+#else /* CONFIG_DEBUG_FS */
+static void em_debug_create_pd(struct em_perf_domain *pd, int cpu) {}
+#endif
static struct em_perf_domain *em_create_pd(cpumask_t *span, int nr_states,
struct em_data_callback *cb)
{
@@ -102,6 +157,8 @@ static struct em_perf_domain *em_create_pd(cpumask_t *span,
int nr_states,
pd->nr_cap_states = nr_states;
cpumask_copy(to_cpumask(pd->cpus), span);
+ em_debug_create_pd(pd, cpu);
+
return pd;
free_cs_table:
--
2.20.1
Re: [PATCH] regmap: no need to check return value of debugfs_create functions
On Tue, Jan 22, 2019 at 04:38:26PM +, Mark Brown wrote: > On Tue, Jan 22, 2019 at 05:08:57PM +0100, Greg Kroah-Hartman wrote: > > On Tue, Jan 22, 2019 at 03:56:06PM +, Mark Brown wrote: > > > > Given that all the rest of the function is doing is further debugfs > > > operations and when it fails people trying to use the debugfs do welcome > > > some diagnostics I'm not sure that's particularly helpful. > > > The only way it will fail is if we are out of memory. And you are in a > > bigger mess then, no one cares about debugfs calls, just make them and > > move on, you should never care about the result of such a call. > > No, it also fails if there's already something with the same name in > debugfs which can happen as as a result of configuration. This gets > confusing for users, they see the debugfs files they're expecting but > the contents don't match up at all. How can you allow a duplicate name for the other regmap stuff? Will that not also cause a collision somewhere else? Anyway, if this is that big of a problem, ok, but then your code will run differently if debugfs is enabled or not, which isn't ok. Don't rely on debugfs to do your name filtering for you :) thanks, greg k-h
Re: [PATCH 4.20 000/111] 4.20.4-stable review
On Tue, Jan 22, 2019 at 07:03:28PM +0530, Naresh Kamboju wrote: > On Mon, 21 Jan 2019 at 19:16, Greg Kroah-Hartman > wrote: > > > > This is the start of the stable review cycle for the 4.20.4 release. > > There are 111 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 Jan 23 12:23:56 UTC 2019. > > 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.20.4-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.20.y > > and the diffstat can be found below. > > > > thanks, > > > > greg k-h > > > > Results from Linaro’s test farm. > No regressions on arm64, arm, x86_64, and i386. Thanks for testing all of these and letting me know. greg k-h
Re: [PATCH 1/2] mm: introduce put_user_page*(), placeholder versions
On Tue, Jan 22, 2019 at 04:24:59PM +0100, Jan Kara wrote: > On Thu 17-01-19 10:17:59, Jerome Glisse wrote: > > On Thu, Jan 17, 2019 at 10:30:47AM +0100, Jan Kara wrote: > > > On Wed 16-01-19 08:08:14, Jerome Glisse wrote: > > > > On Wed, Jan 16, 2019 at 12:38:19PM +0100, Jan Kara wrote: > > > > > On Tue 15-01-19 09:07:59, Jan Kara wrote: > > > > > > Agreed. So with page lock it would actually look like: > > > > > > > > > > > > get_page_pin() > > > > > > lock_page(page); > > > > > > wait_for_stable_page(); > > > > > > atomic_add(&page->_refcount, PAGE_PIN_BIAS); > > > > > > unlock_page(page); > > > > > > > > > > > > And if we perform page_pinned() check under page lock, then if > > > > > > page_pinned() returned false, we are sure page is not and will not > > > > > > be > > > > > > pinned until we drop the page lock (and also until page writeback is > > > > > > completed if needed). > > > > > > > > > > After some more though, why do we even need wait_for_stable_page() and > > > > > lock_page() in get_page_pin()? > > > > > > > > > > During writepage page_mkclean() will write protect all page tables. So > > > > > there can be no new writeable GUP pins until we unlock the page as > > > > > all such > > > > > GUPs will have to first go through fault and ->page_mkwrite() > > > > > handler. And > > > > > that will wait on page lock and do wait_for_stable_page() for us > > > > > anyway. > > > > > Am I just confused? > > > > > > > > Yeah with page lock it should synchronize on the pte but you still > > > > need to check for writeback iirc the page is unlocked after file > > > > system has queue up the write and thus the page can be unlock with > > > > write back pending (and PageWriteback() == trye) and i am not sure > > > > that in that states we can safely let anyone write to that page. I > > > > am assuming that in some case the block device also expect stable > > > > page content (RAID stuff). > > > > > > > > So the PageWriteback() test is not only for racing page_mkclean()/ > > > > test_set_page_writeback() and GUP but also for pending write back. > > > > > > But this is prevented by wait_for_stable_page() that is already present in > > > ->page_mkwrite() handlers. Look: > > > > > > ->writepage() > > > /* Page is locked here */ > > > clear_page_dirty_for_io(page) > > > page_mkclean(page) > > > -> page tables get writeprotected > > > /* The following line will be added by our patches */ > > > if (page_pinned(page)) -> bounce > > > TestClearPageDirty(page) > > > set_page_writeback(page); > > > unlock_page(page); > > > ...submit_io... > > > > > > IRQ > > > - IO completion > > > end_page_writeback() > > > > > > So if GUP happens before page_mkclean() writeprotects corresponding PTE > > > (and these two actions are synchronized on the PTE lock), page_pinned() > > > will see the increment and report the page as pinned. > > > > > > If GUP happens after page_mkclean() writeprotects corresponding PTE, it > > > will fault: > > > handle_mm_fault() > > > do_wp_page() > > > wp_page_shared() > > > do_page_mkwrite() > > > ->page_mkwrite() - that is block_page_mkwrite() or > > > iomap_page_mkwrite() or whatever filesystem provides > > > lock_page(page) > > > ... prepare page ... > > > wait_for_stable_page(page) -> this blocks until IO completes > > > if someone cares about pages not being modified while under IO. > > > > The case i am worried is GUP see pte with write flag set but has not > > lock the page yet (GUP is get pte first, then pte to page then lock > > page), then it locks the page but the lock page can make it wait for a > > racing page_mkclean()...write back that have not yet write protected > > the pte the GUP just read. So by the time GUP has the page locked the > > pte it read might no longer have the write flag set. Hence why you need > > to also check for write back after taking the page lock. Alternatively > > you could recheck the pte after a successful try_lock on the page. > > This isn't really possible. GUP does: > > get_user_pages() > ... > follow_page_mask() > ... > follow_page_pte() > ptep = pte_offset_map_lock() > check permissions and page sanity > if (flags & FOLL_GET) > get_page(page); -> this would become > atomic_add(&page->_refcount, PAGE_PIN_BIAS); > pte_unmap_unlock(ptep, ptl); > > page_mkclean() on the other hand grabs the same pte lock to change the pte > to write-protected. So after page_mkclean() has modified the PTE we are > racing on for access, we are sure to either see increased _refcount or get > page fault from GUP. > > If we see increased _refcount, we bounce the page and are fine. If GUP > faults, we will wait for page lock (so wait until page is prepared for IO > and has PageWriteback set) while handling the fault, then enter > ->page_mkwrite, which will do wait_for_stable_page() -> wait for > outstand
Re: [PATCHv4 1/4] arm64: dts: qcom: sdm845: Add Coresight support
Hi Suzuki,
On 1/22/2019 9:38 PM, Suzuki K Poulose wrote:
By inconsistent, I meant the registers provides values which are not
the same on two different CPUs of the *same type*. And it is expected
that two different CPU/ETM implementations will have different PIDs.
SDM845 has 4 Kryo 385 Gold (ARM A75) + 4 Kryo 385 Silver (ARM A55),
so the PID values should be same for 4 ETMs atleast. But here one
pid value(001bb803) is same for 6 ETMs and other one for 2
ETMs(001bb802) which seems odd and hence the doubt if these pids
are even valid ones.
Below are the PIDs read for SDM845:
[ 5.996448] resname=etm@704 pid=001bb803
[ 6.052891] resname=etm@714 pid=001bb803
.. (Same pid=001bb803 for etm@724 to etm@754 but differs
for other 2 cpus as shown below)
[ 6.266687] resname=etm@764 pid=001bb802
[ 6.329171] resname=etm@774 pid=001bb802
This is the case for MSM8996 also as shown below where PID
value is not correct and has to be hardcoded.
They differ because they are two different types of CPU cores (and thus
different ETM PIDs). What does the Register descriptions say for
the Cores ?
To me it looks like there are two different types of Qualcomm
Cores with their respective ETMs which are missing in the ETM4x
driver and we are trying to "make the ETM" work by faking it as
something else, which is not nice. I would rather prefer
to add the appropriate masks and the expected value for these
two different ETM implementations and be done with it, instead
of faking it in all the DTs where these cores appear.
ETM4x driver does not have entries for A55 and A75. Could you please
let me know the PIDs for these CPUs so that we can compare?
For MSM8996:
resname=etm@3b4 pid=102f0205
I don't know what CPUs the MSM8996 have. If they don't have A53, you
must add the actual PIDs to the table once and for all.
But again, this PID is some invalid value. And does not correspond
to any of the ARM CPU cores and would be MSM8996 specific.
MSM8996 has 2+2 Kryo cores which are not ARM derivative as SDM845
if I am right.
+ etm@704 {
+ compatible = "arm,coresight-etm4x", "arm,primecell";
+ arm,primecell-periphid = <0x000bb95d> > +
reg = <0 0x0704 0 0x1000>;
+
+ cpu = <&CPU0>;
+
You seem to be specifying the PID of A53 ETM all over, while at least
one of your cores is ETMv4.2 (from the other patch) and A53 is not
ETMv4.2. As above, it would be good to add the PID to the table.
As explained in above comment, PID values read are not correct. Please
let me know if I am not clear.
There is no measure for "correctness" here. If the ETM exposes different
PID than what you expect from the TRM, then we could think of overriding
it. Otherwise please add the PIDs to the table.
This is exactly the case, ETM PID registers are exposing some invalid
value and hence we override in DT.
Thanks,
Sai
--
QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member
of Code Aurora Forum, hosted by The Linux Foundation
Re: [BUG] "access dev->iommu_fwspec" cause crash on BPI-R2
Hi Frank, thanks for the report! On Tue, Jan 22, 2019 at 05:09:09PM +0100, Frank Wunderlich wrote: > Hi, > > the following Patch breaks hdmi (at least) on Bananapi R2 (mt7623): > > a9bf2eec5a6fc01a0a5250eaf0bf61dfd382a78a "iommu/mediatek: Use helper > functions to access dev->iommu_fwspec" Does the attached diff fix the issue for you? Thanks, Joerg diff --git a/drivers/iommu/mtk_iommu_v1.c b/drivers/iommu/mtk_iommu_v1.c index 6ede4286b835..f60bdb85c4c0 100644 --- a/drivers/iommu/mtk_iommu_v1.c +++ b/drivers/iommu/mtk_iommu_v1.c @@ -442,6 +442,10 @@ static int mtk_iommu_add_device(struct device *dev) iommu_spec.args_count = count; mtk_iommu_create_mapping(dev, &iommu_spec); + + /* dev->iommu_fwspec might have changed */ + fwspec = dev_iommu_fwspec_get(dev); + of_node_put(iommu_spec.np); }
