Re: [RFC v5 02/38] powerpc: Free up four 64K PTE bits in 64K backed HPTE pages
On Wed, 5 Jul 2017 14:21:39 -0700 Ram Pai wrote: > Rearrange 64K PTE bits to free up bits 3, 4, 5 and 6 > in the 64K backed HPTE pages. This along with the earlier > patch will entirely free up the four bits from 64K PTE. > The bit numbers are big-endian as defined in the ISA3.0 > > This patch does the following change to 64K PTE backed > by 64K HPTE. > > H_PAGE_F_SECOND (S) which occupied bit 4 moves to the > second part of the pte to bit 60. > H_PAGE_F_GIX (G,I,X) which occupied bit 5, 6 and 7 also > moves to the second part of the pte to bit 61, > 62, 63, 64 respectively > > since bit 7 is now freed up, we move H_PAGE_BUSY (B) from > bit 9 to bit 7. > > The second part of the PTE will hold > (H_PAGE_F_SECOND|H_PAGE_F_GIX) at bit 60,61,62,63. > > Before the patch, the 64K HPTE backed 64k PTE format was > as follows > > 0 1 2 3 4 5 6 7 8 9 10...63 > : : : : : : : : : : :: > v v v v v v v v v v vv > > ,-,-,-,-,--,--,--,--,-,-,-,-,-,--,-,-,-, > |x|x|x| |S |G |I |X |x|B|x|x|x||.|.|.|.| <- primary pte > '_'_'_'_'__'__'__'__'_'_'_'_'_''_'_'_'_' > | | | | | | | | | | | | |..| | | | | <- secondary pte > '_'_'_'_'__'__'__'__'_'_'_'_'__'_'_'_'_' > It's not entirely clear what the secondary pte contains today and how many of the bits are free today? Balbir Singh.
Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support
On 10-07-17, 13:02, Dietmar Eggemann wrote: > Yes, I will change this. The #define approach is not really necessary > here since we're not in the scheduler hot-path and inlining is not > really required here. It would be part of scheduler hot-path for the fast-switching case, isn't it ? (I am not arguing against using weak functions, just wanted to correct above statement). -- viresh
Re: [PATCH v2 3/4] btrfs: Add zstd support
On 7/10/17, 9:57 PM, "Nick Terrell" wrote:
> The problem is caused by a gcc-7 bug [1]. It miscompiles
> ZSTD_wildcopy(void *dst, void const *src, ptrdiff_t len) when len is 0.
> It only happens when it can't analyze ZSTD_copy8(), which is the case in
> the kernel, because memcpy() is implemented with inline assembly. The
> generated code is slow anyways, so I propose this workaround, which will
> be included in the next patch set. I've confirmed that it fixes the bug for
> me. This alternative implementation is also 10-20x faster, and compiles to
> the same x86 assembly as the original ZSTD_wildcopy() with the userland
> memcpy() implementation [2].
>
> [1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81388#add_comment
> [2] https://godbolt.org/g/q5YpLx
>
> Signed-off-by: Nick Terrell
> ---
> lib/zstd/zstd_internal.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/lib/zstd/zstd_internal.h b/lib/zstd/zstd_internal.h
> index 6748719..ade0365 100644
> --- a/lib/zstd/zstd_internal.h
> +++ b/lib/zstd/zstd_internal.h
> @@ -126,7 +126,9 @@ static const U32 OF_defaultNormLog = OF_DEFAULTNORMLOG;
> /*-***
> * Shared functions to include for inlining
> */
> -static void ZSTD_copy8(void *dst, const void *src) { memcpy(dst, src, 8); }
> +static void ZSTD_copy8(void *dst, const void *src) {
> + ZSTD_write64(dst, ZSTD_read64(src));
> +}
Sorry, my patch still triggered the gcc bug, I used the wrong compiler.
This patch works, and runs about the same speed as before the patch for
small inputs, and slightly faster for larger inputs (100+ bytes). I'll
look for a faster workaround if benchmarks show it matters.
Signed-off-by: Nick Terrell
---
lib/zstd/zstd_internal.h | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/lib/zstd/zstd_internal.h b/lib/zstd/zstd_internal.h
index 6748719..839014d 100644
--- a/lib/zstd/zstd_internal.h
+++ b/lib/zstd/zstd_internal.h
@@ -139,12 +139,8 @@ static void ZSTD_copy8(void *dst, const void *src) {
memcpy(dst, src, 8); }
#define WILDCOPY_OVERLENGTH 8
ZSTD_STATIC void ZSTD_wildcopy(void *dst, const void *src, ptrdiff_t length)
{
- const BYTE *ip = (const BYTE *)src;
- BYTE *op = (BYTE *)dst;
- BYTE *const oend = op + length;
- do
- COPY8(op, ip)
- while (op < oend);
+ if (length > 0)
+ memcpy(dst, src, length);
}
ZSTD_STATIC void ZSTD_wildcopy_e(void *dst, const void *src, void *dstEnd) /*
should be faster for decoding, but strangely, not verified on all platform */
--
2.9.3
Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable()
On Tue 11-07-17 09:58:42, Anshuman Khandual wrote: > On 07/10/2017 07:19 PM, Michal Hocko wrote: > > On Mon 10-07-17 16:40:59, Anshuman Khandual wrote: > >> As 'delta' is an unsigned long, 'end' (vma->vm_end + delta) cannot > >> be less than 'vma->vm_end'. > > > > This just doesn't make any sense. This is exactly what the overflow > > check is for. Maybe vm_end + delta can never overflow because of > > (old_len == vma->vm_end - addr) and guarantee old_len < new_len > > in mremap but I haven't checked that too deeply. > > Irrespective of that, just looking at the variables inside this > particular function where delta is an 'unsigned long', 'end' cannot > be less than vma->vm_end. Is not that true ? no. What happens when end is too large? [...] > > here. This is hardly something that would save many cycles in a > > relatively cold path. > > Though I have not done any detailed instruction level measurement, > there is a reduction in real and system amount of time to execute > the test with and without the patch. > > Without the patch > > real 0m2.100s > user 0m0.162s > sys 0m1.937s > > With this patch > > real 0m0.928s > user 0m0.161s > sys 0m0.756s Are you telling me that two if conditions cause more than a second difference? That sounds suspicious. -- Michal Hocko SUSE Labs
[PATCH] tracing/ring_buffer: Try harder to allocate
ftrace can fail to allocate per-CPU ring buffer on systems with a large
number of CPUs coupled while large amounts of cache happening in the
page cache. Currently the ring buffer allocation doesn't retry in the VM
implementation even if direct-reclaim made some progress but still
wasn't able to find a free page. On retrying I see that the allocations
almost always succeed. The retry doesn't happen because __GFP_NORETRY is
used in the tracer to prevent the case where we might OOM, however if we
drop __GFP_NORETRY, we risk destabilizing the system if OOM killer is
triggered. To prevent this situation, use the __GFP_RETRY_MAYFAIL flag
introduced recently [1].
Tested the following still succeeds without destabilizing a system with
1GB memory.
echo 30 > /sys/kernel/debug/tracing/buffer_size_kb
[1] https://marc.info/?l=linux-mm=149820805124906=2
Cc: Alexander Duyck
Cc: Mel Gorman
Cc: Hao Lee
Cc: Vladimir Davydov
Cc: Johannes Weiner
Cc: Joonsoo Kim
Cc: Michal Hocko
Cc: Tim Murray
Cc: Ingo Molnar
Cc: Steven Rostedt
Cc: sta...@vger.kernel.org
Signed-off-by: Joel Fernandes
---
kernel/trace/ring_buffer.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 4ae268e687fe..529cc50d7243 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -1136,12 +1136,12 @@ static int __rb_allocate_pages(long nr_pages, struct
list_head *pages, int cpu)
for (i = 0; i < nr_pages; i++) {
struct page *page;
/*
-* __GFP_NORETRY flag makes sure that the allocation fails
-* gracefully without invoking oom-killer and the system is
-* not destabilized.
+* __GFP_RETRY_MAYFAIL flag makes sure that the allocation fails
+* gracefully without invoking oom-killer and the system is not
+* destabilized.
*/
bpage = kzalloc_node(ALIGN(sizeof(*bpage), cache_line_size()),
- GFP_KERNEL | __GFP_NORETRY,
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL,
cpu_to_node(cpu));
if (!bpage)
goto free_pages;
@@ -1149,7 +1149,7 @@ static int __rb_allocate_pages(long nr_pages, struct
list_head *pages, int cpu)
list_add(>list, pages);
page = alloc_pages_node(cpu_to_node(cpu),
- GFP_KERNEL | __GFP_NORETRY, 0);
+ GFP_KERNEL | __GFP_RETRY_MAYFAIL, 0);
if (!page)
goto free_pages;
bpage->page = page_address(page);
--
2.13.2.725.g09c95d1e9-goog
Re: [PATCH v10] vfio: ABI for mdev display dma-buf operation
> +/**
> + * VFIO_DEVICE_QUERY_GFX_PLANE - _IOW(VFIO_TYPE, VFIO_BASE + 14,
> + * struct vfio_device_query_gfx_plane)
> + * Return: 0 on success, -errno on failure.
> + */
> +
> +struct vfio_device_gfx_plane_info {
> + __u64 start;
> + __u64 drm_format_mod;
> + __u32 drm_format;
> + __u32 width;
> + __u32 height;
> + __u32 stride;
> + __u32 size;
> + __u32 x_pos;
> + __u32 y_pos;
> +};
Do we want keep that as separate struct? Given we now have only a
single struct using that as sub-struct it looks pointless, at least
from a API point of view. Does the driver use the struct internally?
> +
> +struct vfio_device_query_gfx_plane {
> + __u32 argsz;
> + __u32 flags;
> + struct vfio_device_gfx_plane_info plane_info;
> + __u32 plane_type;
> + __s32 fd; /* dma-buf fd */
> + __u32 plane_id;
What is plane_id?
cheers,
Gerd
Re: [GIT pull] smp/hotplug updates for 4.13
On Sun, 9 Jul 2017, Linus Torvalds wrote: > On Sun, Jul 9, 2017 at 2:07 AM, Thomas Gleixner wrote: > > > > + /* Wait for the CPU to reach CPUHP_AP_ONLINE_IDLE */ > > wait_for_completion(>done); > > + BUG_ON(!cpu_online(cpu)); > > I realize this isn't actually a new BUG_ON(), just a moved one, but > can we *please* just agree to get rid of these machine-killing > idiocies? > > The fact that it would be a bug if this triggers doesn't excuse it one > whit. BUG_ON() is simply bad. > > Either it's a "this cannot happen", in which case it should be > deleted, or it's a "if this ever happens, we really should notice it", > in which case it should probably be a > >if (WARN_ON)ONCE(..)) > return -EBUSY; > > or something. > > That routine very much has an existing error path for failure to bring > up a CPU, dammit! > > THERE IS NO EXCUSE FOR KILLING THE MACHINE THERE! Agreed. I noticed when I moved the code, but then forgot. Will queue a fix. Thanks, tglx
Re: [PATCH] tracing/ring_buffer: Try harder to allocate
On Mon 10-07-17 23:05:00, Joel Fernandes wrote:
> ftrace can fail to allocate per-CPU ring buffer on systems with a large
> number of CPUs coupled while large amounts of cache happening in the
> page cache. Currently the ring buffer allocation doesn't retry in the VM
> implementation even if direct-reclaim made some progress but still
> wasn't able to find a free page. On retrying I see that the allocations
> almost always succeed. The retry doesn't happen because __GFP_NORETRY is
> used in the tracer to prevent the case where we might OOM, however if we
> drop __GFP_NORETRY, we risk destabilizing the system if OOM killer is
> triggered. To prevent this situation, use the __GFP_RETRY_MAYFAIL flag
> introduced recently [1].
>
> Tested the following still succeeds without destabilizing a system with
> 1GB memory.
> echo 30 > /sys/kernel/debug/tracing/buffer_size_kb
>
> [1] https://marc.info/?l=linux-mm=149820805124906=2
Yes this is the correct usage of the new flag.
> Cc: Alexander Duyck
> Cc: Mel Gorman
> Cc: Hao Lee
> Cc: Vladimir Davydov
> Cc: Johannes Weiner
> Cc: Joonsoo Kim
> Cc: Michal Hocko
> Cc: Tim Murray
> Cc: Ingo Molnar
> Cc: Steven Rostedt
> Cc: sta...@vger.kernel.org
I do not think stable tag is appropriate. The new flag hasn't been
merged yet and it is not a stable material.
> Signed-off-by: Joel Fernandes
Feel free to add
Acked-by: Michal Hocko
> ---
> kernel/trace/ring_buffer.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 4ae268e687fe..529cc50d7243 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -1136,12 +1136,12 @@ static int __rb_allocate_pages(long nr_pages, struct
> list_head *pages, int cpu)
> for (i = 0; i < nr_pages; i++) {
> struct page *page;
> /*
> - * __GFP_NORETRY flag makes sure that the allocation fails
> - * gracefully without invoking oom-killer and the system is
> - * not destabilized.
> + * __GFP_RETRY_MAYFAIL flag makes sure that the allocation fails
> + * gracefully without invoking oom-killer and the system is not
> + * destabilized.
>*/
> bpage = kzalloc_node(ALIGN(sizeof(*bpage), cache_line_size()),
> - GFP_KERNEL | __GFP_NORETRY,
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL,
> cpu_to_node(cpu));
> if (!bpage)
> goto free_pages;
> @@ -1149,7 +1149,7 @@ static int __rb_allocate_pages(long nr_pages, struct
> list_head *pages, int cpu)
> list_add(>list, pages);
>
> page = alloc_pages_node(cpu_to_node(cpu),
> - GFP_KERNEL | __GFP_NORETRY, 0);
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL, 0);
> if (!page)
> goto free_pages;
> bpage->page = page_address(page);
> --
> 2.13.2.725.g09c95d1e9-goog
--
Michal Hocko
SUSE Labs
Re: [PATCH v10] vfio: ABI for mdev display dma-buf operation
Hi,
> > +struct vfio_device_query_gfx_plane {
> > + __u32 argsz;
> > + __u32 flags;
> > + struct vfio_device_gfx_plane_info plane_info;
> > + __u32 plane_type;
> > + __s32 fd; /* dma-buf fd */
> > + __u32 plane_id;
> > +};
> > +
>
> It would be better to have comment here about what are expected
> values
> for plane_type and plane_id.
plane_type is DRM_PLANE_TYPE_*.
yes, a comment saying so would be good, same for drm_format which is
DRM_FORMAT_*. While looking at these two: renaming plane_type to
drm_plane_type (for consistency) is probably a good idea too.
plane_id needs a specification.
cheers,
Gerd
Re: [PATCH] scripts/package: snap-pkg target
On Mon, Jul 10, 2017 at 6:46 PM, Jim Davis wrote: >> >> For more information on snap packages: https://snapcraft.io/docs/ > > Is this something a user could do with the existing deb target, and > then running deb2snap? No, the kernel snap is a special case, it requires communication with the store (e.g. to download the core snap) and some extra steps, therefore you should use snapcraft and abstract from these details. -- bye, p.
Re: [PATCH] tracing/ring_buffer: Try harder to allocate
On 07/11/2017 08:05 AM, Joel Fernandes wrote:
> ftrace can fail to allocate per-CPU ring buffer on systems with a large
> number of CPUs coupled while large amounts of cache happening in the
> page cache. Currently the ring buffer allocation doesn't retry in the VM
> implementation even if direct-reclaim made some progress but still
> wasn't able to find a free page. On retrying I see that the allocations
> almost always succeed. The retry doesn't happen because __GFP_NORETRY is
> used in the tracer to prevent the case where we might OOM, however if we
> drop __GFP_NORETRY, we risk destabilizing the system if OOM killer is
> triggered. To prevent this situation, use the __GFP_RETRY_MAYFAIL flag
> introduced recently [1].
>
> Tested the following still succeeds without destabilizing a system with
> 1GB memory.
> echo 30 > /sys/kernel/debug/tracing/buffer_size_kb
>
> [1] https://marc.info/?l=linux-mm=149820805124906=2
>
> Cc: Alexander Duyck
> Cc: Mel Gorman
> Cc: Hao Lee
> Cc: Vladimir Davydov
> Cc: Johannes Weiner
> Cc: Joonsoo Kim
> Cc: Michal Hocko
> Cc: Tim Murray
> Cc: Ingo Molnar
> Cc: Steven Rostedt
> Cc: sta...@vger.kernel.org
Not stable, as Michal mentioned.
Acked-by: Vlastimil Babka
> Signed-off-by: Joel Fernandes
> ---
> kernel/trace/ring_buffer.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index 4ae268e687fe..529cc50d7243 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -1136,12 +1136,12 @@ static int __rb_allocate_pages(long nr_pages, struct
> list_head *pages, int cpu)
> for (i = 0; i < nr_pages; i++) {
> struct page *page;
> /*
> - * __GFP_NORETRY flag makes sure that the allocation fails
> - * gracefully without invoking oom-killer and the system is
> - * not destabilized.
> + * __GFP_RETRY_MAYFAIL flag makes sure that the allocation fails
> + * gracefully without invoking oom-killer and the system is not
> + * destabilized.
>*/
> bpage = kzalloc_node(ALIGN(sizeof(*bpage), cache_line_size()),
> - GFP_KERNEL | __GFP_NORETRY,
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL,
> cpu_to_node(cpu));
> if (!bpage)
> goto free_pages;
> @@ -1149,7 +1149,7 @@ static int __rb_allocate_pages(long nr_pages, struct
> list_head *pages, int cpu)
> list_add(>list, pages);
>
> page = alloc_pages_node(cpu_to_node(cpu),
> - GFP_KERNEL | __GFP_NORETRY, 0);
> + GFP_KERNEL | __GFP_RETRY_MAYFAIL, 0);
> if (!page)
> goto free_pages;
> bpage->page = page_address(page);
>
[PATCH] crypto: caam - free qman_fq after kill_fq
kill_fq removes a complete frame queue, it needs to free the qman_fq in the last. Else kmemleak will report the below warning: unreferenced object 0x800073085c80 (size 128): comm "cryptomgr_test", pid 199, jiffies 4294937850 (age 67.840s) hex dump (first 32 bytes): 00 00 00 00 00 00 00 00 a0 80 7e 00 00 80 ff ff 00 00 00 00 00 00 00 00 04 00 04 00 5c 01 00 00 backtrace: [] create_object+0xf8/0x258 [] kmemleak_alloc+0x58/0xa0 [] kmem_cache_alloc_trace+0x2c8/0x358 [] create_caam_req_fq+0x40/0x170 [] caam_drv_ctx_update+0x54/0x248 [] aead_setkey+0x154/0x300 [] setkey+0x50/0xf0 [] __test_aead+0x5ec/0x1028 [] test_aead+0x44/0xc8 [] alg_test_aead+0x58/0xd0 [] alg_test+0x14c/0x308 [] cryptomgr_test+0x50/0x58 [] kthread+0xdc/0xf0 [] ret_from_fork+0x10/0x50 And check where the function kill_fq() is called to remove the additional kfree to qman_fq. Signed-off-by: Xulin Sun --- drivers/crypto/caam/qi.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/crypto/caam/qi.c b/drivers/crypto/caam/qi.c index 1990ed4..c4b9173 100644 --- a/drivers/crypto/caam/qi.c +++ b/drivers/crypto/caam/qi.c @@ -277,6 +277,7 @@ static int kill_fq(struct device *qidev, struct qman_fq *fq) dev_err(qidev, "OOS of FQID: %u failed\n", fq->fqid); qman_destroy_fq(fq); + kfree(fq); return ret; } @@ -511,7 +512,6 @@ int caam_qi_shutdown(struct device *qidev) if (kill_fq(qidev, per_cpu(pcpu_qipriv.rsp_fq, i))) dev_err(qidev, "Rsp FQ kill failed, cpu: %d\n", i); - kfree(per_cpu(pcpu_qipriv.rsp_fq, i)); } /* -- 2.7.4
Re: [PATCH 08/17] tty: New RISC-V SBI console driver
Palmer Dabbelt writes:
>
...
> +#ifdef CONFIG_EARLY_PRINTK
> +static void sbi_console_write(struct console *co, const char *buf,
> + unsigned int n)
> +{
> + int i;
> +
> + for (i = 0; i < n; ++i) {
> + if (buf[i] == '\n')
> + sbi_console_putchar('\r');
> + sbi_console_putchar(buf[i]);
> + }
> +}
> +
> +static struct console early_console_dev __initdata = {
> + .name = "early",
> + .write = sbi_console_write,
> + .flags = CON_PRINTBUFFER | CON_BOOT,
AFAICS you could add CON_ANYTIME here, which would mean this console
would print output before the CPU is online.
I think it doesn't currently matter because you call parse_early_param()
from setup_arch(), at which point the boot CPU has been marked online.
But if this console can actually work earlier then you might be better
off just registering it unconditionally very early.
cheers
> +static int __init setup_early_printk(char *str)
> +{
> + if (early_console == NULL) {
> + early_console = _console_dev;
> + register_console(early_console);
> + }
> + return 0;
> +}
> +early_param("earlyprintk", setup_early_printk);
> +#endif
> --
> 2.13.0
Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable()
On 07/11/2017 08:03 AM, Michal Hocko wrote: > On Tue 11-07-17 09:58:42, Anshuman Khandual wrote: >>> here. This is hardly something that would save many cycles in a >>> relatively cold path. >> >> Though I have not done any detailed instruction level measurement, >> there is a reduction in real and system amount of time to execute >> the test with and without the patch. >> >> Without the patch >> >> real 0m2.100s >> user 0m0.162s >> sys 0m1.937s >> >> With this patch >> >> real 0m0.928s >> user 0m0.161s >> sys 0m0.756s > > Are you telling me that two if conditions cause more than a second > difference? That sounds suspicious. It's removing also a call to get_unmapped_area(), AFAICS. That means a vma search?
Re: [PATCH 17/17] RISC-V: Build Infastructure
Palmer Dabbelt writes:
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> new file mode 100644
> index ..366f5f2cf106
> --- /dev/null
> +++ b/arch/riscv/Kconfig
> @@ -0,0 +1,294 @@
...
> +
> +config EARLY_PRINTK
> + bool "Early printk"
> + default n
> + help
> + This option enables special console drivers which allow the kernel
> + to print messages very early in the bootup process.
> +
> + This is useful for kernel debugging when your machine crashes very
> + early before the console code is initialized. For normal operation
> + it is not recommended because it looks ugly and doesn't cooperate
> + with klogd/syslogd or the X server. You should normally N here,
> + unless you want to debug such a crash.
Looks like you copied this text from x86? I think the comment about it
being ugly refers to the VGA early console which you don't have, so
that's not really accurate.
And the part about it not cooperating with klogs/syslogd is true, early
printk output doesn't go to syslog. But if you turn EARLY_PRINTK off
then early printk output goes nowhere at all:
#ifdef CONFIG_EARLY_PRINTK
extern asmlinkage __printf(1, 2)
void early_printk(const char *fmt, ...);
#else
static inline __printf(1, 2) __cold
void early_printk(const char *s, ...) { }
#endif
So if you have a functional early console (which I think you do) then
you might be better off just making this def_bool y.
cheers
Re: [PATCH v2 04/10] arm: wire cpufreq input data for frequency-invariant accounting up to the arch
On 10-07-17, 16:13, Dietmar Eggemann wrote: > Since 'arch/$ARCH/include/asm/topology.h' now includes > 'include/linux/arch_topology.h' and 'arch/$ARCH/kernel/topology.c' > already includes 'arch/$ARCH/include/asm/topology.h' I thought it's a > good idea to get rid of this include here. Ahh, makes sense. -- viresh
Re: [PATCH 2/2] phy: Add stingray usb phy driver
hi, On Friday 07 July 2017 06:37 PM, Srinath Mannam wrote: > USB phy driver for stingray USB DRDU3 and DRDU2/USB3H > phy modules > > Signed-off-by: Srinath Mannam > Reviewed-by: Ray Jui > Reviewed-by: Scott Branden > --- > drivers/phy/broadcom/Kconfig| 13 + > drivers/phy/broadcom/Makefile | 1 + > drivers/phy/broadcom/phy-bcm-stingray-usb.c | 722 > > 3 files changed, 736 insertions(+) > create mode 100644 drivers/phy/broadcom/phy-bcm-stingray-usb.c > > diff --git a/drivers/phy/broadcom/Kconfig b/drivers/phy/broadcom/Kconfig > index 37371b8..185a84b 100644 > --- a/drivers/phy/broadcom/Kconfig > +++ b/drivers/phy/broadcom/Kconfig > @@ -58,6 +58,19 @@ config PHY_NS2_USB_DRD > > If unsure, say N. > > +config PHY_STINGRAY_USB > + tristate "Broadcom Stingray USB2 DRD PHY driver" > + depends on OF && (ARCH_BCM_IPROC || COMPILE_TEST) > + select GENERIC_PHY > + default ARCH_BCM_IPROC > + help > + Enable this to support the Broadcom Stingray USB PHY for DRDU3, > + DRDU2 and USB3H. > + This driver initializes the PHY which can be used in both > + Host and Device modes. > + > + If unsure, say N. > + > config PHY_BRCM_SATA > tristate "Broadcom SATA PHY driver" > depends on ARCH_BRCMSTB || ARCH_BCM_IPROC || BMIPS_GENERIC || > COMPILE_TEST > diff --git a/drivers/phy/broadcom/Makefile b/drivers/phy/broadcom/Makefile > index 4eb82ec..78c1033 100644 > --- a/drivers/phy/broadcom/Makefile > +++ b/drivers/phy/broadcom/Makefile > @@ -4,4 +4,5 @@ obj-$(CONFIG_PHY_BCM_NS_USB2) += phy-bcm-ns-usb2.o > obj-$(CONFIG_PHY_BCM_NS_USB3)+= phy-bcm-ns-usb3.o > obj-$(CONFIG_PHY_NS2_PCIE) += phy-bcm-ns2-pcie.o > obj-$(CONFIG_PHY_NS2_USB_DRD)+= phy-bcm-ns2-usbdrd.o > +obj-$(CONFIG_PHY_STINGRAY_USB) += phy-bcm-stingray-usb.o > obj-$(CONFIG_PHY_BRCM_SATA) += phy-brcm-sata.o > diff --git a/drivers/phy/broadcom/phy-bcm-stingray-usb.c > b/drivers/phy/broadcom/phy-bcm-stingray-usb.c > new file mode 100644 > index 000..03603fd > --- /dev/null > +++ b/drivers/phy/broadcom/phy-bcm-stingray-usb.c > @@ -0,0 +1,722 @@ > +/* > + * Copyright (C) 2017 Broadcom > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public License as > + * published by the Free Software Foundation version 2. > + * > + * This program is distributed "as is" WITHOUT ANY WARRANTY of any > + * kind, whether express or implied; without even the implied warranty > + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > + > +#define DRDU2_U2PLL_NDIV_FRAC 0x0 > + > +#define DRDU2_U2PLL_NDIV_INT0x4 > + > +#define DRDU2_U2PLL_CTRL0x8 > +#define DRDU2_U2PLL_LOCKBIT(6) > +#define DRDU2_U2PLL_RESETB BIT(5) > +#define DRDU2_U2PLL_PDIV_MASK 0xF > +#define DRDU2_U2PLL_PDIV_OFFSET 1 > +#define DRDU2_U2PLL_SUSPEND_EN BIT(0) > + > +#define DRDU2_PHY_CTRL 0x0C > +#define DRDU2_U2IDDQBIT(30) > +#define DRDU2_U2SOFT_RST_N BIT(29) > +#define DRDU2_U2PHY_ON_FLAG BIT(22) > +#define DRDU2_U2PHY_PCTL_MASK 0x > +#define DRDU2_U2PHY_PCTL_OFFSET 6 > +#define DRDU2_U2PHY_RESETB BIT(5) > +#define DRDU2_U2PHY_ISO BIT(4) > +#define DRDU2_U2AFE_BG_PWRDWNB BIT(3) > +#define DRDU2_U2AFE_PLL_PWRDWNB BIT(2) > +#define DRDU2_U2AFE_LDO_PWRDWNB BIT(1) > +#define DRDU2_U2CTRL_CORERDYBIT(0) > + > +#define DRDU2_STRAP_CTRL0x18 > +#define DRDU2_FORCE_HOST_MODE BIT(5) > +#define DRDU2_FORCE_DEVICE_MODE BIT(4) > +#define BDC_USB_STP_SPD_MASK0x7 > +#define BDC_USB_STP_SPD_OFFSET 0 > + > +#define DRDU2_PWR_CTRL 0x1C > +#define DRDU2_U2PHY_DFE_SWITCH_PWROKIN_IBIT(2) > +#define DRDU2_U2PHY_DFE_SWITCH_PWRONIN_IBIT(1) > + > +#define DRDU2_SOFT_RESET_CTRL 0x20 > +#define DRDU2_BDC_AXI_SOFT_RST_NBIT(0) > + > +#define USB3H_U2PLL_NDIV_FRAC 0x4 > + > +#define USB3H_U2PLL_NDIV_INT0x8 > + > +#define USB3H_U2PLL_CTRL0xC > +#define USB3H_U2PLL_LOCKBIT(6) > +#define USB3H_U2PLL_RESETB BIT(5) > +#define USB3H_U2PLL_PDIV_MASK 0xF > +#define USB3H_U2PLL_PDIV_OFFSET 1 > + > +#define USB3H_U2PHY_CTRL0x10 > +#define USB3H_U2PHY_ON_FLAG 22 > +#define USB3H_U2PHY_PCTL_MASK 0x > +#define USB3H_U2PHY_PCTL_OFFSET 6 > +#define USB3H_U2PHY_RESETB BIT(5) > +#define USB3H_U2PHY_ISO BIT(4) >
Re: [PATCH v2 02/10] cpufreq: provide data for frequency-invariant load-tracking support
On 10-07-17, 14:46, Rafael J. Wysocki wrote: > This particular change is about a new feature, so making it in the core is OK > in two cases IMO: (a) when you actively want everyone to be affected by it and IMO this change should be done for the whole ARM architecture. And if some regression happens due to this, then we come back and solve it. > (b) when the effect of it on the old systems should not be noticeable. I am not sure about the effects of this on performance really. @Dietmar: Any inputs for that ? -- viresh
[PATCH v2] printk: Modify operators of printed_len and text_len
With commit ("printk: report lost messages in printk
safe/nmi contexts") and commit <8b1742c9c207> ("printk: remove zap_locks()
function"), it seems we can remove initialization, "=0", of text_len and
directly assign result of log_output to printed_len.
Signed-off-by: Pierre Kuo
Reviewed-by: Sergey Senozhatsky
---
Changes since v2:
* Per Petr's friendly reminder, add description of ("printk:
report lost messages in printk
safe/nmi contexts") and remove "=0", the initialization, of text_len.
kernel/printk/printk.c |6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index fc47863..229fbdcb 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1698,10 +1698,10 @@ asmlinkage int vprintk_emit(int facility, int level,
{
static char textbuf[LOG_LINE_MAX];
char *text = textbuf;
- size_t text_len = 0;
+ size_t text_len;
enum log_flags lflags = 0;
unsigned long flags;
- int printed_len = 0;
+ int printed_len;
bool in_sched = false;
if (level == LOGLEVEL_SCHED) {
@@ -1754,7 +1754,7 @@ asmlinkage int vprintk_emit(int facility, int level,
if (dict)
lflags |= LOG_PREFIX|LOG_NEWLINE;
- printed_len += log_output(facility, level, lflags, dict, dictlen, text,
text_len);
+ printed_len = log_output(facility, level, lflags, dict, dictlen, text,
text_len);
logbuf_unlock_irqrestore(flags);
--
1.7.9.5
Re: [PATCH] gpu: host1x: Free the IOMMU domain when there is no device to attach
Thanks for the patch, didn't consider this case. I really need to get
together some system to automatically test on multiple platforms.. :)
Reviewed-by: Mikko Perttunen
On 10.07.2017 22:33, Paul Kocialkowski wrote:
When there is no device to attach to the IOMMU domain, as may be the
case when the device-tree does not contain the proper iommu node, it is
best to keep going without IOMMU support rather than failing.
This allows the driver to probe and function instead of taking down
all of the tegra drm driver, leading to missing display support.
Signed-off-by: Paul Kocialkowski
---
drivers/gpu/host1x/dev.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
index ac65f52850a6..f296738d0de8 100644
--- a/drivers/gpu/host1x/dev.c
+++ b/drivers/gpu/host1x/dev.c
@@ -186,8 +186,13 @@ static int host1x_probe(struct platform_device *pdev)
return -ENOMEM;
err = iommu_attach_device(host->domain, >dev);
- if (err)
+ if (err == -ENODEV) {
+ iommu_domain_free(host->domain);
+ host->domain = NULL;
+ goto iommu_skip;
+ } else if (err) {
goto fail_free_domain;
+ }
geometry = >domain->geometry;
@@ -198,6 +203,7 @@ static int host1x_probe(struct platform_device *pdev)
host->iova_end = geometry->aperture_end;
}
+iommu_skip:
err = host1x_channel_list_init(host);
if (err) {
dev_err(>dev, "failed to initialize channel list\n");
Re: [PATCH v2] mrf24j40: Fix en error handling path in 'mrf24j40_probe()'
Hi Christophe, > If this check fails, we must release some resources as done everywhere > else in this function before returning an error code. > > Signed-off-by: Christophe JAILLET > --- > V2: initialization of ret in this erro path ws missing. Stupid me! > --- > drivers/net/ieee802154/mrf24j40.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) patch has been applied to bluetooth-next tree. Regards Marcel
Re: [PATCH v8 05/10] mm: thp: enable thp migration in generic path
On Sat, Jul 01, 2017 at 09:40:03AM -0400, Zi Yan wrote:
> From: Zi Yan
>
> This patch adds thp migration's core code, including conversions
> between a PMD entry and a swap entry, setting PMD migration entry,
> removing PMD migration entry, and waiting on PMD migration entries.
>
> This patch makes it possible to support thp migration.
> If you fail to allocate a destination page as a thp, you just split
> the source thp as we do now, and then enter the normal page migration.
> If you succeed to allocate destination thp, you enter thp migration.
> Subsequent patches actually enable thp migration for each caller of
> page migration by allowing its get_new_page() callback to
> allocate thps.
>
> ChangeLog v1 -> v2:
> - support pte-mapped thp, doubly-mapped thp
>
> Signed-off-by: Naoya Horiguchi
>
> ChangeLog v2 -> v3:
> - use page_vma_mapped_walk()
> - use pmdp_huge_clear_flush() instead of pmdp_huge_get_and_clear() in
> set_pmd_migration_entry()
>
> ChangeLog v3 -> v4:
> - factor out the code of removing pte pgtable page in zap_huge_pmd()
>
> ChangeLog v4 -> v5:
> - remove unnecessary PTE-mapped THP code in remove_migration_pmd()
> and set_pmd_migration_entry()
> - restructure the code in zap_huge_pmd() to avoid factoring out
> the pte pgtable page code
> - in zap_huge_pmd(), check that PMD swap entries are migration entries
> - change author information
>
> ChangeLog v5 -> v7
> - use macro to disable the code when thp migration is not enabled
>
> ChangeLog v7 -> v8
> - use IS_ENABLED instead of macro to make code look clean in
> zap_huge_pmd() and page_vma_mapped_walk()
> - remove BUILD_BUG() in pmd_to_swp_entry() and swp_entry_to_pmd() to
> avoid compilation error
> - rename variable 'migration' to 'flush_needed' and invert the logic in
> zap_huge_pmd() to make code more descriptive
> - use pmdp_invalidate() in set_pmd_migration_entry() to avoid race
> with MADV_DONTNEED
> - remove unnecessary tlb flush in remove_migration_pmd()
> - add the missing migration flag check in page_vma_mapped_walk()
>
> Signed-off-by: Zi Yan
> Cc: Kirill A. Shutemov
> ---
> arch/x86/include/asm/pgtable_64.h | 2 +
> include/linux/swapops.h | 67 ++-
> mm/huge_memory.c | 84
> ---
> mm/migrate.c | 32 ++-
> mm/page_vma_mapped.c | 18 +++--
> mm/pgtable-generic.c | 3 +-
> mm/rmap.c | 13 ++
> 7 files changed, 207 insertions(+), 12 deletions(-)
>
...
> diff --git a/mm/rmap.c b/mm/rmap.c
> index 91948fbbb0bb..b28f633cd569 100644
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -1302,6 +1302,7 @@ static bool try_to_unmap_one(struct page *page, struct
> vm_area_struct *vma,
> bool ret = true;
> enum ttu_flags flags = (enum ttu_flags)arg;
>
> +
> /* munlock has nothing to gain from examining un-locked vmas */
> if ((flags & TTU_MUNLOCK) && !(vma->vm_flags & VM_LOCKED))
> return true;
> @@ -1312,6 +1313,18 @@ static bool try_to_unmap_one(struct page *page, struct
> vm_area_struct *vma,
> }
>
> while (page_vma_mapped_walk()) {
> +#ifdef CONFIG_ARCH_ENABLE_THP_MIGRATION
> + /* PMD-mapped THP migration entry */
> + if (flags & TTU_MIGRATION) {
My testing based on mmotm-2017-07-06-16-18 showed that migrating shmem thp
caused kernel crash. I don't think this is critical because that case is
just not-prepared yet. So in order to avoid the crash, please add
PageAnon(page) check here. This makes shmem thp migration just fail.
+ if (!PageAnon(page))
+ continue;
> + if (!pvmw.pte && page) {
Just from curiosity, do we really need this page check?
try_to_unmap() always passes down the parameter 'page' to try_to_unmap_one()
via rmap_walk_* family, so I think we can assume page is always non-NULL.
Thanks,
Naoya Horiguchi
> + VM_BUG_ON_PAGE(PageHuge(page) ||
> !PageTransCompound(page),
> + page);
> + set_pmd_migration_entry(, page);
> + continue;
> + }
> + }
> +#endif
> +
> /*
>* If the page is mlock()d, we cannot swap it out.
>* If it's recently referenced (perhaps page_referenced
> --
> 2.11.0
>
>
Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable()
On Tue 11-07-17 08:26:40, Vlastimil Babka wrote: > On 07/11/2017 08:03 AM, Michal Hocko wrote: > > On Tue 11-07-17 09:58:42, Anshuman Khandual wrote: > >>> here. This is hardly something that would save many cycles in a > >>> relatively cold path. > >> > >> Though I have not done any detailed instruction level measurement, > >> there is a reduction in real and system amount of time to execute > >> the test with and without the patch. > >> > >> Without the patch > >> > >> real 0m2.100s > >> user 0m0.162s > >> sys0m1.937s > >> > >> With this patch > >> > >> real 0m0.928s > >> user 0m0.161s > >> sys0m0.756s > > > > Are you telling me that two if conditions cause more than a second > > difference? That sounds suspicious. > > It's removing also a call to get_unmapped_area(), AFAICS. That means a > vma search? Ohh, right. I have somehow missed that. Is this removal intentional? The changelog is silent about it. -- Michal Hocko SUSE Labs
Re: [PATCH 1/3] mfd: Add support for FTDI FT232H devices
On Mon, 10 Jul 2017 14:34:27 +0200
Johan Hovold jo...@kernel.org wrote:
>On Fri, Jul 07, 2017 at 11:53:29AM +0200, Anatolij Gustschin wrote:
>> On Fri, 07 Jul 2017 09:48:48 +0200
>> Bjørn Mork bj...@mork.no wrote:
>>
>> >[adding Johan on the CC list]
>> >
>> >Anatolij Gustschin writes:
>> >
>> >> +static struct usb_device_id ftdi_mfd_table[] = {
>> >> + { USB_DEVICE(0x0403, 0x6014) },
>> >> + {}
>> >> +};
>> >> +MODULE_DEVICE_TABLE(usb, ftdi_mfd_table);
>> >
>> >This device ID is currently handled by the ftdi_sio driver, so I believe
>> >you at least have to explain how you intend these two drivers to
>> >cooperate...
>>
>> these drivers cannot cooperate, the different ftdi function modes
>> use same device pins as in UART mode. So, you either can use the
>> device in UART interface mode or in some different mode. I do not
>> load the ftdi_sio module or do unbind the USB device from the
>> ftdio_sio driver and bind it to the mfd driver, e.g.:
>>
>> sh -c "echo -n "3-2:1.0" > /sys/bus/usb/drivers/ftdi_sio/unbind"
>> sh -c "echo -n "3-2:1.0" > /sys/bus/usb/drivers/ftdi-mfd/bind"
>
>I'm afraid that's not good enough. If we're going to support a non-UART
>mode through a separate driver, we need to have all drivers for these
>devices be able to retrieve the current mode during probe and only bind
>when the mode matches.
Can we reliably retrieve the current mode? For devices with connected
EEPROM some modes (including UART) are configurable in the EEPROM. For
devices without EEPROM the default mode is always UART, but FIFO-,
Bitbang- and MPSSE-mode can be switched via commands to the the chip.
Anatolij
[RESEND PATCH] ARM: dts: stm32: Add DMA support for STM32F746 SoC
This patch adds DMA support for STM32F746 SoC.
Signed-off-by: Pierre-Yves MORDRET
---
arch/arm/boot/dts/stm32f746.dtsi | 31 +++
1 file changed, 31 insertions(+)
diff --git a/arch/arm/boot/dts/stm32f746.dtsi b/arch/arm/boot/dts/stm32f746.dtsi
index c2765ce..bfffea4 100644
--- a/arch/arm/boot/dts/stm32f746.dtsi
+++ b/arch/arm/boot/dts/stm32f746.dtsi
@@ -344,6 +344,37 @@
assigned-clocks = < 1 CLK_HSE_RTC>;
assigned-clock-rates = <100>;
};
+
+ dma1: dma@40026000 {
+ compatible = "st,stm32-dma";
+ reg = <0x40026000 0x400>;
+ interrupts = <11>,
+<12>,
+<13>,
+<14>,
+<15>,
+<16>,
+<17>,
+<47>;
+ clocks = < 0 21>;
+ #dma-cells = <4>;
+ };
+
+ dma2: dma@40026400 {
+ compatible = "st,stm32-dma";
+ reg = <0x40026400 0x400>;
+ interrupts = <56>,
+<57>,
+<58>,
+<59>,
+<60>,
+<68>,
+<69>,
+<70>;
+ clocks = < 0 22>;
+ #dma-cells = <4>;
+ st,mem2mem;
+ };
};
};
--
2.7.4
Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable()
On 07/11/2017 08:50 AM, Michal Hocko wrote:
> On Tue 11-07-17 08:26:40, Vlastimil Babka wrote:
>> On 07/11/2017 08:03 AM, Michal Hocko wrote:
>>>
>>> Are you telling me that two if conditions cause more than a second
>>> difference? That sounds suspicious.
>>
>> It's removing also a call to get_unmapped_area(), AFAICS. That means a
>> vma search?
>
> Ohh, right. I have somehow missed that. Is this removal intentional?
I think it is: "Checking for availability of virtual address range at
the end of the VMA for the incremental size is also reduntant at this
point."
> The
> changelog is silent about it.
It doesn't explain why it's redundant, indeed. Unfortunately, the commit
f106af4e90ea ("fix checks for expand-in-place mremap") which added this,
also doesn't explain why it's needed.
Re: [GIT pull] irq updates for 4.13
On Mon, 10 Jul 2017, Linus Torvalds wrote: > On Mon, Jul 10, 2017 at 6:35 AM, Sebastian Reichel > wrote: > > > > This patch apparently breaks OMAP platform: > > > > 46e48e257360f0845fe17089713cbad4db611e70 is the first bad commit > > commit 46e48e257360f0845fe17089713cbad4db611e70 > > Author: Thomas Gleixner > > Date: Thu Jun 29 23:33:38 2017 +0200 > > > > genirq: Move irq resource handling out of spinlocked region > > > > Boot failure log from Droid 4: > > [ ... snip snip ..] > > > > Droid 4 boots current master again after applying the patch below > > (which is git revet of above patch, but I provide the patch, since > > it did not revet cleanly). > > Hmm. Do you actually need the full revert? > > I think it's only the __setup_irq() part that looks like it may be garbage. > > For example, I think it releases the resources twice if the > __irq_set_trigger() call fails. Yes, I missed that. Sorry. > But it looks questionably in other ways too - notably, the change to > make the request call be in the same context as the freeing is done is > apparently done entirely for symmetry reasons, not for any actual > *reason* reasons. There is a reasons reason. The whole purpose was to move out the request/free resources call from the spinlocked and irq disabled reason. I noticed the free ordering issue, when I was working on that. The fact that the patch breaks the OMAP boot, points to something else. The only user of the irq_request_resources() callback at the moment is the GPIO subsystem and some pinctrl drivers, which are not involved in the OMAP case. In case of OMAP it uses the gpiolib generic implementation which does: try_module_get(chip->gpiodev->owner); gpiochip_lock_as_irq(chip, d->hwirq); I have no idea at the moment why this would break anything. The double release in the __irq_set_trigger() error path is the only issue I can find there. Sebastian, can you please provide a .config and a full boot log, preferably with initcall_debug on the kernel command line? Thanks, tglx
[RESEND PATCH] ARM: dts: stm32: Add DMAMUX support for STM32H743 SoC
This patch adds DMAMUX support for STM32H743 SoC.
Signed-off-by: Pierre-Yves MORDRET
---
arch/arm/boot/dts/stm32h743.dtsi | 20
1 file changed, 20 insertions(+)
diff --git a/arch/arm/boot/dts/stm32h743.dtsi b/arch/arm/boot/dts/stm32h743.dtsi
index 3d1cd88..0e24a4c 100644
--- a/arch/arm/boot/dts/stm32h743.dtsi
+++ b/arch/arm/boot/dts/stm32h743.dtsi
@@ -106,6 +106,26 @@
#dma-cells = <4>;
st,mem2mem;
};
+
+ dmamux1: dma-router@40020800 {
+ compatible = "st,stm32h7-dmamux";
+ reg = <0x40020800 0x1c>;
+ #dma-cells = <3>;
+ dma-channels = <8>;
+ dma-requests = <128>;
+ dma-masters = <>;
+ clocks = <_clk>;
+ };
+
+ dmamux2: dma-router@40020820 {
+ compatible = "st,stm32h7-dmamux";
+ reg = <0x40020820 0x1c>;
+ #dma-cells = <3>;
+ dma-channels = <8>;
+ dma-requests = <128>;
+ dma-masters = <>;
+ clocks = <_clk>;
+ };
};
};
--
2.7.4
[RESEND PATCH] ARM: dts: stm32: Add DMA support for STM32H743 SoC
This patch adds DMA support for STM32H743 SoC.
Signed-off-by: Pierre-Yves MORDRET
---
arch/arm/boot/dts/stm32h743.dtsi | 32
1 file changed, 32 insertions(+)
diff --git a/arch/arm/boot/dts/stm32h743.dtsi b/arch/arm/boot/dts/stm32h743.dtsi
index 4685629..3d1cd88 100644
--- a/arch/arm/boot/dts/stm32h743.dtsi
+++ b/arch/arm/boot/dts/stm32h743.dtsi
@@ -74,6 +74,38 @@
interrupts = <50>;
clocks = <_clk>;
};
+
+ dma1: dma@4002 {
+ compatible = "st,stm32-dma";
+ reg = <0x4002 0x400>;
+ interrupts = <11>,
+<12>,
+<13>,
+<14>,
+<15>,
+<16>,
+<17>,
+<47>;
+ clocks = <_clk>;
+ #dma-cells = <4>;
+ st,mem2mem;
+ };
+
+ dma2: dma@40020400 {
+ compatible = "st,stm32-dma";
+ reg = <0x40020400 0x400>;
+ interrupts = <56>,
+<57>,
+<58>,
+<59>,
+<60>,
+<68>,
+<69>,
+<70>;
+ clocks = <_clk>;
+ #dma-cells = <4>;
+ st,mem2mem;
+ };
};
};
--
2.7.4
Re: [RFC PATCH] mm, oom: allow oom reaper to race with exit_mmap
On Mon 10-07-17 16:55:22, David Rientjes wrote:
> On Mon, 26 Jun 2017, Michal Hocko wrote:
>
> > diff --git a/mm/mmap.c b/mm/mmap.c
> > index 3bd5ecd20d4d..253808e716dc 100644
> > --- a/mm/mmap.c
> > +++ b/mm/mmap.c
> > @@ -2962,6 +2962,11 @@ void exit_mmap(struct mm_struct *mm)
> > /* Use -1 here to ensure all VMAs in the mm are unmapped */
> > unmap_vmas(, vma, 0, -1);
> >
> > + /*
> > +* oom reaper might race with exit_mmap so make sure we won't free
> > +* page tables or unmap VMAs under its feet
> > +*/
> > + down_write(>mmap_sem);
> > free_pgtables(, vma, FIRST_USER_ADDRESS, USER_PGTABLES_CEILING);
> > tlb_finish_mmu(, 0, -1);
> >
> > @@ -2974,7 +2979,9 @@ void exit_mmap(struct mm_struct *mm)
> > nr_accounted += vma_pages(vma);
> > vma = remove_vma(vma);
> > }
> > + mm->mmap = NULL;
> > vm_unacct_memory(nr_accounted);
> > + up_write(>mmap_sem);
> > }
> >
> > /* Insert vm structure into process list sorted by address
> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > index 0e2c925e7826..5dc0ff22d567 100644
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -472,36 +472,8 @@ static bool __oom_reap_task_mm(struct task_struct
> > *tsk, struct mm_struct *mm)
> > struct vm_area_struct *vma;
> > bool ret = true;
> >
> > - /*
> > -* We have to make sure to not race with the victim exit path
> > -* and cause premature new oom victim selection:
> > -* __oom_reap_task_mm exit_mm
> > -* mmget_not_zero
> > -*mmput
> > -* atomic_dec_and_test
> > -*exit_oom_victim
> > -* [...]
> > -* out_of_memory
> > -*select_bad_process
> > -* # no TIF_MEMDIE task selects new
> > victim
> > -* unmap_page_range # frees some memory
> > -*/
> > - mutex_lock(_lock);
> > -
> > - if (!down_read_trylock(>mmap_sem)) {
> > - ret = false;
> > - goto unlock_oom;
> > - }
> > -
> > - /*
> > -* increase mm_users only after we know we will reap something so
> > -* that the mmput_async is called only when we have reaped something
> > -* and delayed __mmput doesn't matter that much
> > -*/
> > - if (!mmget_not_zero(mm)) {
> > - up_read(>mmap_sem);
> > - goto unlock_oom;
> > - }
> > + if (!down_read_trylock(>mmap_sem))
> > + return false;
>
> I think this should return true if mm->mmap == NULL here.
This?
---
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index 5dc0ff22d567..e155d1d8064f 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -470,11 +470,14 @@ static bool __oom_reap_task_mm(struct task_struct *tsk,
struct mm_struct *mm)
{
struct mmu_gather tlb;
struct vm_area_struct *vma;
- bool ret = true;
if (!down_read_trylock(>mmap_sem))
return false;
+ /* There is nothing to reap so bail out without signs in the log */
+ if (!mm->mmap)
+ goto unlock;
+
/*
* Tell all users of get_user/copy_from_user etc... that the content
* is no longer stable. No barriers really needed because unmapping
@@ -508,9 +511,10 @@ static bool __oom_reap_task_mm(struct task_struct *tsk,
struct mm_struct *mm)
K(get_mm_counter(mm, MM_ANONPAGES)),
K(get_mm_counter(mm, MM_FILEPAGES)),
K(get_mm_counter(mm, MM_SHMEMPAGES)));
+unlock:
up_read(>mmap_sem);
- return ret;
+ return true;
}
#define MAX_OOM_REAP_RETRIES 10
--
Michal Hocko
SUSE Labs
Re: [PATCH] drm/udl: Make page_flip asynchronous
On Fri, Jul 7, 2017 at 7:48 AM, Dawid Kurek wrote: > In page_flip vblank is sent with no delay. Driver does not know when the > actual update is present on the display and has no means for getting > this information from a device. It is practically impossible to say > exactly *when* as there is also i.e. a usb delay. > > When we are unable to determine when the vblank actually happens we may > assume it will behave accordingly, i.e. it will present frames with > proper timing. In the worst case scenario it should take up to duration > of one frame (we may get new frame in the device just after presenting > current one so we would need to wait for the whole frame). > > Because of the asynchronous nature of the delay we need to synchronize: > * read/write vrefresh/page_flip data when changing mode and >preparing/executing vblank > * USB requests to prevent interleaved access to URBs for two different >frame buffers > > All those changes are backports from ChromeOS: > 1. https://chromium-review.googlesource.com/250622 > 2. https://chromium-review.googlesource.com/249450 > partially, only change in udl_modeset.c for 'udl_flip_queue' > 3. https://chromium-review.googlesource.com/321378 > 4. https://chromium-review.googlesource.com/324119 > + fixes for checkpatch and latest drm changes > > Cc: h...@chromium.org > Cc: marc...@chromium.org > Cc: za...@chromium.org > Cc: db...@google.com > Signed-off-by: Dawid Kurek Can't we roll this driver over to the atomic helpers instead? There you get nonblocking pretty much for free ... I'm not sure extending the old modeset code has all that much benefit really. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
[PATCH v7 0/7] perf report: Show branch type
v7:
---
Redefine the common branch types according to review comments from
Michael Ellerman
Now the patch series just defines a minimum but more common set of
branch types.
PERF_BR_NONE: unknown
PERF_BR_COND：conditional
PERF_BR_UNCOND : unconditional
PERF_BR_IND : indirect
PERF_BR_CALL: function call
PERF_BR_IND_CALL: indirect function call
PERF_BR_RET : function return
PERF_BR_SYSCALL : syscall
PERF_BR_SYSRET : syscall return
PERF_BR_COND_CALL : conditional function call
PERF_BR_COND_RET: conditional function return
v6:
---
Update according to the review comments from
Jiri Olsa . Major modifications are:
1. Move that multiline conditional code inside {} brackets.
2. Move branch_type_stat_display() from builtin-report.c to
branch.c. Move branch_type_str() from callchain.c to
branch.c.
3. Keep the original branch info display order, that is:
predicted, abort, cycles, iterations
v5:
---
Mainly the v5 patch series are updated according to
comments from Jiri Olsa .
The kernel part doesn't have functional change. It just
solve the merge issue.
In userspace, the functions of branch type counting and
branch type name resolving are moved to the new files:
util/branch.c, util/branch.h.
And refactor the branch info printing code for better
maintenance.
Not changed (or just fix merge issue):
perf/core: Define the common branch type classification
perf/x86/intel: Record branch type
perf record: Create a new option save_type in --branch-filter
New patches:
perf report: Refactor the branch info printing code
perf util: Create branch.c/.h for common branch functions
Changed:
perf report: Show branch type statistics for stdio mode
perf report: Show branch type in callchain entry
v4:
---
1. Describe the major changes in patch description.
Thanks for Peter Zijlstra's reminding.
2. Initialize branch type to 0 in intel_pmu_lbr_read_32 and
intel_pmu_lbr_read_64. Remove the invalid else code in
intel_pmu_lbr_filter.
v3:
---
1. Move the JCC forward/backward and cross page computing from
kernel to userspace.
2. Use lookup table to replace original switch/case processing.
Changed:
perf/core: Define the common branch type classification
perf/x86/intel: Record branch type
perf report: Show branch type statistics for stdio mode
perf report: Show branch type in callchain entry
Not changed:
perf record: Create a new option save_type in --branch-filter
v2:
---
1. Use 4 bits in perf_branch_entry to record branch type.
2. Pull out some common branch types from FAR_BRANCH. Now the branch
types defined in perf_event.h:
Jin Yao (7):
perf/core: Define the common branch type classification
perf/x86/intel: Record branch type
perf record: Create a new option save_type in --branch-filter
perf report: Refactor the branch info printing code
perf util: Create branch.c/.h for common branch functions
perf report: Show branch type statistics for stdio mode
perf report: Show branch type in callchain entry
arch/x86/events/intel/lbr.c | 53 +-
include/uapi/linux/perf_event.h | 27 -
tools/include/uapi/linux/perf_event.h| 27 -
tools/perf/Documentation/perf-record.txt | 1 +
tools/perf/builtin-report.c | 25 +
tools/perf/util/Build| 1 +
tools/perf/util/branch.c | 166 +++
tools/perf/util/branch.h | 25 +
tools/perf/util/callchain.c | 140 ++
tools/perf/util/callchain.h | 5 +-
tools/perf/util/event.h | 3 +-
tools/perf/util/hist.c | 5 +-
tools/perf/util/machine.c| 26 +++--
tools/perf/util/parse-branch-options.c | 1 +
14 files changed, 421 insertions(+), 84 deletions(-)
create mode 100644 tools/perf/util/branch.c
create mode 100644 tools/perf/util/branch.h
--
2.7.4
[PATCH v7 7/7] perf report: Show branch type in callchain entry
Show branch type in callchain entry. The branch type is printed
with other LBR information (such as cycles/abort/...).
For example:
perf record -g -j any,save_type
perf report --branch-history --stdio --no-children
38.50% div.c:45[.] maindiv
|
---main div.c:42 (RET CROSS_2M cycles:2)
compute_flag div.c:28 (cycles:2)
compute_flag div.c:27 (RET CROSS_2M cycles:1)
rand rand.c:28 (cycles:1)
rand rand.c:28 (RET CROSS_2M cycles:1)
__random random.c:298 (cycles:1)
__random random.c:297 (COND_BWD CROSS_2M cycles:1)
__random random.c:295 (cycles:1)
__random random.c:295 (COND_BWD CROSS_2M cycles:1)
__random random.c:295 (cycles:1)
__random random.c:295 (RET CROSS_2M cycles:9)
Change log
--
v7: Not changed.
v6: Remove the branch_type_str() since it's moved to branch.c.
v5: Rewrite the branch info print code in util/callchain.c.
v4: Comparing to previous version, the major changes are:
Signed-off-by: Jin Yao
---
tools/perf/util/callchain.c | 38 +-
tools/perf/util/callchain.h | 5 -
tools/perf/util/machine.c | 26 +-
3 files changed, 50 insertions(+), 19 deletions(-)
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index abc6908..4fff550 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -23,6 +23,7 @@
#include "sort.h"
#include "machine.h"
#include "callchain.h"
+#include "branch.h"
#define CALLCHAIN_PARAM_DEFAULT\
.mode = CHAIN_GRAPH_ABS, \
@@ -571,6 +572,11 @@ fill_node(struct callchain_node *node, struct
callchain_cursor *cursor)
call->cycles_count = cursor_node->branch_flags.cycles;
call->iter_count = cursor_node->nr_loop_iter;
call->samples_count = cursor_node->samples;
+
+ branch_type_count(>brtype_stat,
+ _node->branch_flags,
+ cursor_node->branch_from,
+ cursor_node->ip);
}
list_add_tail(>list, >val);
@@ -688,6 +694,11 @@ static enum match_result match_chain(struct
callchain_cursor_node *node,
cnode->cycles_count += node->branch_flags.cycles;
cnode->iter_count += node->nr_loop_iter;
cnode->samples_count += node->samples;
+
+ branch_type_count(>brtype_stat,
+ >branch_flags,
+ node->branch_from,
+ node->ip);
}
return MATCH_EQ;
@@ -922,7 +933,7 @@ merge_chain_branch(struct callchain_cursor *cursor,
list_for_each_entry_safe(list, next_list, >val, list) {
callchain_cursor_append(cursor, list->ip,
list->ms.map, list->ms.sym,
- false, NULL, 0, 0);
+ false, NULL, 0, 0, 0);
list_del(>list);
map__zput(list->ms.map);
free(list);
@@ -962,7 +973,7 @@ int callchain_merge(struct callchain_cursor *cursor,
int callchain_cursor_append(struct callchain_cursor *cursor,
u64 ip, struct map *map, struct symbol *sym,
bool branch, struct branch_flags *flags,
- int nr_loop_iter, int samples)
+ int nr_loop_iter, int samples, u64 branch_from)
{
struct callchain_cursor_node *node = *cursor->last;
@@ -986,6 +997,7 @@ int callchain_cursor_append(struct callchain_cursor *cursor,
memcpy(>branch_flags, flags,
sizeof(struct branch_flags));
+ node->branch_from = branch_from;
cursor->nr++;
cursor->last = >next;
@@ -1241,14 +1253,19 @@ static int count_float_printf(int index, const char
*str, float value,
static int counts_str_build(char *bf, int bfsize,
u64 branch_count, u64 predicted_count,
u64 abort_count, u64 cycles_count,
-u64 iter_count, u64 samples_count)
+u64 iter_count, u64 samples_count,
+struct branch_type_stat *brtype_stat)
{
u64 cycles;
- int printed = 0, i = 0;
+ int printed, i = 0;
if (branch_count == 0)
return scnprintf(bf, bfsize, " (calltrace)");
+ printed = branch_type_str(brtype_stat, bf, bfsize);
+ if (printed)
+ i++;
+
if (predicted_count < branch_count) {
printed +=
[PATCH v7 3/7] perf record: Create a new option save_type in --branch-filter
The option indicates the kernel to save branch type during sampling.
One example:
perf record -g --branch-filter any,save_type
Change log
--
v7: Not changed.
v6: Not changed.
v5: Not changed.
Signed-off-by: Jin Yao
---
tools/perf/Documentation/perf-record.txt | 1 +
tools/perf/util/parse-branch-options.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/tools/perf/Documentation/perf-record.txt
b/tools/perf/Documentation/perf-record.txt
index b0e9e92..9bdea04 100644
--- a/tools/perf/Documentation/perf-record.txt
+++ b/tools/perf/Documentation/perf-record.txt
@@ -332,6 +332,7 @@ following filters are defined:
- no_tx: only when the target is not in a hardware transaction
- abort_tx: only when the target is a hardware transaction abort
- cond: conditional branches
+ - save_type: save branch type during sampling in case binary is not
available later
+
The option requires at least one branch type among any, any_call, any_ret,
ind_call, cond.
diff --git a/tools/perf/util/parse-branch-options.c
b/tools/perf/util/parse-branch-options.c
index 38fd115..e71fb5f 100644
--- a/tools/perf/util/parse-branch-options.c
+++ b/tools/perf/util/parse-branch-options.c
@@ -28,6 +28,7 @@ static const struct branch_mode branch_modes[] = {
BRANCH_OPT("cond", PERF_SAMPLE_BRANCH_COND),
BRANCH_OPT("ind_jmp", PERF_SAMPLE_BRANCH_IND_JUMP),
BRANCH_OPT("call", PERF_SAMPLE_BRANCH_CALL),
+ BRANCH_OPT("save_type", PERF_SAMPLE_BRANCH_TYPE_SAVE),
BRANCH_END
};
--
2.7.4
[PATCH v7 2/7] perf/x86/intel: Record branch type
Perf already has support for disassembling the branch instruction
and using the branch type for filtering. The patch just records
the branch type in perf_branch_entry.
Before recording, the patch converts the x86 branch type to
common branch type.
Change log
--
v7: Just convert following x86 branch types to common branch types.
X86_BR_CALL -> PERF_BR_CALL
X86_BR_RET -> PERF_BR_RET
X86_BR_JCC -> PERF_BR_COND
X86_BR_JMP -> PERF_BR_UNCOND
X86_BR_IND_CALL -> PERF_BR_IND_CALL
X86_BR_ZERO_CALL -> PERF_BR_CALL
X86_BR_IND_JMP -> PERF_BR_IND
X86_BR_SYSCALL -> PERF_BR_SYSCALL
X86_BR_SYSRET-> PERF_BR_SYSRET
Others are set to PERF_BR_NONE
v6: Not changed.
v5: Just fix the merge error. No other update.
v4: Comparing to previous version, the major changes are:
1. Uses a lookup table to convert x86 branch type to common branch
type.
2. Move the JCC forward/JCC backward and cross page computing to
user space.
3. Initialize branch type to 0 in intel_pmu_lbr_read_32 and
intel_pmu_lbr_read_64
Signed-off-by: Jin Yao
---
arch/x86/events/intel/lbr.c | 53 -
1 file changed, 52 insertions(+), 1 deletion(-)
diff --git a/arch/x86/events/intel/lbr.c b/arch/x86/events/intel/lbr.c
index eb26165..5c239b1 100644
--- a/arch/x86/events/intel/lbr.c
+++ b/arch/x86/events/intel/lbr.c
@@ -109,6 +109,9 @@ enum {
X86_BR_ZERO_CALL= 1 << 15,/* zero length call */
X86_BR_CALL_STACK = 1 << 16,/* call stack */
X86_BR_IND_JMP = 1 << 17,/* indirect jump */
+
+ X86_BR_TYPE_SAVE= 1 << 18,/* indicate to save branch type */
+
};
#define X86_BR_PLM (X86_BR_USER | X86_BR_KERNEL)
@@ -510,6 +513,7 @@ static void intel_pmu_lbr_read_32(struct cpu_hw_events
*cpuc)
cpuc->lbr_entries[i].in_tx = 0;
cpuc->lbr_entries[i].abort = 0;
cpuc->lbr_entries[i].cycles = 0;
+ cpuc->lbr_entries[i].type = 0;
cpuc->lbr_entries[i].reserved = 0;
}
cpuc->lbr_stack.nr = i;
@@ -596,6 +600,7 @@ static void intel_pmu_lbr_read_64(struct cpu_hw_events
*cpuc)
cpuc->lbr_entries[out].in_tx = in_tx;
cpuc->lbr_entries[out].abort = abort;
cpuc->lbr_entries[out].cycles= cycles;
+ cpuc->lbr_entries[out].type = 0;
cpuc->lbr_entries[out].reserved = 0;
out++;
}
@@ -673,6 +678,10 @@ static int intel_pmu_setup_sw_lbr_filter(struct perf_event
*event)
if (br_type & PERF_SAMPLE_BRANCH_CALL)
mask |= X86_BR_CALL | X86_BR_ZERO_CALL;
+
+ if (br_type & PERF_SAMPLE_BRANCH_TYPE_SAVE)
+ mask |= X86_BR_TYPE_SAVE;
+
/*
* stash actual user request into reg, it may
* be used by fixup code for some CPU
@@ -926,6 +935,44 @@ static int branch_type(unsigned long from, unsigned long
to, int abort)
return ret;
}
+#define X86_BR_TYPE_MAP_MAX16
+
+static int
+common_branch_type(int type)
+{
+ int i, mask;
+ const int branch_map[X86_BR_TYPE_MAP_MAX] = {
+ PERF_BR_CALL, /* X86_BR_CALL */
+ PERF_BR_RET,/* X86_BR_RET */
+ PERF_BR_SYSCALL,/* X86_BR_SYSCALL */
+ PERF_BR_SYSRET, /* X86_BR_SYSRET */
+ PERF_BR_NONE, /* X86_BR_INT */
+ PERF_BR_NONE, /* X86_BR_IRET */
+ PERF_BR_COND, /* X86_BR_JCC */
+ PERF_BR_UNCOND, /* X86_BR_JMP */
+ PERF_BR_NONE, /* X86_BR_IRQ */
+ PERF_BR_IND_CALL, /* X86_BR_IND_CALL */
+ PERF_BR_NONE, /* X86_BR_ABORT */
+ PERF_BR_NONE, /* X86_BR_IN_TX */
+ PERF_BR_NONE, /* X86_BR_NO_TX */
+ PERF_BR_CALL, /* X86_BR_ZERO_CALL */
+ PERF_BR_NONE, /* X86_BR_CALL_STACK */
+ PERF_BR_IND,/* X86_BR_IND_JMP */
+ };
+
+ type >>= 2; /* skip X86_BR_USER and X86_BR_KERNEL */
+ mask = ~(~0 << 1);
+
+ for (i = 0; i < X86_BR_TYPE_MAP_MAX; i++) {
+ if (type & mask)
+ return branch_map[i];
+
+ type >>= 1;
+ }
+
+ return PERF_BR_NONE;
+}
+
/*
* implement actual branch filter based on user demand.
* Hardware may not exactly satisfy that request, thus
@@ -942,7 +989,8 @@ intel_pmu_lbr_filter(struct cpu_hw_events *cpuc)
bool compress = false;
/* if sampling all branches, then nothing to filter */
- if ((br_sel & X86_BR_ALL) == X86_BR_ALL)
+ if (((br_sel & X86_BR_ALL) == X86_BR_ALL) &&
+ ((br_sel & X86_BR_TYPE_SAVE) != X86_BR_TYPE_SAVE))
return;
for (i = 0; i < cpuc->lbr_stack.nr; i++) {
@@ -963,6 +1011,9 @@
[PATCH v7 6/7] perf report: Show branch type statistics for stdio mode
Show the branch type statistics at the end of perf report --stdio.
For example:
perf report --stdio
COND_FWD: 28.5%
COND_BWD: 9.4%
CROSS_4K: 0.7%
CROSS_2M: 14.1%
COND: 37.9%
UNCOND: 0.2%
IND: 6.7%
CALL: 26.5%
RET: 28.7%
SYSRET: 0.0%
The branch types are:
-
COND_FWD: conditional forward
COND_BWD: conditional backward
COND: conditional branch
UNCOND: unconditional branch
IND: indirect
CALL: function call
IND_CALL: indirect function call
RET: function return
SYSCALL: syscall
SYSRET: syscall return
COND_CALL: conditional function call
COND_RET: conditional function return
CROSS_4K and CROSS_2M:
--
They are the metrics checking for branches cross 4K or 2MB pages.
It's an approximate computing. We don't know if the area is 4K or
2MB, so always compute both.
To make the output simple, if a branch crosses 2M area, CROSS_4K
will not be incremented.
Change log
--
v7: Since the common branch type definitions are changed, some
tags/strings are updated accordingly.
v6: Remove branch_type_stat_display() since it's moved to branch.c.
v5: Remove the unnecessary sort__mode checking in
hist_iter__branch_callback().
v4: Comparing to previous version, the major changes are:
Add the computing of JCC forward/JCC backward and cross page checking
by using the from and to addresses.
Signed-off-by: Jin Yao
---
tools/perf/builtin-report.c | 25 +
tools/perf/util/hist.c | 5 +
2 files changed, 26 insertions(+), 4 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 79a33eb..9b7f107 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -38,6 +38,7 @@
#include "util/time-utils.h"
#include "util/auxtrace.h"
#include "util/units.h"
+#include "util/branch.h"
#include
#include
@@ -73,6 +74,7 @@ struct report {
u64 queue_size;
int socket_filter;
DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS);
+ struct branch_type_stat brtype_stat;
};
static int report__config(const char *var, const char *value, void *cb)
@@ -150,6 +152,22 @@ static int hist_iter__report_callback(struct
hist_entry_iter *iter,
return err;
}
+static int hist_iter__branch_callback(struct hist_entry_iter *iter,
+ struct addr_location *al __maybe_unused,
+ bool single __maybe_unused,
+ void *arg)
+{
+ struct hist_entry *he = iter->he;
+ struct report *rep = arg;
+ struct branch_info *bi;
+
+ bi = he->branch_info;
+ branch_type_count(>brtype_stat, >flags,
+ bi->from.addr, bi->to.addr);
+
+ return 0;
+}
+
static int process_sample_event(struct perf_tool *tool,
union perf_event *event,
struct perf_sample *sample,
@@ -188,6 +206,8 @@ static int process_sample_event(struct perf_tool *tool,
*/
if (!sample->branch_stack)
goto out_put;
+
+ iter.add_entry_cb = hist_iter__branch_callback;
iter.ops = _iter_branch;
} else if (rep->mem_mode) {
iter.ops = _iter_mem;
@@ -410,6 +430,9 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist
*evlist,
perf_read_values_destroy(>show_threads_values);
}
+ if (sort__mode == SORT_MODE__BRANCH)
+ branch_type_stat_display(stdout, >brtype_stat);
+
return 0;
}
@@ -943,6 +966,8 @@ int cmd_report(int argc, const char **argv)
if (has_br_stack && branch_call_mode)
symbol_conf.show_branchflag_count = true;
+ memset(_stat, 0, sizeof(struct branch_type_stat));
+
/*
* Branch mode is a tristate:
* -1 means default, so decide based on the file having branch data.
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index cf0186a..2f6c5e6 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -749,12 +749,9 @@ iter_prepare_branch_entry(struct hist_entry_iter *iter,
struct addr_location *al
}
static int
-iter_add_single_branch_entry(struct hist_entry_iter *iter,
+iter_add_single_branch_entry(struct hist_entry_iter *iter __maybe_unused,
struct addr_location *al __maybe_unused)
{
- /* to avoid calling callback function */
- iter->he = NULL;
-
return 0;
}
--
2.7.4
[PATCH v7 5/7] perf util: Create branch.c/.h for common branch functions
Create new util/branch.c and util/branch.h to contain the common
branch functions. Such as:
branch_type_count(): Count the numbers of branch types
branch_type_name() : Return the name of branch type
branch_type_stat_display(): Display branch type statistics info
branch_type_str(): Construct the branch type string.
The branch type is saved in branch_flags.
Change log
--
v7: Since the common branch type name is changed (e.g. JCC->COND),
this patch is performed the modification accordingly.
v6: Move that multiline conditional code inside {} brackets.
Move branch_type_stat_display() from builtin-report.c to
branch.c.
Move branch_type_str() from callchain.c to branch.c.
v5: It's a new patch in v5 patch series.
Signed-off-by: Jin Yao
---
tools/perf/util/Build| 1 +
tools/perf/util/branch.c | 166 +++
tools/perf/util/branch.h | 25 +++
tools/perf/util/event.h | 3 +-
4 files changed, 194 insertions(+), 1 deletion(-)
create mode 100644 tools/perf/util/branch.c
create mode 100644 tools/perf/util/branch.h
diff --git a/tools/perf/util/Build b/tools/perf/util/Build
index 79dea95..9857c38 100644
--- a/tools/perf/util/Build
+++ b/tools/perf/util/Build
@@ -93,6 +93,7 @@ libperf-y += drv_configs.o
libperf-y += units.o
libperf-y += time-utils.o
libperf-y += expr-bison.o
+libperf-y += branch.o
libperf-$(CONFIG_LIBBPF) += bpf-loader.o
libperf-$(CONFIG_BPF_PROLOGUE) += bpf-prologue.o
diff --git a/tools/perf/util/branch.c b/tools/perf/util/branch.c
new file mode 100644
index 000..0df4499
--- /dev/null
+++ b/tools/perf/util/branch.c
@@ -0,0 +1,166 @@
+#include "perf.h"
+#include "util/util.h"
+#include "util/debug.h"
+#include "util/branch.h"
+
+static bool cross_area(u64 addr1, u64 addr2, int size)
+{
+ u64 align1, align2;
+
+ align1 = addr1 & ~(size - 1);
+ align2 = addr2 & ~(size - 1);
+
+ return (align1 != align2) ? true : false;
+}
+
+#define AREA_4K4096
+#define AREA_2M(2 * 1024 * 1024)
+
+void branch_type_count(struct branch_type_stat *stat,
+ struct branch_flags *flags,
+ u64 from, u64 to)
+{
+ if (flags->type == PERF_BR_NONE || from == 0)
+ return;
+
+ stat->counts[flags->type]++;
+
+ if (flags->type == PERF_BR_COND) {
+ if (to > from)
+ stat->cond_fwd++;
+ else
+ stat->cond_bwd++;
+ }
+
+ if (cross_area(from, to, AREA_2M))
+ stat->cross_2m++;
+ else if (cross_area(from, to, AREA_4K))
+ stat->cross_4k++;
+}
+
+const char *branch_type_name(int type)
+{
+ const char *branch_names[PERF_BR_MAX] = {
+ "N/A",
+ "COND",
+ "UNCOND",
+ "IND",
+ "CALL",
+ "IND_CALL",
+ "RET",
+ "SYSCALL",
+ "SYSRET",
+ "COND_CALL",
+ "COND_RET"
+ };
+
+ if (type >= 0 && type < PERF_BR_MAX)
+ return branch_names[type];
+
+ return NULL;
+}
+
+void branch_type_stat_display(FILE *fp, struct branch_type_stat *stat)
+{
+ u64 total = 0;
+ int i;
+
+ for (i = 0; i < PERF_BR_MAX; i++)
+ total += stat->counts[i];
+
+ if (total == 0)
+ return;
+
+ fprintf(fp, "\n#");
+ fprintf(fp, "\n# Branch Statistics:");
+ fprintf(fp, "\n#");
+
+ if (stat->cond_fwd > 0) {
+ fprintf(fp, "\n%8s: %5.1f%%",
+ "COND_FWD",
+ 100.0 * (double)stat->cond_fwd / (double)total);
+ }
+
+ if (stat->cond_bwd > 0) {
+ fprintf(fp, "\n%8s: %5.1f%%",
+ "COND_BWD",
+ 100.0 * (double)stat->cond_bwd / (double)total);
+ }
+
+ if (stat->cross_4k > 0) {
+ fprintf(fp, "\n%8s: %5.1f%%",
+ "CROSS_4K",
+ 100.0 * (double)stat->cross_4k / (double)total);
+ }
+
+ if (stat->cross_2m > 0) {
+ fprintf(fp, "\n%8s: %5.1f%%",
+ "CROSS_2M",
+ 100.0 * (double)stat->cross_2m / (double)total);
+ }
+
+ for (i = 0; i < PERF_BR_MAX; i++) {
+ if (stat->counts[i] > 0)
+ fprintf(fp, "\n%8s: %5.1f%%",
+ branch_type_name(i),
+ 100.0 *
+ (double)stat->counts[i] / (double)total);
+ }
+}
+
+static int count_str_printf(int index, const char *str,
+ char *bf, int bfsize)
+{
+ int printed;
+
+ printed = scnprintf(bf, bfsize,
+ "%s%s",
+ (index) ? " " : " (", str);
+
+ return printed;
+}
+
+int branch_type_str(struct branch_type_stat *stat,
+
[PATCH v7 1/7] perf/core: Define the common branch type classification
It is often useful to know the branch types while analyzing branch
data. For example, a call is very different from a conditional branch.
Currently we have to look it up in binary while the binary may later
not be available and even the binary is available but user has to take
some time. It is very useful for user to check it directly in perf
report.
Perf already has support for disassembling the branch instruction
to get the x86 branch type.
To keep consistent on kernel and userspace and make the classification
more common, the patch adds the common branch type classification
in perf_event.h.
The patch only defines a minimum but most common set of branch types.
PERF_BR_NONE: unknown
PERF_BR_COND：conditional
PERF_BR_UNCOND : unconditional
PERF_BR_IND : indirect
PERF_BR_CALL: function call
PERF_BR_IND_CALL: indirect function call
PERF_BR_RET : function return
PERF_BR_SYSCALL : syscall
PERF_BR_SYSRET : syscall return
PERF_BR_COND_CALL : conditional function call
PERF_BR_COND_RET: conditional function return
The patch also adds a new field type (4 bits) in perf_branch_entry
to record the branch type (reserve 5 for future branch types)
Since the disassembling of branch instruction needs some overhead,
a new PERF_SAMPLE_BRANCH_TYPE_SAVE is introduced to indicate if it
needs to disassemble the branch instruction and record the branch
type.
Change log
--
v7: Just keep the most common branch types.
Others are removed.
v6: Not changed.
v5: Not changed. The v5 patch series just change the userspace.
v4: Comparing to previous version, the major changes are:
1. Remove the PERF_BR_JCC_FWD/PERF_BR_JCC_BWD, they will be
computed later in userspace.
2. Remove the "cross" field in perf_branch_entry. The cross page
computing will be done later in userspace.
Signed-off-by: Jin Yao
---
include/uapi/linux/perf_event.h | 27 ++-
tools/include/uapi/linux/perf_event.h | 27 ++-
2 files changed, 52 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index b1c0b18..fcfb98c 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -174,6 +174,8 @@ enum perf_branch_sample_type_shift {
PERF_SAMPLE_BRANCH_NO_FLAGS_SHIFT = 14, /* no flags */
PERF_SAMPLE_BRANCH_NO_CYCLES_SHIFT = 15, /* no cycles */
+ PERF_SAMPLE_BRANCH_TYPE_SAVE_SHIFT = 16, /* save branch type */
+
PERF_SAMPLE_BRANCH_MAX_SHIFT/* non-ABI */
};
@@ -198,9 +200,30 @@ enum perf_branch_sample_type {
PERF_SAMPLE_BRANCH_NO_FLAGS = 1U <<
PERF_SAMPLE_BRANCH_NO_FLAGS_SHIFT,
PERF_SAMPLE_BRANCH_NO_CYCLES= 1U <<
PERF_SAMPLE_BRANCH_NO_CYCLES_SHIFT,
+ PERF_SAMPLE_BRANCH_TYPE_SAVE=
+ 1U << PERF_SAMPLE_BRANCH_TYPE_SAVE_SHIFT,
+
PERF_SAMPLE_BRANCH_MAX = 1U << PERF_SAMPLE_BRANCH_MAX_SHIFT,
};
+/*
+ * Common flow change classification
+ */
+enum {
+ PERF_BR_NONE= 0,/* unknown */
+ PERF_BR_COND= 1,/* conditional */
+ PERF_BR_UNCOND = 2,/* unconditional */
+ PERF_BR_IND = 3,/* indirect */
+ PERF_BR_CALL= 4,/* function call */
+ PERF_BR_IND_CALL= 5,/* indirect function call */
+ PERF_BR_RET = 6,/* function return */
+ PERF_BR_SYSCALL = 7,/* syscall */
+ PERF_BR_SYSRET = 8,/* syscall return */
+ PERF_BR_COND_CALL = 9,/* conditional function call */
+ PERF_BR_COND_RET= 10, /* conditional function return */
+ PERF_BR_MAX,
+};
+
#define PERF_SAMPLE_BRANCH_PLM_ALL \
(PERF_SAMPLE_BRANCH_USER|\
PERF_SAMPLE_BRANCH_KERNEL|\
@@ -1015,6 +1038,7 @@ union perf_mem_data_src {
* in_tx: running in a hardware transaction
* abort: aborting a hardware transaction
*cycles: cycles from last branch (or 0 if not supported)
+ * type: branch type
*/
struct perf_branch_entry {
__u64 from;
@@ -1024,7 +1048,8 @@ struct perf_branch_entry {
in_tx:1,/* in transaction */
abort:1,/* transaction abort */
cycles:16, /* cycle count to last branch */
- reserved:44;
+ type:4, /* branch type */
+ reserved:40;
};
#endif /* _UAPI_LINUX_PERF_EVENT_H */
diff --git a/tools/include/uapi/linux/perf_event.h
b/tools/include/uapi/linux/perf_event.h
index b1c0b18..fcfb98c 100644
--- a/tools/include/uapi/linux/perf_event.h
+++ b/tools/include/uapi/linux/perf_event.h
@@ -174,6 +174,8 @@ enum perf_branch_sample_type_shift {
PERF_SAMPLE_BRANCH_NO_FLAGS_SHIFT = 14, /* no flags */
PERF_SAMPLE_BRANCH_NO_CYCLES_SHIFT = 15, /* no cycles
[PATCH v7 4/7] perf report: Refactor the branch info printing code
The branch info such as predicted/cycles/... are printed at the
callchain entries.
For example: perf report --branch-history --no-children --stdio
--1.07%--main div.c:39 (predicted:52.4% cycles:1 iterations:17)
main div.c:44 (predicted:52.4% cycles:1)
main div.c:42 (cycles:2)
compute_flag div.c:28 (cycles:2)
compute_flag div.c:27 (cycles:1)
rand rand.c:28 (cycles:1)
rand rand.c:28 (cycles:1)
__random random.c:298 (cycles:1)
__random random.c:297 (cycles:1)
__random random.c:295 (cycles:1)
__random random.c:295 (cycles:1)
__random random.c:295 (cycles:1)
But the current code is difficult to maintain and extend. This patch
refactors the code for easy maintenance.
Change log
--
v7: Not changed
v6: 1. Put the multiline condition code into {} brackets in
counts_str_build()
2. Keep the original display order, that is:
predicted, abort, cycles, iterations
v5: It's a new patch in v5 patch series.
Signed-off-by: Jin Yao
---
tools/perf/util/callchain.c | 106
1 file changed, 47 insertions(+), 59 deletions(-)
diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
index b4204b4..abc6908 100644
--- a/tools/perf/util/callchain.c
+++ b/tools/perf/util/callchain.c
@@ -1214,83 +1214,71 @@ int callchain_branch_counts(struct callchain_root *root,
cycles_count);
}
+static int count_pri64_printf(int index, const char *str, u64 value,
+ char *bf, int bfsize)
+{
+ int printed;
+
+ printed = scnprintf(bf, bfsize,
+ "%s%s:%" PRId64 "",
+ (index) ? " " : " (", str, value);
+
+ return printed;
+}
+
+static int count_float_printf(int index, const char *str, float value,
+ char *bf, int bfsize)
+{
+ int printed;
+
+ printed = scnprintf(bf, bfsize,
+ "%s%s:%.1f%%",
+ (index) ? " " : " (", str, value);
+
+ return printed;
+}
+
static int counts_str_build(char *bf, int bfsize,
u64 branch_count, u64 predicted_count,
u64 abort_count, u64 cycles_count,
u64 iter_count, u64 samples_count)
{
- double predicted_percent = 0.0;
- const char *null_str = "";
- char iter_str[32];
- char cycle_str[32];
- char *istr, *cstr;
u64 cycles;
+ int printed = 0, i = 0;
if (branch_count == 0)
return scnprintf(bf, bfsize, " (calltrace)");
- cycles = cycles_count / branch_count;
-
- if (iter_count && samples_count) {
- if (cycles > 0)
- scnprintf(iter_str, sizeof(iter_str),
-" iterations:%" PRId64 "",
-iter_count / samples_count);
- else
- scnprintf(iter_str, sizeof(iter_str),
-"iterations:%" PRId64 "",
-iter_count / samples_count);
- istr = iter_str;
- } else
- istr = (char *)null_str;
-
- if (cycles > 0) {
- scnprintf(cycle_str, sizeof(cycle_str),
- "cycles:%" PRId64 "", cycles);
- cstr = cycle_str;
- } else
- cstr = (char *)null_str;
-
- predicted_percent = predicted_count * 100.0 / branch_count;
+ if (predicted_count < branch_count) {
+ printed += count_float_printf(i++, "predicted",
+ predicted_count * 100.0 / branch_count,
+ bf + printed, bfsize - printed);
+ }
- if ((predicted_count == branch_count) && (abort_count == 0)) {
- if ((cycles > 0) || (istr != (char *)null_str))
- return scnprintf(bf, bfsize, " (%s%s)", cstr, istr);
- else
- return scnprintf(bf, bfsize, "%s", (char *)null_str);
+ if (abort_count) {
+ printed += count_float_printf(i++, "abort",
+ abort_count * 100.0 / branch_count,
+ bf + printed, bfsize - printed);
}
- if ((predicted_count < branch_count) && (abort_count == 0)) {
- if ((cycles > 0) || (istr != (char *)null_str))
- return scnprintf(bf, bfsize,
- " (predicted:%.1f%% %s%s)",
- predicted_percent, cstr, istr);
- else {
- return scnprintf(bf, bfsize,
- " (predicted:%.1f%%)",
- predicted_percent);
- }
+ cycles = cycles_count / branch_count;
+ if (cycles) {
+
[PATCH] KVM: VMX: Fix invalid guest state detection after task-switch emulation
From: Wanpeng Li This can be reproduced by EPT=1, unrestricted_guest=N, emulate_invalid_state=Y or EPT=0, the trace of kvm-unit-tests/taskswitch2.flat is like below, it tries to emulate invalid guest state task-switch: kvm_exit: reason TASK_SWITCH rip 0x0 info 4058 0 kvm_emulate_insn: 42000:0:0f 0b (0x2) kvm_emulate_insn: 42000:0:0f 0b (0x2) failed kvm_inj_exception: #UD (0x0) kvm_entry: vcpu 0 kvm_exit: reason TASK_SWITCH rip 0x0 info 4058 0 kvm_emulate_insn: 42000:0:0f 0b (0x2) kvm_emulate_insn: 42000:0:0f 0b (0x2) failed kvm_inj_exception: #UD (0x0) .. It appears that the task-switch emulation updates rflags (and vm86 flag) only after the segments are loaded, causing vmx->emulation_required to be set, when in fact invalid guest state emulation is not needed. This patch fixes it by updating vmx->emulation_required after the rflags (and vm86 flag) is updated in task-switch emulation. Suggested-by: Nadav Amit Cc: Paolo Bonzini Cc: Radim Krčmář Cc: Nadav Amit Signed-off-by: Wanpeng Li --- arch/x86/kvm/vmx.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c index f50cbfd..70270a2 100644 --- a/arch/x86/kvm/vmx.c +++ b/arch/x86/kvm/vmx.c @@ -6255,6 +6255,7 @@ static int handle_task_switch(struct kvm_vcpu *vcpu) * TODO: What about debug traps on tss switch? * Are we supposed to inject them and update dr6? */ + vmx->emulation_required = emulation_required(vcpu); return 1; } -- 2.7.4
Re: [PATCH RFC v4] cpufreq: schedutil: Make iowait boost more energy efficient
On 10/07/17 22:12, Joel Fernandes wrote:
> Hi Juri,
>
> On Mon, Jul 10, 2017 at 3:55 AM, Juri Lelli wrote:
> > Hi Joel,
> >
> > On 09/07/17 10:08, Joel Fernandes wrote:
[...]
> >> static void sugov_iowait_boost(struct sugov_cpu *sg_cpu, unsigned long
> >> *util,
> >> -unsigned long *max)
> >> +unsigned long *max, unsigned int flags)
> >> {
> >> unsigned long boost_util = sg_cpu->iowait_boost;
> >> unsigned long boost_max = sg_cpu->iowait_boost_max;
> >> @@ -195,7 +222,16 @@ static void sugov_iowait_boost(struct sugov_cpu
> >> *sg_cpu, unsigned long *util,
> >> *util = boost_util;
> >> *max = boost_max;
> >> }
> >> - sg_cpu->iowait_boost >>= 1;
> >> +
> >> + /*
> >> + * Incase iowait boost just happened on this CPU, don't reduce it
> >> right
> >> + * away since then the iowait boost will never increase on subsequent
> >> + * in_iowait wakeups.
> >> + */
> >> + if (flags & SCHED_CPUFREQ_IOWAIT && this_cpu_ptr(_cpu) ==
> >> sg_cpu)
> >> + return;
> >> +
> >> + sugov_decay_iowait_boost(sg_cpu);
> >
> > Mmm, do we need to decay even when we are computing frequency requests
> > for a domain?
> >
> > Considering it a per-cpu thing, isn't enough that it gets bumped up or
> > decayed only when a CPU does an update (by using the above from
> > sugov_update_shared)?
> >
> > If we go this way I think we will only need to reset prev_iowait_boost
> > if delta_ns > TICK_NSEC during the for_each_cpu() loop of sugov_next_
> > freq_shared().
> >
>
> Actually the "decay" was already being done before (without this
> patch), I am just preserving the same old behavior where we do decay.
> Perhaps your proposal can be a separate match? Or did I miss something
> else subtle here?
>
True, we are currently decaying anyway.
Looking again at this path made me however think if we really need to. I
guess we need currently, as we bump frenquency to max and then decay it.
But, with your changes, I was wondering if we can simplify the thing and
decay only on the per-CPU update path.
The other reason for trying to simplify this is that I don't
particularly like adding and consuming flags argument at this point, but
I guess we could refactor the code if this is really a problem.
Thanks,
- Juri
Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable()
On Tue 11-07-17 08:56:04, Vlastimil Babka wrote:
> On 07/11/2017 08:50 AM, Michal Hocko wrote:
> > On Tue 11-07-17 08:26:40, Vlastimil Babka wrote:
> >> On 07/11/2017 08:03 AM, Michal Hocko wrote:
> >>>
> >>> Are you telling me that two if conditions cause more than a second
> >>> difference? That sounds suspicious.
> >>
> >> It's removing also a call to get_unmapped_area(), AFAICS. That means a
> >> vma search?
> >
> > Ohh, right. I have somehow missed that. Is this removal intentional?
>
> I think it is: "Checking for availability of virtual address range at
> the end of the VMA for the incremental size is also reduntant at this
> point."
I though this referred to this check
if (vma->vm_next && vma->vm_next->vm_start < end)
becuase get_unampped_area with MAP_FIXED doesn't really check
anything. It will simply return the given address. Btw. this also rules
out find_vma.
> > The
> > changelog is silent about it.
>
> It doesn't explain why it's redundant, indeed. Unfortunately, the commit
> f106af4e90ea ("fix checks for expand-in-place mremap") which added this,
> also doesn't explain why it's needed.
Because it doesn't do anything AFAICS.
--
Michal Hocko
SUSE Labs
Re: [RFC PATCH] fs: ext4: don't trap kswapd and allocating tasks on ext4 inode IO
Hi Jan,
We seemed to get this issue on a file system with data=order mode, and
it can steadily be re-produced by creating lots of symlinks and each
links to a 100KB file created by dd in advance. During the process of
dd and making symlinks, copying a large file via samba to the file
system would get stuck with the following call stacks:
kswapd0
===
[] jbd2_log_wait_commit+0x93/0x100
[] jbd2_complete_transaction+0x50/0x80
[] ext4_evict_inode+0x27c/0x400
[] evict+0xae/0x170
[] dispose_list+0x35/0x50
[] prune_icache_sb+0x46/0x60
[] super_cache_scan+0x14c/0x1a0
[] shrink_slab.part.36+0x19a/0x250
[] shrink_zone+0x23c/0x250
[] kswapd+0x54a/0x930
[] kthread+0xd6/0xf0
[] ret_from_fork+0x3f/0x70
[] 0x
kworker/u8:4
[] wait_transaction_locked+0x74/0xa0
[] start_this_handle+0x233/0x5d0
[] jbd2__journal_start+0xf2/0x180
[] __ext4_journal_start_sb+0x57/0x80
[] ext4_writepages+0x2da/0xad0
[] do_writepages+0x2e/0x70
[] __writeback_single_inode+0x33/0x170
[] writeback_sb_inodes+0x20a/0x460
[] __writeback_inodes_wb+0x84/0xb0
[] wb_writeback+0x1a2/0x1b0
[] wb_workfn+0x1b6/0x320
[] process_one_work+0x139/0x370
[] worker_thread+0x61/0x450
[] kthread+0xd6/0xf0
[] ret_from_fork+0x3f/0x70
[] 0x
jbd2/dm-0-8
===
[] jbd2_journal_commit_transaction+0x1f9/0x1540
[] kjournald2+0xd1/0x250
[] kthread+0xd6/0xf0
[] ret_from_fork+0x3f/0x70
[] 0x
We runs linux-4.2 and the issue is fixed with the patch you mentioned
in previous mail (adding "&& inode->i_data.nrpages"). If there's
anything I could help, feel free to let me know. Thanks!
Jerry
On Mon, Jun 12, 2017 at 4:09 PM, Jan Kara wrote:
> On Tue 16-05-17 18:03:37, Jan Kara wrote:
>> On Tue 16-05-17 11:41:05, Johannes Weiner wrote:
>> > On Tue, May 16, 2017 at 04:36:45PM +0200, Jan Kara wrote:
>> > > On Mon 15-05-17 11:46:34, Johannes Weiner wrote:
>> > > > We have observed across several workloads situations where kswapd and
>> > > > direct reclaimers get stuck in the inode shrinker of the ext4 / mount,
>> > > > causing allocation latencies across tasks in the system, while there
>> > > > are dozens of gigabytes of clean page cache covering multiple disks.
>> > > >
>> > > > The stack traces of such an instance looks like this:
>> > > >
>> > > > [] jbd2_log_wait_commit+0x95/0x110
>> > > > [] jbd2_complete_transaction+0x59/0x90
>> > > > [] ext4_evict_inode+0x2da/0x480
>> > > > [] evict+0xc0/0x190
>> > > > [] dispose_list+0x39/0x50
>> > > > [] prune_icache_sb+0x4b/0x60
>> > > > [] super_cache_scan+0x141/0x190
>> > > > [] shrink_slab+0x235/0x440
>> > > > [] shrink_zone+0x268/0x2d0
>> > > > [] do_try_to_free_pages+0x164/0x410
>> > > > [] try_to_free_pages+0xb5/0x160
>> > > > [] __alloc_pages_nodemask+0x636/0xb30
>> > > > [] alloc_pages_current+0x88/0x120
>> > > > [] skb_page_frag_refill+0xc6/0xf0
>> > > > [] sk_page_frag_refill+0x1d/0x80
>> > > > [] tcp_sendmsg+0x28b/0xb10
>> > > > [] inet_sendmsg+0x67/0xa0
>> > > > [] sock_sendmsg+0x38/0x50
>> > > > [] sock_write_iter+0x78/0xd0
>> > > > [] do_iter_readv_writev+0x5e/0xa0
>> > > > [] do_readv_writev+0x178/0x210
>> > > > [] vfs_writev+0x3c/0x50
>> > > > [] do_writev+0x52/0xd0
>> > > > [] SyS_writev+0x10/0x20
>> > > > [] do_syscall_64+0x50/0xa0
>> > > > [] return_from_SYSCALL_64+0x0/0x6a
>> > > > [] 0x
>> > > >
>> > > > The inode shrinker has provisions to skip any inodes that require
>> > > > writeback, to avoid tarpitting the entire system behind a single
>> > > > object when there are many other pools to recycle memory from. But
>> > > > that logic doesn't cover the situation where an ext4 inode is clean
>> > > > but journaled and tied to a commit that yet needs to hit the platter.
>> > > >
>> > > > Add a superblock operation that lets the generic inode shrinker query
>> > > > the filesystem whether evicting a given inode will require any IO; add
>> > > > an ext4 implementation that checks whether the journal is caught up to
>> > > > the commit id associated with the inode.
>> > > >
>> > > > Fixes: 2d859db3e4a8 ("ext4: fix data corruption in inodes with
>> > > > journalled data")
>> > > > Signed-off-by: Johannes Weiner
>> > >
>> > > OK. I have to say I'm somewhat surprised you use data journalling on some
>> > > of your files / filesystems but whatever - maybe these are long symlink
>> > > after all which would make sense.
>> >
>> > The filesystem is actually mounted data=ordered and we didn't catch
>> > anyone in userspace enabling journaling on individual inodes. So we
>> > assumed this must be from symlinks.
>>
>> OK.
>>
>> > > And I'm actually doubly surprised you can see these stack traces as
>> > > these days inode_lru_isolate() checks inode->i_data.nrpages and
>> > > uncommitted pages cannot be evicted from pagecache
>> > > (ext4_releasepage() will refuse to free them) so I don't see how
>> > > such inode can get to dispose_list(). But maybe the inode doesn't
>> > > really have any pages and i_datasync_tid just happens to be set to
>>
Re: [RFC] mm/mremap: Remove redundant checks inside vma_expandable()
On Tue 11-07-17 09:16:12, Michal Hocko wrote:
> On Tue 11-07-17 08:56:04, Vlastimil Babka wrote:
[...]
> > It doesn't explain why it's redundant, indeed. Unfortunately, the commit
> > f106af4e90ea ("fix checks for expand-in-place mremap") which added this,
> > also doesn't explain why it's needed.
>
> Because it doesn't do anything AFAICS.
Well, it does actually. I have missed security_mmap_addr hook.
--
Michal Hocko
SUSE Labs
[PATCH v2 01/10] net: cdc_ncm: constify attribute_group structures.
attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by work
with const attribute_group. So mark the non-const structs as const.
File size before:
textdata bss dec hex filename
13275 928 1 14204377c drivers/net/usb/cdc_ncm.o
File size After adding 'const':
textdata bss dec hex filename
13339 864 1 14204377c drivers/net/usb/cdc_ncm.o
Signed-off-by: Arvind Yadav
---
Changes in v2:
Added cover later.
drivers/net/usb/cdc_ncm.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/usb/cdc_ncm.c b/drivers/net/usb/cdc_ncm.c
index d103a1d..b401ba9 100644
--- a/drivers/net/usb/cdc_ncm.c
+++ b/drivers/net/usb/cdc_ncm.c
@@ -367,7 +367,7 @@ static DEVICE_ATTR(name, S_IRUGO, cdc_ncm_show_##name, NULL)
NULL,
};
-static struct attribute_group cdc_ncm_sysfs_attr_group = {
+static const struct attribute_group cdc_ncm_sysfs_attr_group = {
.name = "cdc_ncm",
.attrs = cdc_ncm_sysfs_attrs,
};
--
1.9.1
[PATCH v2 00/10] Constify attribute_group structures
attribute_groups are not supposed to change at runtime. So mark the non-const structs as const. Arvind Yadav (10): [PATCH v2 01/10] net: cdc_ncm: constify attribute_group structures. [PATCH v2 02/10] net: can: at91_can: constify attribute_group structures. [PATCH v2 03/10] net: can: janz-ican3: constify attribute_group structures. [PATCH v2 04/10] wireless: ipw2200: constify attribute_group structures. [PATCH v2 05/10] wireless: ipw2100: constify attribute_group structures. [PATCH v2 06/10] wireless: iwlegacy: constify attribute_group structures. [PATCH v2 07/10] wireless: iwlegacy: Constify attribute_group structures. [PATCH v2 08/10] arcnet: com20020-pci: constify attribute_group structures. [PATCH v2 09/10] net: bonding: constify attribute_group structures. [PATCH v2 10/10] net: chelsio: cxgb3: constify attribute_group structures. drivers/net/arcnet/com20020-pci.c | 2 +- drivers/net/bonding/bond_sysfs.c| 2 +- drivers/net/can/at91_can.c | 2 +- drivers/net/can/janz-ican3.c| 2 +- drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 8 ++-- drivers/net/usb/cdc_ncm.c | 2 +- drivers/net/wireless/intel/ipw2x00/ipw2100.c| 2 +- drivers/net/wireless/intel/ipw2x00/ipw2200.c| 2 +- drivers/net/wireless/intel/iwlegacy/3945-mac.c | 2 +- drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +- 10 files changed, 15 insertions(+), 11 deletions(-) -- 1.9.1
Re: [PATCH] iscsi-target: Reject immediate data underflow larger than SCSI transfer length
Hi Bart,
On Thu, 2017-06-08 at 23:55 -0700, Nicholas A. Bellinger wrote:
> On Thu, 2017-06-08 at 15:37 +, Bart Van Assche wrote:
> > On Thu, 2017-06-08 at 04:21 +, Nicholas A. Bellinger wrote:
> > > + /*
> > > + * Check for underflow case where both EDTL and immediate data payload
> > > + * exceeds what is presented by CDB's TRANSFER LENGTH, and what has
> > > + * already been set in target_cmd_size_check() as se_cmd->data_length.
> > > + *
> > > + * For this special case, fail the command and dump the immediate data
> > > + * payload.
> > > + */
> > > + if (cmd->first_burst_len > cmd->se_cmd.data_length) {
> > > + cmd->sense_reason = TCM_INVALID_CDB_FIELD;
> > > + goto after_immediate_data;
> > > + }
> >
> > A quote from the iSCSI RFC (https://tools.ietf.org/html/rfc5048):
> >
> >If SPDTL < EDTL for a task, iSCSI Underflow MUST be signaled in the
> >SCSI Response PDU as specified in [RFC3720]. The Residual Count MUST
> >be set to the numerical value of (EDTL - SPDTL).
> >
> > Sorry but I don't think that sending TCM_INVALID_CDB_FIELD back to the
> > initiator is compliant with the iSCSI RFC.
>
> Alas, the nuance of what this patch actually does was missed when you
> cut the context.
>
> First, a bit of history. LIO has rejected underflow for all WRITEs for
> the first ~12.5 years of RFC-3720, and in the context of iscsi-target
> mode there has never been a single host environment that ever once
> cared.
>
> Since Roland's patch to allow underflow for control CDBs in v4.3+ opened
> this discussion for control CDBs with a WRITE payload in order to make
> MSFT/FCP cert for PERSISTENT_RESERVE_OUT happy, the question has become
> what control CDB WRITE underflow cases should we allow..?
>
> The point with this patch is when a host is sending a underflow with a
> iscsi immediate data payload that exceeds SCSI transfer length, it's a
> bogus request with a garbage payload. It's a garbage payload because
> the SCSI CDB itself obviously doesn't want anything to do it.
>
> I'm very dubious of any host environment who's trying to do this for any
> CDB, and expects achieve expected results.
>
> Of course, since v4.3+ normal overflow where SCSI transfer length
> matches the iscsi immediate data payload just works with or without this
> patch.
>
> So to that extent, I'm going to push this patch as a defensive fix for
> v4.3+, to let those imaginary iscsi host environments know they being
> very, very naughty.
>
> > Please note that a fix that is
> > compliant with the iSCSI RFC is present in the following patch series:
> > [PATCH
> > 00/33] SCSI target driver patches for kernel v4.13, 23 May 2017
> > (https://www.spinics.net/lists/target-devel/msg15370.html).
>
> So I might still consider this as a v4.13-rc item for control CDB
> underflow, but no way as stable material.
>
> Also, there is certainly no way I'm going to allow a patch to randomly
> enable underflow/overflow for all WRITE non control CDBs tree-wide
> across all fabric drivers, because 1) no host environments actually care
> about this, and 2) it's still dangerous to do for all fabrics without
> some serious auditing.
After further consideration, I've decided against allowing iscsi-target
underflow with a immediate data payload larger than SCSI transfer
length.
Any host environment that attempts to send an underflow with a immediate
data payload larger than SCSI transfer length, expects the target to
automatically truncate immediate data, and still return GOOD status is
completely bogus. Any host that attempts this is buggy, and needs to be
fixed.
This is because for the last ~12 years of RFC-3720:
- There has never been a host environment in the wild that exhibits
this behavior.
- There has never been a conformance suite which expects this
behavior.
So rejecting this case as already done in commit abb85a9b51 is the
correct approach for >= v4.3.y.
Of course, the typical underflow scenario which Roland's v4.3.y commit
enabled, underflow where immediate data matches the SCSI transfer length
is supported for control CDBs.
That said, thanks for high-lighting this particular corner case, so it
could be fixed in >= v4.3.y.
[PATCH v2 02/10] net: can: at91_can: constify attribute_group structures.
attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by work
with const attribute_group. So mark the non-const structs as const.
File size before:
textdata bss dec hex filename
6164 304 064681944 drivers/net/can/at91_can.o
File size After adding 'const':
textdata bss dec hex filename
6228 240 064681944 drivers/net/can/at91_can.o
Signed-off-by: Arvind Yadav
---
Changes in v2:
Added cover later.
drivers/net/can/at91_can.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/at91_can.c b/drivers/net/can/at91_can.c
index 0e0df0b..f37ce0e 100644
--- a/drivers/net/can/at91_can.c
+++ b/drivers/net/can/at91_can.c
@@ -1232,7 +1232,7 @@ static DEVICE_ATTR(mb0_id, S_IWUSR | S_IRUGO,
NULL,
};
-static struct attribute_group at91_sysfs_attr_group = {
+static const struct attribute_group at91_sysfs_attr_group = {
.attrs = at91_sysfs_attrs,
};
--
1.9.1
[PATCH v2 03/10] net: can: janz-ican3: constify attribute_group structures.
attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by work
with const attribute_group. So mark the non-const structs as const.
File size before:
textdata bss dec hex filename
11800 368 0 121682f88 drivers/net/can/janz-ican3.o
File size After adding 'const':
textdata bss dec hex filename
11864 304 0 121682f88 drivers/net/can/janz-ican3.o
Signed-off-by: Arvind Yadav
---
Changes in v2:
Added cover later.
drivers/net/can/janz-ican3.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/can/janz-ican3.c b/drivers/net/can/janz-ican3.c
index 2ba1a81..12a53c8 100644
--- a/drivers/net/can/janz-ican3.c
+++ b/drivers/net/can/janz-ican3.c
@@ -1875,7 +1875,7 @@ static DEVICE_ATTR(termination, S_IWUSR | S_IRUGO,
ican3_sysfs_show_term,
NULL,
};
-static struct attribute_group ican3_sysfs_attr_group = {
+static const struct attribute_group ican3_sysfs_attr_group = {
.attrs = ican3_sysfs_attrs,
};
--
1.9.1
Re: [PATCH] tty: Fix TIOCGPTPEER ioctl definition
On Tue, Jul 11, 2017 at 2:12 AM, Gleb Fotengauer-Malinovskiy
wrote:
> This ioctl does nothing to justify an _IOC_READ or _IOC_WRITE flag
> because it doesn't copy anything from/to userspace to access the
> argument.
>
> Fixes: 54ebbfb1 ("tty: add TIOCGPTPEER ioctl")
> Signed-off-by: Gleb Fotengauer-Malinovskiy
Acked-by: Arnd Bergmann
[PATCH v2 00/10] Constify attribute_group structures
attribute_groups are not supposed to change at runtime. So mark the non-const structs as const. Arvind Yadav (10): [PATCH v2 01/10] net: cdc_ncm: constify attribute_group structures. [PATCH v2 02/10] net: can: at91_can: constify attribute_group structures. [PATCH v2 03/10] net: can: janz-ican3: constify attribute_group structures. [PATCH v2 04/10] wireless: ipw2200: constify attribute_group structures. [PATCH v2 05/10] wireless: ipw2100: constify attribute_group structures. [PATCH v2 06/10] wireless: iwlegacy: constify attribute_group structures. [PATCH v2 07/10] wireless: iwlegacy: Constify attribute_group structures. [PATCH v2 08/10] arcnet: com20020-pci: constify attribute_group structures. [PATCH v2 09/10] net: bonding: constify attribute_group structures. [PATCH v2 10/10] net: chelsio: cxgb3: constify attribute_group structures. drivers/net/arcnet/com20020-pci.c | 2 +- drivers/net/bonding/bond_sysfs.c| 2 +- drivers/net/can/at91_can.c | 2 +- drivers/net/can/janz-ican3.c| 2 +- drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 8 ++-- drivers/net/usb/cdc_ncm.c | 2 +- drivers/net/wireless/intel/ipw2x00/ipw2100.c| 2 +- drivers/net/wireless/intel/ipw2x00/ipw2200.c| 2 +- drivers/net/wireless/intel/iwlegacy/3945-mac.c | 2 +- drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +- 10 files changed, 15 insertions(+), 11 deletions(-) -- 1.9.1
[PATCH] fsi: core: register with postcore_initcall
When testing an i2c driver that is a fsi bus driver, I saw the following
oops:
kernel BUG at drivers/base/driver.c:153!
Internal error: Oops - BUG: 0 [#1] ARM
[<8027cb1c>] (driver_register) from [<80344e88>]
(fsi_driver_register+0x2c/0x38)
[<80344e88>] (fsi_driver_register) from [<805f5ebc>]
(fsi_i2c_driver_init+0x1c/0x24)
[<805f5ebc>] (fsi_i2c_driver_init) from [<805d1f14>]
(do_one_initcall+0xb4/0x170)
[<805d1f14>] (do_one_initcall) from [<805d20f0>]
(kernel_init_freeable+0x120/0x1dc)
[<805d20f0>] (kernel_init_freeable) from [<8043f4a8>] (kernel_init+0x18/0x104)
[<8043f4a8>] (kernel_init) from [<8000a5e8>] (ret_from_fork+0x14/0x2c)
This is because the fsi bus had not been registered. This fix registers the bus
with postcore_initcall instead, to ensure it is registered earlier on.
When the fsi core is used as a module this should not be a problem as the fsi
driver will depend on the fsi bus type symbol, and will therefore load the core
before the driver.
Fixes: 0508ad1fff11 ("drivers/fsi: Add empty fsi bus definitions")
Signed-off-by: Joel Stanley
---
drivers/fsi/fsi-core.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index a88df7a2b466..5966ecb72f37 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -889,17 +889,16 @@ struct bus_type fsi_bus_type = {
};
EXPORT_SYMBOL_GPL(fsi_bus_type);
-static int fsi_init(void)
+static int __init fsi_init(void)
{
return bus_register(_bus_type);
}
+postcore_initcall(fsi_init);
static void fsi_exit(void)
{
bus_unregister(_bus_type);
}
-
-module_init(fsi_init);
module_exit(fsi_exit);
module_param(discard_errors, int, 0664);
MODULE_LICENSE("GPL");
--
2.13.2
Re: [PATCH 1/5] mm/persistent-memory: match IORES_DESC name and enum memory_type one
On Wed, Jul 5, 2017 at 11:49 AM, Jerome Glisse wrote:
> On Wed, Jul 05, 2017 at 09:15:35AM -0700, Dan Williams wrote:
>> On Wed, Jul 5, 2017 at 7:25 AM, Jerome Glisse wrote:
>> > On Mon, Jul 03, 2017 at 04:49:18PM -0700, Dan Williams wrote:
>> >> On Mon, Jul 3, 2017 at 2:14 PM, Jérôme Glisse wrote:
>> >> > Use consistent name between IORES_DESC and enum memory_type, rename
>> >> > MEMORY_DEVICE_PUBLIC to MEMORY_DEVICE_PERSISTENT. This is to free up
>> >> > the public name for CDM (cache coherent device memory) for which the
>> >> > term public is a better match.
>> >> >
>> >> > Signed-off-by: Jérôme Glisse
>> >> > Cc: Dan Williams
>> >> > Cc: Ross Zwisler
>> >> > ---
>> >> > include/linux/memremap.h | 4 ++--
>> >> > kernel/memremap.c| 2 +-
>> >> > 2 files changed, 3 insertions(+), 3 deletions(-)
>> >> >
>> >> > diff --git a/include/linux/memremap.h b/include/linux/memremap.h
>> >> > index 57546a07a558..2299cc2d387d 100644
>> >> > --- a/include/linux/memremap.h
>> >> > +++ b/include/linux/memremap.h
>> >> > @@ -41,7 +41,7 @@ static inline struct vmem_altmap
>> >> > *to_vmem_altmap(unsigned long memmap_start)
>> >> > * Specialize ZONE_DEVICE memory into multiple types each having
>> >> > differents
>> >> > * usage.
>> >> > *
>> >> > - * MEMORY_DEVICE_PUBLIC:
>> >> > + * MEMORY_DEVICE_PERSISTENT:
>> >> > * Persistent device memory (pmem): struct page might be allocated in
>> >> > different
>> >> > * memory and architecture might want to perform special actions. It
>> >> > is similar
>> >> > * to regular memory, in that the CPU can access it transparently.
>> >> > However,
>> >> > @@ -59,7 +59,7 @@ static inline struct vmem_altmap
>> >> > *to_vmem_altmap(unsigned long memmap_start)
>> >> > * include/linux/hmm.h and Documentation/vm/hmm.txt.
>> >> > */
>> >> > enum memory_type {
>> >> > - MEMORY_DEVICE_PUBLIC = 0,
>> >> > + MEMORY_DEVICE_PERSISTENT = 0,
>> >> > MEMORY_DEVICE_PRIVATE,
>> >> > };
>> >> >
>> >> > diff --git a/kernel/memremap.c b/kernel/memremap.c
>> >> > index b9baa6c07918..e82456c39a6a 100644
>> >> > --- a/kernel/memremap.c
>> >> > +++ b/kernel/memremap.c
>> >> > @@ -350,7 +350,7 @@ void *devm_memremap_pages(struct device *dev,
>> >> > struct resource *res,
>> >> > }
>> >> > pgmap->ref = ref;
>> >> > pgmap->res = _map->res;
>> >> > - pgmap->type = MEMORY_DEVICE_PUBLIC;
>> >> > + pgmap->type = MEMORY_DEVICE_PERSISTENT;
>> >> > pgmap->page_fault = NULL;
>> >> > pgmap->page_free = NULL;
>> >> > pgmap->data = NULL;
>> >>
>> >> I think we need a different name. There's nothing "persistent" about
>> >> the devm_memremap_pages() path. Why can't they share name, is the only
>> >> difference coherence? I'm thinking something like:
>> >>
>> >> MEMORY_DEVICE_PRIVATE
>> >> MEMORY_DEVICE_COHERENT /* persistent memory and coherent devices */
>> >> MEMORY_DEVICE_IO /* "public", but not coherent */
>> >
>> > No that would not work. Device public (in the context of this patchset)
>> > is like device private ie device public page can be anywhere inside a
>> > process address space either as anonymous memory page or as file back
>> > page of regular filesystem (ie vma->ops is not pointing to anything
>> > specific to the device memory).
>> >
>> > As such device public is different from how persistent memory is use
>> > and those the cache coherency being the same between the two kind of
>> > memory is not a discerning factor. So i need to distinguish between
>> > persistent memory and device public memory.
>> >
>> > I believe keeping enum memory_type close to IORES_DESC naming is the
>> > cleanest way to do that but i am open to other name suggestion.
>> >
>>
>> The IORES_DESC has nothing to do with how the memory range is handled
>> by the core mm. It sounds like the distinction this is trying to make
>> is between MEMORY_DEVICE_{PUBLIC,PRIVATE} and MEMORY_DEVICE_HOST.
>> Where a "host" memory range is one that does not need coordination
>> with a specific device.
>
> I want to distinguish between:
> - device memory that is not accessible by the CPU
> - device memory that is accessible by the CPU just like regular
> memory
> - existing user of devm_memremap_pages() which is persistent memory
> (only pmem seems to call devm_memremap_pages()) that is use like a
> filesystem or block device and thus isn't use like generic page in
> a process address space
>
> So if existing user of devm_memremap_pages() are only persistent memory
> then it made sense to match the IORES_DESC we are expecting to see on
> see such memory.
>
> For public device memory (in the sense introduced by this patchset) i
> do not know how it will be described by IORES_DESC. i think first folks
> with it are IBM with CAPI and i am not sure they defined something for
> that already.
>
> I am open to any name beside public (well any reasonable name :)) but
> i do need to be able to distinguish persistent
[PATCH v2 05/10] wireless: ipw2100: constify attribute_group structures.
attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by work
with const attribute_group. So mark the non-const structs as const.
Signed-off-by: Arvind Yadav
---
Changes in v2:
Added cover later.
drivers/net/wireless/intel/ipw2x00/ipw2100.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2100.c
b/drivers/net/wireless/intel/ipw2x00/ipw2100.c
index aaaca4d..ccbe745 100644
--- a/drivers/net/wireless/intel/ipw2x00/ipw2100.c
+++ b/drivers/net/wireless/intel/ipw2x00/ipw2100.c
@@ -4324,7 +4324,7 @@ static ssize_t store_rf_kill(struct device *d, struct
device_attribute *attr,
NULL,
};
-static struct attribute_group ipw2100_attribute_group = {
+static const struct attribute_group ipw2100_attribute_group = {
.attrs = ipw2100_sysfs_entries,
};
--
1.9.1
[PATCH v2 04/10] wireless: ipw2200: constify attribute_group structures.
attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by work
with const attribute_group. So mark the non-const structs as const.
Signed-off-by: Arvind Yadav
---
Changes in v2:
Added cover later.
drivers/net/wireless/intel/ipw2x00/ipw2200.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
index 9368abd..c311b1a 100644
--- a/drivers/net/wireless/intel/ipw2x00/ipw2200.c
+++ b/drivers/net/wireless/intel/ipw2x00/ipw2200.c
@@ -11500,7 +11500,7 @@ static int ipw_wdev_init(struct net_device *dev)
NULL
};
-static struct attribute_group ipw_attribute_group = {
+static const struct attribute_group ipw_attribute_group = {
.name = NULL, /* put in device directory */
.attrs = ipw_sysfs_entries,
};
--
1.9.1
Re: [PATCH] usb: dwc2: gadget: On disconnect reset device address to zero
Hi, Minas Harutyunyan writes: >>> Minas Harutyunyan writes: USB CV driver stack doesn't perform USB RESET after device disconnect/ connect, so need to reset to zero DEVADDR field in DCFG to pass enumeration again. Signed-off-by: Minas Harutyunyan --- drivers/usb/dwc2/gadget.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c index 98a4a79e7f6e..deb3d901b99d 100644 --- a/drivers/usb/dwc2/gadget.c +++ b/drivers/usb/dwc2/gadget.c @@ -3174,6 +3174,9 @@ void dwc2_hsotg_disconnect(struct dwc2_hsotg *hsotg) return; hsotg->connected = 0; + /* On disconnect reset device address to zero */ + __bic32(hsotg->regs + DCFG, DCFG_DEVADDR_MASK); + >>> >>> Which of the tests are you talking about? Which particular USB CV are >>> you running? >>> >>> I don't remember ever seeing this with dwc3. How should I go about >>> triggering this problem? If this was really the case, then we would have >>> this on *all* UDCs. >>> >>> I just did a fresh install of USB 3 Gen X CV (that I just download from >>> usb.org). Ran Chapter 9 tests against a HS dwc3 board I have around and >>> I can't see the problem you're talking about. >>> >>> Here are all my non-endpoint interrupt events in order. Test passes >>> fine. If disconnect, reconnect and run the tests again, then Reset will >>> be driven on the bus which will cause address to be reset. >>> >>> Can you share more details about the problem you're facing? >> >> I've been looking at dwc2 for a while and I think this is a bug in >> dwc2_hsotg_irq() on the branch for GINTSTS_USBRST. I don't have docs for >> dwc2. >> > > USB RESET not issuing by CV host stack after connect. Below dmesg: > > Disconnect > > [23984.039199] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04008428 0400 > (d0bc3cc4) retry 8 > [23984.039204] dwc2 dwc2.1.auto: GINTSTS_ErlySusp > [23984.042235] dwc2 dwc2.1.auto: gintsts=04008828 gintmsk=d0bc3cc4 > [23984.042240] dwc2 dwc2.1.auto: USB SUSPEND > [23984.042247] dwc2 dwc2.1.auto: DSTS=0x5f4c01 > [23984.042250] dwc2 dwc2.1.auto: DSTS.Suspend Status=1 HWCFG4.Power > Optimize=0 > [23984.042260] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04008028 > (d0bc3cc4) retry 8 > [23984.272308] dwc2 dwc2.1.auto: gintsts=0480902c gintmsk=d0bc3cc4 > [23984.272318] dwc2 dwc2.1.auto: ++OTG Interrupt gotgint=4 [b_peripheral] > [23984.272321] dwc2 dwc2.1.auto: ++OTG Interrupt: Session End > Detected++ (b_peripheral) > [23984.272331] dwc2 dwc2.1.auto: complete: ep 880401a31b28 ep0, req > 8803e495ef00, -108 => a00541da > [23984.272336] dwc2 dwc2.1.auto: dwc2_hsotg_complete_setup: failed -108 > [23984.272346] dwc2 dwc2.1.auto: complete: ep 880401a30328 ep1out, > req 88040b6d7c80, -108 => a008865c > [23984.272435] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04809028 00801000 > (d0bc3cc4) retry 8 > [23984.272437] dwc2 dwc2.1.auto: dwc2_hsotg_irq: USBRstDet > [23984.272442] dwc2 dwc2.1.auto: dwc2_hsotg_irq: USBRst here it is. > [23984.272446] dwc2 dwc2.1.auto: GNPTXSTS=00040300 > [23984.272473] dwc2 dwc2.1.auto: dwc2_hsotg_ep_disable(ep 880401a30528) > [23984.272478] dwc2 dwc2.1.auto: dwc2_hsotg_ep_disable: DxEPCTL=0x084a0200 > [23984.272485] dwc2 dwc2.1.auto: dwc2_hsotg_ep_disable(ep 880401a30328) > [23984.272490] dwc2 dwc2.1.auto: dwc2_hsotg_ep_stop_xfr: stopping > transfer on ep1out > [23984.272513] dwc2 dwc2.1.auto: dwc2_hsotg_ep_disable: DxEPCTL=0x08080200 > [23984.272540] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04008028 > (d0bc3cc4) retry 8 > > Connect > > [24010.664017] dwc2 dwc2.1.auto: gintsts=44008028 gintmsk=d0bc3cc4 > [24010.664023] dwc2 dwc2.1.auto: Session request interrupt - lx_state=0 > [24010.664035] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 04008028 > (d0bc3cc4) retry 8 > > Run test > > First try to enumerate > > [24021.649528] dwc2 dwc2.1.auto: dwc2_hsotg_irq: 0400a028 2000 > (d0bc3cc4) retry 8 > [24021.649536] dwc2 dwc2.1.auto: EnumDone (DSTS=0x0043e902) > [24021.649539] dwc2 dwc2.1.auto: new device is full-speed > [24021.649618] dwc2 dwc2.1.auto: dwc2_hsotg_enqueue_setup: queueing > setup request > [24021.649623] dwc2 dwc2.1.auto: ep0: req 8803e495ef00: > 8@88040ad1d228, noi=0, zero=0, snok=0 > [24021.649631] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: > DxEPCTL=0x80028000, ep 0, dir out > [24021.649636] dwc2 dwc2.1.auto: ureq->length:8 ureq->actual:0 > [24021.649640] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: 1@8/8, 0x00080008 > => 0x0b10 > [24021.649643] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: > 0xd9858800 => 0x0b14 > [24021.649645] dwc2 dwc2.1.auto: ep0 state:0 > [24021.649648] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DxEPCTL=0x80028000 > [24021.649654] dwc2 dwc2.1.auto: dwc2_hsotg_start_req: DXEPCTL=0x80028000 > [24021.649664] dwc2 dwc2.1.auto: EP0: DIEPCTL0=0x8000, > DOEPCTL0=0x80028000 > [24021.849430] dwc2 dwc2.1.auto:
[PATCH v2 00/10] Constify attribute_group structures
attribute_groups are not supposed to change at runtime. So mark the non-const structs as const. Arvind Yadav (10): [PATCH v2 01/10] net: cdc_ncm: constify attribute_group structures. [PATCH v2 02/10] net: can: at91_can: constify attribute_group structures. [PATCH v2 03/10] net: can: janz-ican3: constify attribute_group structures. [PATCH v2 04/10] wireless: ipw2200: constify attribute_group structures. [PATCH v2 05/10] wireless: ipw2100: constify attribute_group structures. [PATCH v2 06/10] wireless: iwlegacy: constify attribute_group structures. [PATCH v2 07/10] wireless: iwlegacy: Constify attribute_group structures. [PATCH v2 08/10] arcnet: com20020-pci: constify attribute_group structures. [PATCH v2 09/10] net: bonding: constify attribute_group structures. [PATCH v2 10/10] net: chelsio: cxgb3: constify attribute_group structures. drivers/net/arcnet/com20020-pci.c | 2 +- drivers/net/bonding/bond_sysfs.c| 2 +- drivers/net/can/at91_can.c | 2 +- drivers/net/can/janz-ican3.c| 2 +- drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 8 ++-- drivers/net/usb/cdc_ncm.c | 2 +- drivers/net/wireless/intel/ipw2x00/ipw2100.c| 2 +- drivers/net/wireless/intel/ipw2x00/ipw2200.c| 2 +- drivers/net/wireless/intel/iwlegacy/3945-mac.c | 2 +- drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +- 10 files changed, 15 insertions(+), 11 deletions(-) -- 1.9.1
[PATCH v2 00/10] Constify attribute_group structures
attribute_groups are not supposed to change at runtime. So mark the non-const structs as const. Arvind Yadav (10): [PATCH v2 01/10] net: cdc_ncm: constify attribute_group structures. [PATCH v2 02/10] net: can: at91_can: constify attribute_group structures. [PATCH v2 03/10] net: can: janz-ican3: constify attribute_group structures. [PATCH v2 04/10] wireless: ipw2200: constify attribute_group structures. [PATCH v2 05/10] wireless: ipw2100: constify attribute_group structures. [PATCH v2 06/10] wireless: iwlegacy: constify attribute_group structures. [PATCH v2 07/10] wireless: iwlegacy: Constify attribute_group structures. [PATCH v2 08/10] arcnet: com20020-pci: constify attribute_group structures. [PATCH v2 09/10] net: bonding: constify attribute_group structures. [PATCH v2 10/10] net: chelsio: cxgb3: constify attribute_group structures. drivers/net/arcnet/com20020-pci.c | 2 +- drivers/net/bonding/bond_sysfs.c| 2 +- drivers/net/can/at91_can.c | 2 +- drivers/net/can/janz-ican3.c| 2 +- drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 8 ++-- drivers/net/usb/cdc_ncm.c | 2 +- drivers/net/wireless/intel/ipw2x00/ipw2100.c| 2 +- drivers/net/wireless/intel/ipw2x00/ipw2200.c| 2 +- drivers/net/wireless/intel/iwlegacy/3945-mac.c | 2 +- drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +- 10 files changed, 15 insertions(+), 11 deletions(-) -- 1.9.1
[PATCH v2 07/10] wireless: iwlegacy: Constify attribute_group structures.
attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by work
with const attribute_group. So mark the non-const structs as const.
Signed-off-by: Arvind Yadav
---
Changes in v2:
Added cover later.
drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intel/iwlegacy/4965-mac.c
b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
index 5b51fba..de9b652 100644
--- a/drivers/net/wireless/intel/iwlegacy/4965-mac.c
+++ b/drivers/net/wireless/intel/iwlegacy/4965-mac.c
@@ -4654,7 +4654,7 @@ static DEVICE_ATTR(tx_power, S_IWUSR | S_IRUGO,
il4965_show_tx_power,
NULL
};
-static struct attribute_group il_attribute_group = {
+static const struct attribute_group il_attribute_group = {
.name = NULL, /* put in device directory */
.attrs = il_sysfs_entries,
};
--
1.9.1
[PATCH v2 06/10] wireless: iwlegacy: constify attribute_group structures.
attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by work
with const attribute_group. So mark the non-const structs as const.
Signed-off-by: Arvind Yadav
---
Changes in v2:
Added cover later.
drivers/net/wireless/intel/iwlegacy/3945-mac.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intel/iwlegacy/3945-mac.c
b/drivers/net/wireless/intel/iwlegacy/3945-mac.c
index 38bf403..329f3a6 100644
--- a/drivers/net/wireless/intel/iwlegacy/3945-mac.c
+++ b/drivers/net/wireless/intel/iwlegacy/3945-mac.c
@@ -3464,7 +3464,7 @@ static DEVICE_ATTR(antenna, S_IWUSR | S_IRUGO,
il3945_show_antenna,
NULL
};
-static struct attribute_group il3945_attribute_group = {
+static const struct attribute_group il3945_attribute_group = {
.name = NULL, /* put in device directory */
.attrs = il3945_sysfs_entries,
};
--
1.9.1
Re: [PATCH] usb: dwc2: gadget: On disconnect reset device address to zero
Hi, Minas Harutyunyan writes: >> Minas Harutyunyan writes: >>> USB CV driver stack doesn't perform USB RESET after device disconnect/ >>> connect, so need to reset to zero DEVADDR field in DCFG to pass >>> enumeration again. >>> >>> Signed-off-by: Minas Harutyunyan >>> --- >>> drivers/usb/dwc2/gadget.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c >>> index 98a4a79e7f6e..deb3d901b99d 100644 >>> --- a/drivers/usb/dwc2/gadget.c >>> +++ b/drivers/usb/dwc2/gadget.c >>> @@ -3174,6 +3174,9 @@ void dwc2_hsotg_disconnect(struct dwc2_hsotg *hsotg) >>> return; >>> >>> hsotg->connected = 0; >>> + /* On disconnect reset device address to zero */ >>> + __bic32(hsotg->regs + DCFG, DCFG_DEVADDR_MASK); >>> + >> >> Which of the tests are you talking about? Which particular USB CV are >> you running? >> > I used USB 3 Gen X CV (downloaded from usb.org 1-2 week ago). Tests are: > 1. "Device Summary" - after 1st pass, disconnect then connect again test > failed. Actually it show as "test passed" but not able to enumerate > device again. > 2. MSC "USB Mass Storage Power Up Test". After disconnect, by suite > request, and then connect test failed (pass, if reloading driver). I'll try these tests tomorrow next time I'm in the office. Woke up sick, working from home today :-s >> I don't remember ever seeing this with dwc3. How should I go about >> triggering this problem? If this was really the case, then we would have >> this on *all* UDCs. >> > dwc2 driver resetting DEVADDR in DCFG register only in function > dwc2_hsotg_core_init_disconnected() by soft reset. This function not > called on disconnect/connect with CV SW stack (function call not seen in > dmesg). right, this is a bug in dwc2. You should clear DEVADDR from your Reset handler, not disconnect. > This issue not seen with any other EHCI/XHCI hosts either on > Linux/Windows because these hosts issuing USB RESET, as result called > dwc2_hsotg_core_init_disconnected(). > > In dwc3 per my understanding same stuff done in function > dwc3_gadget_reset_interrupt(), am I right? right, as it should. If dwc3_gadget_reset_interrupt() runs, this means the host *is* issuing USB reset signalling. >> I just did a fresh install of USB 3 Gen X CV (that I just download from >> usb.org). Ran Chapter 9 tests against a HS dwc3 board I have around and >> I can't see the problem you're talking about. >> > Yes, this issue not seen with dwc3. because we clear DEVADDR on Reset. -- balbi
[PATCH v2 00/10] Constify attribute_group structures
attribute_groups are not supposed to change at runtime. So mark the non-const structs as const. Arvind Yadav (10): [PATCH v2 01/10] net: cdc_ncm: constify attribute_group structures. [PATCH v2 02/10] net: can: at91_can: constify attribute_group structures. [PATCH v2 03/10] net: can: janz-ican3: constify attribute_group structures. [PATCH v2 04/10] wireless: ipw2200: constify attribute_group structures. [PATCH v2 05/10] wireless: ipw2100: constify attribute_group structures. [PATCH v2 06/10] wireless: iwlegacy: constify attribute_group structures. [PATCH v2 07/10] wireless: iwlegacy: Constify attribute_group structures. [PATCH v2 08/10] arcnet: com20020-pci: constify attribute_group structures. [PATCH v2 09/10] net: bonding: constify attribute_group structures. [PATCH v2 10/10] net: chelsio: cxgb3: constify attribute_group structures. drivers/net/arcnet/com20020-pci.c | 2 +- drivers/net/bonding/bond_sysfs.c| 2 +- drivers/net/can/at91_can.c | 2 +- drivers/net/can/janz-ican3.c| 2 +- drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 8 ++-- drivers/net/usb/cdc_ncm.c | 2 +- drivers/net/wireless/intel/ipw2x00/ipw2100.c| 2 +- drivers/net/wireless/intel/ipw2x00/ipw2200.c| 2 +- drivers/net/wireless/intel/iwlegacy/3945-mac.c | 2 +- drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +- 10 files changed, 15 insertions(+), 11 deletions(-) -- 1.9.1
Re: [PATCH 00/13]mpt3sas driver NVMe support:
Is there any update on this ? Will include linux-n...@lists.infradead.org in next version of this patch submission, if there is any change suggestion. Thanks, Suganath Prabu S On Thu, Jun 29, 2017 at 8:01 PM, Johannes Thumshirn wrote: > On Thu, Jun 29, 2017 at 07:49:01PM +0530, Suganath Prabu S wrote: >> Ventura Series controller are Tri-mode. The controller and >> firmware are capable of supporting NVMe devices and >> PCIe switches to be connected with the controller. This >> patch set adds driver level support for NVMe devices and >> PCIe switches. > > Hi Suganath, > Can you please also Cc linux-n...@lists.infradead.org for NVMe related topics. > > Thanks, > Johannes > -- > Johannes Thumshirn Storage > jthumsh...@suse.de+49 911 74053 689 > SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: Felix Imendörffer, Jane Smithard, Graham Norton > HRB 21284 (AG Nürnberg) > Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
[PATCH v2 08/10] arcnet: com20020-pci: constify attribute_group structures.
attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by work
with const attribute_group. So mark the non-const structs as const.
File size before:
textdata bss dec hex filename
3409 948 2843851121 drivers/net/arcnet/com20020-pci.o
File size After adding 'const':
textdata bss dec hex filename
3473 884 2843851121 drivers/net/arcnet/com20020-pci.o
Signed-off-by: Arvind Yadav
---
Changes in v2:
Added cover later.
drivers/net/arcnet/com20020-pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/arcnet/com20020-pci.c
b/drivers/net/arcnet/com20020-pci.c
index 01cab95..eb7f767 100644
--- a/drivers/net/arcnet/com20020-pci.c
+++ b/drivers/net/arcnet/com20020-pci.c
@@ -109,7 +109,7 @@ static ssize_t backplane_mode_show(struct device *dev,
NULL,
};
-static struct attribute_group com20020_state_group = {
+static const struct attribute_group com20020_state_group = {
.name = NULL,
.attrs = com20020_state_attrs,
};
--
1.9.1
[PATCH v2 09/10] net: bonding: constify attribute_group structures.
attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by work
with const attribute_group. So mark the non-const structs as const.
File size before:
textdata bss dec hex filename
45121472 059841760 drivers/net/bonding/bond_sysfs.o
File size After adding 'const':
textdata bss dec hex filename
45761408 059841760 drivers/net/bonding/bond_sysfs.o
Signed-off-by: Arvind Yadav
---
Changes in v2:
Added cover later.
drivers/net/bonding/bond_sysfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
index 770623a..040b493 100644
--- a/drivers/net/bonding/bond_sysfs.c
+++ b/drivers/net/bonding/bond_sysfs.c
@@ -759,7 +759,7 @@ static DEVICE_ATTR(ad_user_port_key, S_IRUGO | S_IWUSR,
NULL,
};
-static struct attribute_group bonding_group = {
+static const struct attribute_group bonding_group = {
.name = "bonding",
.attrs = per_bond_attrs,
};
--
1.9.1
[PATCH v2 00/10] Constify attribute_group structures
attribute_groups are not supposed to change at runtime. So mark the non-const structs as const. Arvind Yadav (10): [PATCH v2 01/10] net: cdc_ncm: constify attribute_group structures. [PATCH v2 02/10] net: can: at91_can: constify attribute_group structures. [PATCH v2 03/10] net: can: janz-ican3: constify attribute_group structures. [PATCH v2 04/10] wireless: ipw2200: constify attribute_group structures. [PATCH v2 05/10] wireless: ipw2100: constify attribute_group structures. [PATCH v2 06/10] wireless: iwlegacy: constify attribute_group structures. [PATCH v2 07/10] wireless: iwlegacy: Constify attribute_group structures. [PATCH v2 08/10] arcnet: com20020-pci: constify attribute_group structures. [PATCH v2 09/10] net: bonding: constify attribute_group structures. [PATCH v2 10/10] net: chelsio: cxgb3: constify attribute_group structures. drivers/net/arcnet/com20020-pci.c | 2 +- drivers/net/bonding/bond_sysfs.c| 2 +- drivers/net/can/at91_can.c | 2 +- drivers/net/can/janz-ican3.c| 2 +- drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 8 ++-- drivers/net/usb/cdc_ncm.c | 2 +- drivers/net/wireless/intel/ipw2x00/ipw2100.c| 2 +- drivers/net/wireless/intel/ipw2x00/ipw2200.c| 2 +- drivers/net/wireless/intel/iwlegacy/3945-mac.c | 2 +- drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +- 10 files changed, 15 insertions(+), 11 deletions(-) -- 1.9.1
Re: [PATCH 00/13]mpt3sas driver NVMe support:
On Tue, Jul 11, 2017 at 01:05:29PM +0530, Suganath Prabu Subramani wrote: > Is there any update on this ? > Will include linux-n...@lists.infradead.org in next version of this > patch submission, if there is any change suggestion. can you please re-send with Cc to linux-nvme? Thanks, Johannes -- Johannes Thumshirn Storage jthumsh...@suse.de+49 911 74053 689 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: Felix Imendörffer, Jane Smithard, Graham Norton HRB 21284 (AG Nürnberg) Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
[PATCH v2 10/10] net: chelsio: cxgb3: constify attribute_group structures.
attribute_groups are not supposed to change at runtime. All functions
working with attribute_groups provided by work
with const attribute_group. So mark the non-const structs as const.
File size before:
textdata bss dec hex filename
28720 985 12 297177415 drivers/net/.../cxgb3/cxgb3_main.o
File size After adding 'const':
textdata bss dec hex filename
28848 857 12 297177415 drivers/net/.../cxgb3/cxgb3_main.o
Signed-off-by: Arvind Yadav
---
Changes in v2:
Added cover later.
drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
index 0bc6a4f..6a01536 100644
--- a/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
+++ b/drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c
@@ -793,7 +793,9 @@ static DEVICE_ATTR(name, S_IRUGO | S_IWUSR, show_##name,
store_method)
NULL
};
-static struct attribute_group cxgb3_attr_group = {.attrs = cxgb3_attrs };
+static const struct attribute_group cxgb3_attr_group = {
+ .attrs = cxgb3_attrs,
+};
static ssize_t tm_attr_show(struct device *d,
char *buf, int sched)
@@ -880,7 +882,9 @@ static DEVICE_ATTR(name, S_IRUGO | S_IWUSR, show_##name,
store_##name)
NULL
};
-static struct attribute_group offload_attr_group = {.attrs = offload_attrs };
+static const struct attribute_group offload_attr_group = {
+ .attrs = offload_attrs,
+};
/*
* Sends an sk_buff to an offload queue driver
--
1.9.1
Re: [PATCH] fsi: core: register with postcore_initcall
Hi Joel, > This fix registers the bus with postcore_initcall instead, to ensure > it is registered earlier on. Looks good to me. Acked-by: Jeremy Kerr Cheers, Jeremy
[PATCH v2 00/10] Constify attribute_group structures
attribute_groups are not supposed to change at runtime. So mark the non-const structs as const. Arvind Yadav (10): [PATCH v2 01/10] net: cdc_ncm: constify attribute_group structures. [PATCH v2 02/10] net: can: at91_can: constify attribute_group structures. [PATCH v2 03/10] net: can: janz-ican3: constify attribute_group structures. [PATCH v2 04/10] wireless: ipw2200: constify attribute_group structures. [PATCH v2 05/10] wireless: ipw2100: constify attribute_group structures. [PATCH v2 06/10] wireless: iwlegacy: constify attribute_group structures. [PATCH v2 07/10] wireless: iwlegacy: Constify attribute_group structures. [PATCH v2 08/10] arcnet: com20020-pci: constify attribute_group structures. [PATCH v2 09/10] net: bonding: constify attribute_group structures. [PATCH v2 10/10] net: chelsio: cxgb3: constify attribute_group structures. drivers/net/arcnet/com20020-pci.c | 2 +- drivers/net/bonding/bond_sysfs.c| 2 +- drivers/net/can/at91_can.c | 2 +- drivers/net/can/janz-ican3.c| 2 +- drivers/net/ethernet/chelsio/cxgb3/cxgb3_main.c | 8 ++-- drivers/net/usb/cdc_ncm.c | 2 +- drivers/net/wireless/intel/ipw2x00/ipw2100.c| 2 +- drivers/net/wireless/intel/ipw2x00/ipw2200.c| 2 +- drivers/net/wireless/intel/iwlegacy/3945-mac.c | 2 +- drivers/net/wireless/intel/iwlegacy/4965-mac.c | 2 +- 10 files changed, 15 insertions(+), 11 deletions(-) -- 1.9.1
[PATCH 3/3] ARM: dts: at91: sama5d2: use sama5d2 compatible string for SMC
A new compatible string has been introduced for sama5d2 SMC to allow to
manage the registers mapping change.
Signed-off-by: Ludovic Desroches
---
arch/arm/boot/dts/sama5d2.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
index 4fcd5bb219e3..60e69aeacbdb 100644
--- a/arch/arm/boot/dts/sama5d2.dtsi
+++ b/arch/arm/boot/dts/sama5d2.dtsi
@@ -1048,7 +1048,7 @@
};
hsmc: hsmc@f8014000 {
- compatible = "atmel,sama5d3-smc", "syscon",
"simple-mfd";
+ compatible = "atmel,sama5d2-smc", "syscon",
"simple-mfd";
reg = <0xf8014000 0x1000>;
interrupts = <17 IRQ_TYPE_LEVEL_HIGH 6>;
clocks = <_clk>;
--
2.12.2
[PATCH 1/3] mfd: syscon: update Atmel SMC binding doc
A new compatible string is introduced for SMC on sama5d2 to manage a different layout of the registers. Signed-off-by: Ludovic Desroches --- Documentation/devicetree/bindings/mfd/atmel-smc.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/mfd/atmel-smc.txt b/Documentation/devicetree/bindings/mfd/atmel-smc.txt index 26eeed373934..1103ce2030fb 100644 --- a/Documentation/devicetree/bindings/mfd/atmel-smc.txt +++ b/Documentation/devicetree/bindings/mfd/atmel-smc.txt @@ -8,6 +8,7 @@ Required properties: - compatible: Should be one of the following "atmel,at91sam9260-smc", "syscon" "atmel,sama5d3-smc", "syscon" + "atmel,sama5d2-smc", "syscon" - reg: Contains offset/length value of the SMC memory region. -- 2.12.2
[PATCH 0/3] Fix SMC layout register for sama5d2
Hi, The offset of some registers of the SMC is different between sama5d3/sama5d4 and sama5d2. An helper function has been added to retrieve the correct layout with the help of a new compatible string for sama5d2. The atmel-ebi device will store the register layout to allow the nand controller to setup correctly the data interface. Ludovic Desroches (3): mfd: syscon: update Atmel SMC binding doc mfd: syscon: atmel-smc: add helper to retrieve register layout ARM: dts: at91: sama5d2: use sama5d2 compatible string for SMC .../devicetree/bindings/mfd/atmel-smc.txt | 1 + arch/arm/boot/dts/sama5d2.dtsi | 2 +- drivers/memory/atmel-ebi.c | 13 +++-- drivers/mfd/atmel-smc.c| 67 +- drivers/mtd/nand/atmel/nand-controller.c | 10 +++- include/linux/mfd/syscon/atmel-smc.h | 32 --- 6 files changed, 94 insertions(+), 31 deletions(-) -- 2.12.2
[PATCH 2/3] mfd: syscon: atmel-smc: add helper to retrieve register layout
For HSMC controller, the register layout depends on the device i.e. the
offset of setup, pulse, cycle, mode and timings registers is not the
same. An helper is added to provide the correct register layout.
Fixes: fe9d7cb22ef3 ("mfd: syscon: atmel-smc: Add new helpers to ease
SMC regs manipulation")
Suggested-by: Boris Brezillon
Signed-off-by: Ludovic Desroches
---
drivers/memory/atmel-ebi.c | 13 +--
drivers/mfd/atmel-smc.c | 67 +---
drivers/mtd/nand/atmel/nand-controller.c | 10 +++--
include/linux/mfd/syscon/atmel-smc.h | 32 ++-
4 files changed, 92 insertions(+), 30 deletions(-)
diff --git a/drivers/memory/atmel-ebi.c b/drivers/memory/atmel-ebi.c
index 99e644cda4d1..63c9e7a76854 100644
--- a/drivers/memory/atmel-ebi.c
+++ b/drivers/memory/atmel-ebi.c
@@ -51,6 +51,7 @@ struct atmel_ebi {
struct {
struct regmap *regmap;
struct clk *clk;
+ const struct atmel_hsmc_reg_layout *layout;
} smc;
struct device *dev;
@@ -84,8 +85,8 @@ static void at91sam9_ebi_get_config(struct atmel_ebi_dev
*ebid,
static void sama5_ebi_get_config(struct atmel_ebi_dev *ebid,
struct atmel_ebi_dev_config *conf)
{
- atmel_hsmc_cs_conf_get(ebid->ebi->smc.regmap, conf->cs,
- >smcconf);
+ atmel_hsmc_cs_conf_get(ebid->ebi->smc.regmap, ebid->ebi->smc.layout,
+ conf->cs, >smcconf);
}
static const struct atmel_smc_timing_xlate timings_xlate_table[] = {
@@ -285,8 +286,8 @@ static void at91sam9_ebi_apply_config(struct atmel_ebi_dev
*ebid,
static void sama5_ebi_apply_config(struct atmel_ebi_dev *ebid,
struct atmel_ebi_dev_config *conf)
{
- atmel_hsmc_cs_conf_apply(ebid->ebi->smc.regmap, conf->cs,
->smcconf);
+ atmel_hsmc_cs_conf_apply(ebid->ebi->smc.regmap, ebid->ebi->smc.layout,
+conf->cs, >smcconf);
}
static int atmel_ebi_dev_setup(struct atmel_ebi *ebi, struct device_node *np,
@@ -525,6 +526,10 @@ static int atmel_ebi_probe(struct platform_device *pdev)
if (IS_ERR(ebi->smc.regmap))
return PTR_ERR(ebi->smc.regmap);
+ ebi->smc.layout = atmel_hsmc_get_reg_layout(smc_np);
+ if (IS_ERR(ebi->smc.layout))
+ return PTR_ERR(ebi->smc.layout);
+
ebi->smc.clk = of_clk_get(smc_np, 0);
if (IS_ERR(ebi->smc.clk)) {
if (PTR_ERR(ebi->smc.clk) != -ENOENT)
diff --git a/drivers/mfd/atmel-smc.c b/drivers/mfd/atmel-smc.c
index 954cf0f66a31..1ad44e63b511 100644
--- a/drivers/mfd/atmel-smc.c
+++ b/drivers/mfd/atmel-smc.c
@@ -258,19 +258,21 @@ EXPORT_SYMBOL_GPL(atmel_smc_cs_conf_apply);
* atmel_hsmc_cs_conf_apply - apply an SMC CS conf
* @regmap: the HSMC regmap
* @cs: the CS id
+ * @layout: the layout of registers
* @conf the SMC CS conf to apply
*
* Applies an SMC CS configuration.
* Only valid on post-sama5 SoCs.
*/
-void atmel_hsmc_cs_conf_apply(struct regmap *regmap, int cs,
- const struct atmel_smc_cs_conf *conf)
+void atmel_hsmc_cs_conf_apply(struct regmap *regmap,
+ const struct atmel_hsmc_reg_layout *layout,
+ int cs, const struct atmel_smc_cs_conf *conf)
{
- regmap_write(regmap, ATMEL_HSMC_SETUP(cs), conf->setup);
- regmap_write(regmap, ATMEL_HSMC_PULSE(cs), conf->pulse);
- regmap_write(regmap, ATMEL_HSMC_CYCLE(cs), conf->cycle);
- regmap_write(regmap, ATMEL_HSMC_TIMINGS(cs), conf->timings);
- regmap_write(regmap, ATMEL_HSMC_MODE(cs), conf->mode);
+ regmap_write(regmap, ATMEL_HSMC_SETUP(layout, cs), conf->setup);
+ regmap_write(regmap, ATMEL_HSMC_PULSE(layout, cs), conf->pulse);
+ regmap_write(regmap, ATMEL_HSMC_CYCLE(layout, cs), conf->cycle);
+ regmap_write(regmap, ATMEL_HSMC_TIMINGS(layout, cs), conf->timings);
+ regmap_write(regmap, ATMEL_HSMC_MODE(layout, cs), conf->mode);
}
EXPORT_SYMBOL_GPL(atmel_hsmc_cs_conf_apply);
@@ -297,18 +299,55 @@ EXPORT_SYMBOL_GPL(atmel_smc_cs_conf_get);
* atmel_hsmc_cs_conf_get - retrieve the current SMC CS conf
* @regmap: the HSMC regmap
* @cs: the CS id
+ * @layout: the layout of registers
* @conf: the SMC CS conf object to store the current conf
*
* Retrieve the SMC CS configuration.
* Only valid on post-sama5 SoCs.
*/
-void atmel_hsmc_cs_conf_get(struct regmap *regmap, int cs,
- struct atmel_smc_cs_conf *conf)
+void atmel_hsmc_cs_conf_get(struct regmap *regmap,
+ const struct atmel_hsmc_reg_layout *layout,
+ int cs, struct atmel_smc_cs_conf *conf)
{
- regmap_read(regmap, ATMEL_HSMC_SETUP(cs), >setup);
- regmap_read(regmap, ATMEL_HSMC_PULSE(cs), >pulse);
- regmap_read(regmap,
Re: [PATCH RFC 0/2] KVM: x86: Support using the VMX preemption timer for APIC Timer periodic/oneshot mode
On 11/07/2017 02:13, Andy Lutomirski wrote: > On 10/11/2016 05:17 AM, Wanpeng Li wrote: >> Most windows guests which I have on hand currently still utilize APIC >> Timer >> periodic/oneshot mode instead of APIC Timer tsc-deadline mode: >> - windows 2008 server r2 >> - windows 2012 server r2 >> - windows 7 >> - windows 10 >> >> This patchset adds the support using the VMX preemption timer for APIC >> Timer >> periodic/oneshot mode. >> >> I add a print in oneshot mode testcase of kvm-unit-tests/apic.flat and >> observed >> that w/ patch the latency is ~2% of w/o patch. I think maybe something >> is still >> not right in the patchset, in addition, tmcct in apic_get_tmcct() >> maybe is not >> calculated correctly. Your comments to improve the patchset is a great >> appreciated. >> >> Wanpeng Li (2): >>KVM: lapic: Extract start_sw_period() to handle oneshot/periodic mode >>KVM: x86: Support using the vmx preemption timer for APIC Timer >> periodic/one mode >> >> arch/x86/kvm/lapic.c | 162 >> ++- >> 1 file changed, 95 insertions(+), 67 deletions(-) >> > > I think this is a step in the right direction, but I think there's a > different approach that would be much, much faster: use the VMX > preemption timer for *host* preemption. Specifically, do this: > > 1. Refactor the host TSC deadline timer a bit to allow the TSC deadline > timer to be "borrow". It might look something like this: > > u64 borrow_tsc_deadline(void (*timer_callback)()); > > The caller is now permitted to use the TSC deadline timer for its own > nefarious purposes. The caller promises to call return_tsc_deadline() > in a timely manner if the TSC exceeds the return value while the > deadline timer is borrowed. > > If the TSC deadline fires while it's borrowed, timer_callback() will be > called. > > void return_tsc_deadline(bool timer_fired); > > The caller is done borrowing the TSC deadline timer. The caller need > not reset the TSC deadline timer MSR to its previous value before > calling this. It must be called with IRQs on and preemption off. > > Getting this to work cleanly without races may be a bit tricky. So be it. > > 2. Teach KVM to use the VMX preemption timer as a substitute deadline > timer while in guest mode. Specifically, KVM will borrow_tsc_deadline() > (if TSC deadline is enabled) when entering guest mode and > return_tsc_deadline() when returning out of guest mode. > > 3. Now KVM can change its MSR bitmaps to allow the guest to program the > TSC deadline MSR directly. No exit at all needed to handle guest writes > to the deadline timer. This assumes that the TSC deadline MSR observes the guest TSC offset, which I'm not at all sure of. If you can't, you break live migration. Also, while it would halve the cost of a guest's programming of the timer tick, you would still incur the cost of a vmexit to call timer_callback (it would be different if you could program the TSC deadline timer to send a posted interrupt, of course). Things would be half as slow, but still a far cry from bare metal. Really, we should just ask Intel to virtualize the TSC deadline MSR. Paolo
Re: [PATCH v2 00/10] Constify attribute_group structures
On 07/11/2017 09:29 AM, Arvind Yadav wrote: > attribute_groups are not supposed to change at runtime. So mark the > non-const structs as const. [...] > drivers/net/can/at91_can.c | 2 +- > drivers/net/can/janz-ican3.c| 2 +- For the can drivers: Acked-by: Marc Kleine-Budde -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions| Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917- | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | signature.asc Description: OpenPGP digital signature
Re: [PATCH v4 3/3] KVM: nVMX: Emulate EPTP switching for the L1 hypervisor
On 10.07.2017 22:49, Bandan Das wrote:
> When L2 uses vmfunc, L0 utilizes the associated vmexit to
> emulate a switching of the ept pointer by reloading the
> guest MMU.
>
> Signed-off-by: Paolo Bonzini
> Signed-off-by: Bandan Das
> ---
> arch/x86/include/asm/vmx.h | 6 +
> arch/x86/kvm/vmx.c | 58
> +++---
> 2 files changed, 61 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/vmx.h b/arch/x86/include/asm/vmx.h
> index da5375e..5f63a2e 100644
> --- a/arch/x86/include/asm/vmx.h
> +++ b/arch/x86/include/asm/vmx.h
> @@ -115,6 +115,10 @@
> #define VMX_MISC_SAVE_EFER_LMA 0x0020
> #define VMX_MISC_ACTIVITY_HLT0x0040
>
> +/* VMFUNC functions */
> +#define VMX_VMFUNC_EPTP_SWITCHING 0x0001
> +#define VMFUNC_EPTP_ENTRIES 512
> +
> static inline u32 vmx_basic_vmcs_revision_id(u64 vmx_basic)
> {
> return vmx_basic & GENMASK_ULL(30, 0);
> @@ -200,6 +204,8 @@ enum vmcs_field {
> EOI_EXIT_BITMAP2_HIGH = 0x2021,
> EOI_EXIT_BITMAP3= 0x2022,
> EOI_EXIT_BITMAP3_HIGH = 0x2023,
> + EPTP_LIST_ADDRESS = 0x2024,
> + EPTP_LIST_ADDRESS_HIGH = 0x2025,
> VMREAD_BITMAP = 0x2026,
> VMWRITE_BITMAP = 0x2028,
> XSS_EXIT_BITMAP = 0x202C,
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index fe8f5fc..0a969fb 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -246,6 +246,7 @@ struct __packed vmcs12 {
> u64 eoi_exit_bitmap1;
> u64 eoi_exit_bitmap2;
> u64 eoi_exit_bitmap3;
> + u64 eptp_list_address;
> u64 xss_exit_bitmap;
> u64 guest_physical_address;
> u64 vmcs_link_pointer;
> @@ -771,6 +772,7 @@ static const unsigned short vmcs_field_to_offset_table[]
> = {
> FIELD64(EOI_EXIT_BITMAP1, eoi_exit_bitmap1),
> FIELD64(EOI_EXIT_BITMAP2, eoi_exit_bitmap2),
> FIELD64(EOI_EXIT_BITMAP3, eoi_exit_bitmap3),
> + FIELD64(EPTP_LIST_ADDRESS, eptp_list_address),
> FIELD64(XSS_EXIT_BITMAP, xss_exit_bitmap),
> FIELD64(GUEST_PHYSICAL_ADDRESS, guest_physical_address),
> FIELD64(VMCS_LINK_POINTER, vmcs_link_pointer),
> @@ -1402,6 +1404,13 @@ static inline bool nested_cpu_has_vmfunc(struct vmcs12
> *vmcs12)
> return nested_cpu_has2(vmcs12, SECONDARY_EXEC_ENABLE_VMFUNC);
> }
>
> +static inline bool nested_cpu_has_eptp_switching(struct vmcs12 *vmcs12)
> +{
> + return nested_cpu_has_vmfunc(vmcs12) &&
> + (vmcs12->vm_function_control &
I wonder if it makes sense to rename vm_function_control to
- vmfunc_control
- vmfunc_controls (so it matches nested_vmx_vmfunc_controls)
- vmfunc_ctrl
> + VMX_VMFUNC_EPTP_SWITCHING);
> +}
> +
> static inline bool is_nmi(u32 intr_info)
> {
> return (intr_info & (INTR_INFO_INTR_TYPE_MASK | INTR_INFO_VALID_MASK))
> @@ -2791,7 +2800,12 @@ static void nested_vmx_setup_ctls_msrs(struct vcpu_vmx
> *vmx)
> if (cpu_has_vmx_vmfunc()) {
> vmx->nested.nested_vmx_secondary_ctls_high |=
> SECONDARY_EXEC_ENABLE_VMFUNC;
> - vmx->nested.nested_vmx_vmfunc_controls = 0;
> + /*
> + * Advertise EPTP switching unconditionally
> + * since we emulate it
> + */
> + vmx->nested.nested_vmx_vmfunc_controls =
> + VMX_VMFUNC_EPTP_SWITCHING;> }
>
> /*
> @@ -7772,6 +7786,9 @@ static int handle_vmfunc(struct kvm_vcpu *vcpu)
> struct vcpu_vmx *vmx = to_vmx(vcpu);
> struct vmcs12 *vmcs12;
> u32 function = vcpu->arch.regs[VCPU_REGS_RAX];
> + u32 index = vcpu->arch.regs[VCPU_REGS_RCX];
> + struct page *page = NULL;
> + u64 *l1_eptp_list, address;
>
> /*
>* VMFUNC is only supported for nested guests, but we always enable the
> @@ -7784,11 +7801,46 @@ static int handle_vmfunc(struct kvm_vcpu *vcpu)
> }
>
> vmcs12 = get_vmcs12(vcpu);
> - if ((vmcs12->vm_function_control & (1 << function)) == 0)
> + if (((vmcs12->vm_function_control & (1 << function)) == 0) ||
> + WARN_ON_ONCE(function))
"... instruction causes a VM exit if the bit at position EAX is 0 in the
VM-function controls (the selected VM function is
not enabled)."
So g2 can trigger this WARN_ON_ONCE, no? I think we should drop it then
completely.
> + goto fail;
> +
> + if (!nested_cpu_has_ept(vmcs12) ||
> + !nested_cpu_has_eptp_switching(vmcs12))
> + goto fail;
> +
> + if (!vmcs12->eptp_list_address || index >= VMFUNC_EPTP_ENTRIES)
> + goto fail;
I can find the definition for an vmexit in case of index >=
VMFUNC_EPTP_ENTRIES, but not for !vmcs12->eptp_list_address in the SDM.
Can you give me a hint?
> +
> + page = nested_get_page(vcpu,
Re: [PATCH 3/3] ARM: dts: at91: sama5d2: use sama5d2 compatible string for SMC
On 11/07/2017 at 09:40, Ludovic Desroches wrote:
> A new compatible string has been introduced for sama5d2 SMC to allow to
> manage the registers mapping change.
>
> Signed-off-by: Ludovic Desroches
> ---
> arch/arm/boot/dts/sama5d2.dtsi | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
> index 4fcd5bb219e3..60e69aeacbdb 100644
> --- a/arch/arm/boot/dts/sama5d2.dtsi
> +++ b/arch/arm/boot/dts/sama5d2.dtsi
> @@ -1048,7 +1048,7 @@
> };
>
> hsmc: hsmc@f8014000 {
> - compatible = "atmel,sama5d3-smc", "syscon",
> "simple-mfd";
> + compatible = "atmel,sama5d2-smc", "syscon",
> "simple-mfd";
You'd better use something like:
compatible = "atmel,sama5d2-smc", "atmel,sama5d3-smc", "syscon", "simple-mfd";
So that this patch is independent from the rest of the series and
we can avoid to having to synchronize with mfd or mtd/nand for this part.
Regards,
> reg = <0xf8014000 0x1000>;
> interrupts = <17 IRQ_TYPE_LEVEL_HIGH 6>;
> clocks = <_clk>;
>
--
Nicolas Ferre
Re: [PATCH] drm: inhibit drm drivers register to uninitialized drm core
On Mon, Jul 10, 2017 at 09:56:20PM +0200, Alexandru Moise wrote:
> On Mon, Jul 10, 2017 at 08:00:37PM +0200, Daniel Vetter wrote:
> > On Mon, Jul 10, 2017 at 9:14 AM, Alexandru Moise
> > <00moses.alexande...@gmail.com> wrote:
> > > On Mon, Jul 10, 2017 at 08:52:46AM +0200, Daniel Vetter wrote:
> > >> On Sat, Jul 08, 2017 at 11:43:52PM +0200, Alexandru Moise wrote:
> > >> > If the DRM core fails to init for whatever reason, ensure that
> > >> > no driver ever calls drm_dev_register().
> > >> >
> > >> > This is best done at drm_dev_init() as it covers drivers that call
> > >> > drm_dev_alloc() as well as drivers that prefer to embed struct
> > >> > drm_device into their own device struct and call drm_dev_init()
> > >> > themselves.
> > >> >
> > >> > In my case I had so many dynamic device majors used that the major
> > >> > number for DRM (226) was stolen, causing DRM core init to fail after
> > >> > failing to register a chrdev, and ultimately calling debugfs_remove()
> > >> > on drm_debugfs_root in drm_core_exit().
> > >> >
> > >> > After drm core failed to init, VGEM was still calling
> > >> > drm_dev_register(),
> > >> > ultimately leading to drm_debugfs_init(), with drm_debugfs_root passed
> > >> > as the root for the new debugfs dir at debugfs_create_dir().
> > >> >
> > >> > This led to a kernel panic once we were either derefencing
> > >> > root->d_inode
> > >> > while it was NULL or calling root->d_inode->i_op->lookup() while it was
> > >> > NULL in debugfs at inode_lock() or lookup_*().
> > >> >
> > >> > Signed-off-by: Alexandru Moise <00moses.alexande...@gmail.com>
> > >> > ---
> > >> > drivers/gpu/drm/drm_drv.c | 16
> > >> > 1 file changed, 16 insertions(+)
> > >> >
> > >> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > >> > index 37b8ad3e30d8..2ed2d919beae 100644
> > >> > --- a/drivers/gpu/drm/drm_drv.c
> > >> > +++ b/drivers/gpu/drm/drm_drv.c
> > >> > @@ -63,6 +63,15 @@ module_param_named(debug, drm_debug, int, 0600);
> > >> > static DEFINE_SPINLOCK(drm_minor_lock);
> > >> > static struct idr drm_minors_idr;
> > >> >
> > >> > +/*
> > >> > + * If the drm core fails to init for whatever reason,
> > >> > + * we should prevent any drivers from registering with it.
> > >> > + * It's best to check this at drm_dev_init(), as some drivers
> > >> > + * prefer to embed struct drm_device into their own device
> > >> > + * structure and call drm_dev_init() themselves.
> > >> > + */
> > >> > +static bool drm_core_init_complete = false;
> > >> > +
> > >> > static struct dentry *drm_debugfs_root;
> > >> >
> > >> > #define DRM_PRINTK_FMT "[" DRM_NAME ":%s]%s %pV"
> > >> > @@ -484,6 +493,11 @@ int drm_dev_init(struct drm_device *dev,
> > >> > {
> > >> > int ret;
> > >> >
> > >> > + if (!drm_core_init_complete) {
> > >> > + DRM_ERROR("DRM core is not initialized\n");
> > >> > + return -ENODEV;
> > >> > + }
> > >> > +
> > >> > kref_init(>ref);
> > >> > dev->dev = parent;
> > >> > dev->driver = driver;
> > >> > @@ -966,6 +980,8 @@ static int __init drm_core_init(void)
> > >> > if (ret < 0)
> > >> > goto error;
> > >> >
> > >> > + drm_core_init_complete = true;
> > >> > +
> > >> > DRM_DEBUG("Initialized\n");
> > >> > return 0;
> > >>
> > >> Isn't the correct fix to pass down the error value, which iirc should
> > >> make the kmod stuff unload the module again? Or does this not work'
> > >> -Daniel
> > >
> > > What if everything is built in?
> >
> > I feared that would be the answer :-/ Still feels funny that everyone
> > will need to hand-roll this, or does everyone simply assume that their
> > subsystem's module_init never fails?
> >
> > Adding a pile of kmod and driver folks in the hopes of getting a
> > better answer. If there's no better answer pls remind me to merge your
> > patch in 1-2 weeks, I'll likely forget ...
> > -Daniel
>
> I took a look at all the DRM drivers and they all seem pretty sane,
> they all handle the case of the DRM core failure, the fault is clearly
> on the DRM core side, it makes absolutely no sense trying to register
> with a subsystem that failed to init.
>
> This is certainly not a subsystem I am intimately acquainted with
> however, I just stumbled on this crash while working with debugfs.
>
> Most of the DRM core code looks pretty solid, and this is a relatively
> small corner-case, I believe most people out there won't be eating up
> that many major numbers. But I believe it's still the Right Thing™ to do.
>
> It might save some headaches in the future as DRM becomes more complex.
I'm not worried about drm or drm drivers here, I'm just wondered how all
the other subystems handle this same problem. Pretty sure we're not the
only ones who register a chardev block in the core subsystem module (and
sysfs classes, and tons of other stuff), and then drivers would fall over
if that's not there. Having to open code a $foo_init_successful everywhere
seems somewhat
Re: [RFC][PATCH] drm: kirin: Restrict modes to known good mode clocks
On Mon, Jul 10, 2017 at 03:47:54PM -0700, John Stultz wrote:
> On Mon, Jul 10, 2017 at 2:18 PM, Sean Paul wrote:
> > On Mon, Jul 10, 2017 at 01:48:02PM -0700, John Stultz wrote:
> >> Currently the hikey dsi logic cannot generate accurate byte
> >> clocks values for all pixel clock values. Thus if a mode clock
> >> is selected that cannot match the calculated byte clock, the
> >> device will boot with a blank screen.
> >>
> >> This patch uses the new mode_valid callback (many thanks to
> >> Jose Abreu for upstreaming it!) to enforces known good mode
> >> clocks for well known resolutions, which should allow the
> >> display to work from given EDID options, and ensures for a given
> >> resolution & refresh, the right mode clock is selected.
> >>
> >> Cc: Daniel Vetter
> >> Cc: Jani Nikula
> >> Cc: Sean Paul
> >> Cc: David Airlie
> >> Cc: Rob Clark
> >> Cc: Xinliang Liu
> >> Cc: Xinliang Liu
> >> Cc: Rongrong Zou
> >> Cc: Xinwei Kong
> >> Cc: Chen Feng
> >> Cc: Jose Abreu
> >> Cc: Archit Taneja
> >> Cc: dri-de...@lists.freedesktop.org
> >> Signed-off-by: John Stultz
> >> ---
> >> drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c | 37
> >>
> >> 1 file changed, 37 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> >> b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> >> index f77dcfa..a84f4bb 100644
> >> --- a/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> >> +++ b/drivers/gpu/drm/hisilicon/kirin/dw_drm_dsi.c
> >> @@ -603,6 +603,42 @@ static void dsi_encoder_enable(struct drm_encoder
> >> *encoder)
> >> dsi->enable = true;
> >> }
> >>
> >> +static enum drm_mode_status dsi_encoder_mode_valid(struct drm_encoder
> >> *crtc,
> >> + const struct drm_display_mode *mode)
> >> +{
> >> + /*
> >> + * kirin cannot generate all modes, so use the whitelist below
> >> + */
> >> + DRM_DEBUG("%s: Checking mode %ix%i@%i clock: %i...", __func__,
> >> + mode->hdisplay, mode->vdisplay, drm_mode_vrefresh(mode),
> >> mode->clock);
> >> + if ((mode->hdisplay == 1920 && mode->vdisplay == 1080 && mode->clock
> >> == 148500) ||
> >> + (mode->hdisplay == 1920 && mode->vdisplay == 1080 && mode->clock
> >> == 80192) ||
> >> + (mode->hdisplay == 1920 && mode->vdisplay == 1080 && mode->clock
> >> == 74250) ||
> >> + (mode->hdisplay == 1920 && mode->vdisplay == 1080 && mode->clock
> >> == 61855) ||
> >> + (mode->hdisplay == 1680 && mode->vdisplay == 1050 && mode->clock
> >> == 147116) ||
> >> + (mode->hdisplay == 1680 && mode->vdisplay == 1050 && mode->clock
> >> == 146250) ||
> >> + (mode->hdisplay == 1680 && mode->vdisplay == 1050 && mode->clock
> >> == 144589) ||
> >> + (mode->hdisplay == 1600 && mode->vdisplay == 1200 && mode->clock
> >> == 160961) ||
> >> + (mode->hdisplay == 1600 && mode->vdisplay == 900 && mode->clock
> >> == 118963) ||
> >> + (mode->hdisplay == 1440 && mode->vdisplay == 900 && mode->clock
> >> == 126991) ||
> >> + (mode->hdisplay == 1280 && mode->vdisplay == 1024 && mode->clock
> >> == 128946) ||
> >> + (mode->hdisplay == 1280 && mode->vdisplay == 1024 && mode->clock
> >> == 98619) ||
> >> + (mode->hdisplay == 1280 && mode->vdisplay == 960 && mode->clock
> >> == 102081) ||
> >> + (mode->hdisplay == 1280 && mode->vdisplay == 800 && mode->clock
> >> == 83496) ||
> >> + (mode->hdisplay == 1280 && mode->vdisplay == 720 && mode->clock
> >> == 74440) ||
> >> + (mode->hdisplay == 1280 && mode->vdisplay == 720 && mode->clock
> >> == 74250) ||
> >> + (mode->hdisplay == 1024 && mode->vdisplay == 768 && mode->clock
> >> == 78800) ||
> >> + (mode->hdisplay == 1024 && mode->vdisplay == 768 && mode->clock
> >> == 75000) ||
> >> + (mode->hdisplay == 1024 && mode->vdisplay == 768 && mode->clock
> >> == 81833) ||
> >> + (mode->hdisplay == 800 && mode->vdisplay == 600 && mode->clock
> >> == 48907) ||
> >> + (mode->hdisplay == 800 && mode->vdisplay == 600 && mode->clock
> >> == 4)) {
> >
> > Bikeshed incoming:
> >
> > Can you break this out into a lookup table? It's kind of long-winded as-is.
> > It'd
>
> That's fair. The list has slowly grown from 4 or so modes to what it
> is now, so it is a bit longer then it was originally.
>
> I'll spin the patches with that change.
>
> > be even better if you could calculate whether the mode is valid, but I
> > didn't
> > spend enough time to figure out if this is possible.
>
> Theoretically that might be possible, checking if the requested freq
> matches the calculated freq, and I've tried that but so far I've not
> been able to get it to work, as in some cases the freq on the current
> whitelist don't exactly match but do work on the large majority of
> monitors tested (while other freq have a similar error but don't work
> on most
Re: [PATCH 3/3] ARM: dts: at91: sama5d2: use sama5d2 compatible string for SMC
On Tue, Jul 11, 2017 at 09:52:58AM +0200, Nicolas Ferre wrote:
> On 11/07/2017 at 09:40, Ludovic Desroches wrote:
> > A new compatible string has been introduced for sama5d2 SMC to allow to
> > manage the registers mapping change.
> >
> > Signed-off-by: Ludovic Desroches
> > ---
> > arch/arm/boot/dts/sama5d2.dtsi | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
> > index 4fcd5bb219e3..60e69aeacbdb 100644
> > --- a/arch/arm/boot/dts/sama5d2.dtsi
> > +++ b/arch/arm/boot/dts/sama5d2.dtsi
> > @@ -1048,7 +1048,7 @@
> > };
> >
> > hsmc: hsmc@f8014000 {
> > - compatible = "atmel,sama5d3-smc", "syscon",
> > "simple-mfd";
> > + compatible = "atmel,sama5d2-smc", "syscon",
> > "simple-mfd";
>
> You'd better use something like:
> compatible = "atmel,sama5d2-smc", "atmel,sama5d3-smc", "syscon",
> "simple-mfd";
But it's not true, during data interface setup, we will write values in
the wrong place if we fallback on "atmel,sama5d3-smc".
Regards
Ludovic
>
> So that this patch is independent from the rest of the series and
> we can avoid to having to synchronize with mfd or mtd/nand for this part.
>
> Regards,
>
> > reg = <0xf8014000 0x1000>;
> > interrupts = <17 IRQ_TYPE_LEVEL_HIGH 6>;
> > clocks = <_clk>;
> >
>
>
> --
> Nicolas Ferre
Re: [PATCH v4 01/14] drm/atomic: export drm_atomic_replace_property_blob
On Thu, Jul 06, 2017 at 02:20:35PM +0200, Peter Rosin wrote:
> While at it, add some words in the kernel-doc about the 'replaced' arg and
> remove a faulty kernel-doc comment on the return value.
>
> Also remove a redundant return statement.
>
> Signed-off-by: Peter Rosin
> ---
> drivers/gpu/drm/drm_atomic.c | 17 +
> include/drm/drm_atomic.h | 4
> 2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 09ca662..b7d9696 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -414,13 +414,15 @@ EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
> * @new_blob: the new blob to replace with
> * @replaced: whether the blob has been replaced
> *
> - * RETURNS:
> - * Zero on success, error code on failure
> + * Note that you are required to initialize @replaced to false before the
> + * call, since it is only set to true when the blob property is changed and
> + * not set to false when the property is not changed. This enables a series
> + * of calls to be made where you are interested in if any property is
> + * replaced, but not care so much about exactly which of them was replaced.
> */
> -static void
> -drm_atomic_replace_property_blob(struct drm_property_blob **blob,
> - struct drm_property_blob *new_blob,
> - bool *replaced)
> +void drm_atomic_replace_property_blob(struct drm_property_blob **blob,
> + struct drm_property_blob *new_blob,
> + bool *replaced)
Yes I know I'm super-annoying, but this function now feels misplaced. It
has nothing to do with atomic, it just replaces a pointer to a blob with
anther pointer. I think it'd be much better if we move this function to
drm_property.c, and rename it to drm_property_replace_blob.
Second, instead of typing a huge paragraph explaining how replaced works,
I think the better option would be to drop that parameter and instead
return a boolean indicating whether the blob was replaced or not.
That's a bit more work, but imo functions which are exported need to pass
a higher barrier wrt api polish.
Thanks, Daniel
> {
> struct drm_property_blob *old_blob = *blob;
>
> @@ -432,9 +434,8 @@ drm_atomic_replace_property_blob(struct drm_property_blob
> **blob,
> drm_property_blob_get(new_blob);
> *blob = new_blob;
> *replaced = true;
> -
> - return;
> }
> +EXPORT_SYMBOL(drm_atomic_replace_property_blob);
>
> static int
> drm_atomic_replace_property_blob_from_id(struct drm_device *dev,
> diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
> index dcc8e0c..8b32ea5 100644
> --- a/include/drm/drm_atomic.h
> +++ b/include/drm/drm_atomic.h
> @@ -321,6 +321,10 @@ int drm_atomic_connector_set_property(struct
> drm_connector *connector,
> struct drm_connector_state *state, struct drm_property
> *property,
> uint64_t val);
>
> +void drm_atomic_replace_property_blob(struct drm_property_blob **blob,
> + struct drm_property_blob *new_blob,
> + bool *replaced);
> +
> void * __must_check
> drm_atomic_get_private_obj_state(struct drm_atomic_state *state,
> void *obj,
> --
> 2.1.4
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH v4 02/14] drm/atomic-helper: update lut props directly in ..._legacy_gamma_set
On Thu, Jul 06, 2017 at 02:20:36PM +0200, Peter Rosin wrote:
> Do not waste cycles looking up the property id when we have the
> actual property already.
>
> Signed-off-by: Peter Rosin
With the names adjusted per my comments on patch 1 this lgtm. Btw good
practice is to cc original authors of the code, a combo of git blame and
scripts/get_maintainers.pl helps with that.
-Daniel
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 23 ---
> 1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index 667ec97..5a4a344 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -3769,11 +3769,11 @@ int drm_atomic_helper_legacy_gamma_set(struct
> drm_crtc *crtc,
> struct drm_modeset_acquire_ctx *ctx)
> {
> struct drm_device *dev = crtc->dev;
> - struct drm_mode_config *config = >mode_config;
> struct drm_atomic_state *state;
> struct drm_crtc_state *crtc_state;
> struct drm_property_blob *blob = NULL;
> struct drm_color_lut *blob_data;
> + bool replaced = false;
> int i, ret = 0;
>
> state = drm_atomic_state_alloc(crtc->dev);
> @@ -3805,20 +3805,13 @@ int drm_atomic_helper_legacy_gamma_set(struct
> drm_crtc *crtc,
> }
>
> /* Reset DEGAMMA_LUT and CTM properties. */
> - ret = drm_atomic_crtc_set_property(crtc, crtc_state,
> - config->degamma_lut_property, 0);
> - if (ret)
> - goto fail;
> -
> - ret = drm_atomic_crtc_set_property(crtc, crtc_state,
> - config->ctm_property, 0);
> - if (ret)
> - goto fail;
> -
> - ret = drm_atomic_crtc_set_property(crtc, crtc_state,
> - config->gamma_lut_property, blob->base.id);
> - if (ret)
> - goto fail;
> + drm_atomic_replace_property_blob(_state->degamma_lut,
> + NULL, );
> + drm_atomic_replace_property_blob(_state->ctm,
> + NULL, );
> + drm_atomic_replace_property_blob(_state->gamma_lut,
> + blob, );
> + crtc_state->color_mgmt_changed |= replaced;
>
> ret = drm_atomic_commit(state);
>
> --
> 2.1.4
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH 3/3] ARM: dts: at91: sama5d2: use sama5d2 compatible string for SMC
On 11/07/2017 at 09:58, Ludovic Desroches wrote:
> On Tue, Jul 11, 2017 at 09:52:58AM +0200, Nicolas Ferre wrote:
>> On 11/07/2017 at 09:40, Ludovic Desroches wrote:
>>> A new compatible string has been introduced for sama5d2 SMC to allow to
>>> manage the registers mapping change.
>>>
>>> Signed-off-by: Ludovic Desroches
>>> ---
>>> arch/arm/boot/dts/sama5d2.dtsi | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm/boot/dts/sama5d2.dtsi b/arch/arm/boot/dts/sama5d2.dtsi
>>> index 4fcd5bb219e3..60e69aeacbdb 100644
>>> --- a/arch/arm/boot/dts/sama5d2.dtsi
>>> +++ b/arch/arm/boot/dts/sama5d2.dtsi
>>> @@ -1048,7 +1048,7 @@
>>> };
>>>
>>> hsmc: hsmc@f8014000 {
>>> - compatible = "atmel,sama5d3-smc", "syscon",
>>> "simple-mfd";
>>> + compatible = "atmel,sama5d2-smc", "syscon",
>>> "simple-mfd";
>>
>> You'd better use something like:
>> compatible = "atmel,sama5d2-smc", "atmel,sama5d3-smc", "syscon",
>> "simple-mfd";
>
> But it's not true, during data interface setup, we will write values in
> the wrong place if we fallback on "atmel,sama5d3-smc".
Yes, I wrote too quickly: sorry for the noise ;-)
Bye,
>> So that this patch is independent from the rest of the series and
>> we can avoid to having to synchronize with mfd or mtd/nand for this part.
>>
>> Regards,
>>
>>> reg = <0xf8014000 0x1000>;
>>> interrupts = <17 IRQ_TYPE_LEVEL_HIGH 6>;
>>> clocks = <_clk>;
>>>
>>
>>
>> --
>> Nicolas Ferre
>
--
Nicolas Ferre
Re: [PATCH 2/3] mfd: syscon: atmel-smc: add helper to retrieve register layout
On Tue, 11 Jul 2017 09:40:14 +0200
Ludovic Desroches wrote:
> For HSMC controller, the register layout depends on the device i.e. the
> offset of setup, pulse, cycle, mode and timings registers is not the
> same. An helper is added to provide the correct register layout.
>
> Fixes: fe9d7cb22ef3 ("mfd: syscon: atmel-smc: Add new helpers to ease
> SMC regs manipulation")
> Suggested-by: Boris Brezillon
> Signed-off-by: Ludovic Desroches
Acked-by: Boris Brezillon
> ---
> drivers/memory/atmel-ebi.c | 13 +--
> drivers/mfd/atmel-smc.c | 67
> +---
> drivers/mtd/nand/atmel/nand-controller.c | 10 +++--
> include/linux/mfd/syscon/atmel-smc.h | 32 ++-
> 4 files changed, 92 insertions(+), 30 deletions(-)
>
> diff --git a/drivers/memory/atmel-ebi.c b/drivers/memory/atmel-ebi.c
> index 99e644cda4d1..63c9e7a76854 100644
> --- a/drivers/memory/atmel-ebi.c
> +++ b/drivers/memory/atmel-ebi.c
> @@ -51,6 +51,7 @@ struct atmel_ebi {
> struct {
> struct regmap *regmap;
> struct clk *clk;
> + const struct atmel_hsmc_reg_layout *layout;
> } smc;
>
> struct device *dev;
> @@ -84,8 +85,8 @@ static void at91sam9_ebi_get_config(struct atmel_ebi_dev
> *ebid,
> static void sama5_ebi_get_config(struct atmel_ebi_dev *ebid,
>struct atmel_ebi_dev_config *conf)
> {
> - atmel_hsmc_cs_conf_get(ebid->ebi->smc.regmap, conf->cs,
> ->smcconf);
> + atmel_hsmc_cs_conf_get(ebid->ebi->smc.regmap, ebid->ebi->smc.layout,
> +conf->cs, >smcconf);
> }
>
> static const struct atmel_smc_timing_xlate timings_xlate_table[] = {
> @@ -285,8 +286,8 @@ static void at91sam9_ebi_apply_config(struct
> atmel_ebi_dev *ebid,
> static void sama5_ebi_apply_config(struct atmel_ebi_dev *ebid,
> struct atmel_ebi_dev_config *conf)
> {
> - atmel_hsmc_cs_conf_apply(ebid->ebi->smc.regmap, conf->cs,
> - >smcconf);
> + atmel_hsmc_cs_conf_apply(ebid->ebi->smc.regmap, ebid->ebi->smc.layout,
> + conf->cs, >smcconf);
> }
>
> static int atmel_ebi_dev_setup(struct atmel_ebi *ebi, struct device_node *np,
> @@ -525,6 +526,10 @@ static int atmel_ebi_probe(struct platform_device *pdev)
> if (IS_ERR(ebi->smc.regmap))
> return PTR_ERR(ebi->smc.regmap);
>
> + ebi->smc.layout = atmel_hsmc_get_reg_layout(smc_np);
> + if (IS_ERR(ebi->smc.layout))
> + return PTR_ERR(ebi->smc.layout);
> +
> ebi->smc.clk = of_clk_get(smc_np, 0);
> if (IS_ERR(ebi->smc.clk)) {
> if (PTR_ERR(ebi->smc.clk) != -ENOENT)
> diff --git a/drivers/mfd/atmel-smc.c b/drivers/mfd/atmel-smc.c
> index 954cf0f66a31..1ad44e63b511 100644
> --- a/drivers/mfd/atmel-smc.c
> +++ b/drivers/mfd/atmel-smc.c
> @@ -258,19 +258,21 @@ EXPORT_SYMBOL_GPL(atmel_smc_cs_conf_apply);
> * atmel_hsmc_cs_conf_apply - apply an SMC CS conf
> * @regmap: the HSMC regmap
> * @cs: the CS id
> + * @layout: the layout of registers
> * @conf the SMC CS conf to apply
> *
> * Applies an SMC CS configuration.
> * Only valid on post-sama5 SoCs.
> */
> -void atmel_hsmc_cs_conf_apply(struct regmap *regmap, int cs,
> - const struct atmel_smc_cs_conf *conf)
> +void atmel_hsmc_cs_conf_apply(struct regmap *regmap,
> + const struct atmel_hsmc_reg_layout *layout,
> + int cs, const struct atmel_smc_cs_conf *conf)
> {
> - regmap_write(regmap, ATMEL_HSMC_SETUP(cs), conf->setup);
> - regmap_write(regmap, ATMEL_HSMC_PULSE(cs), conf->pulse);
> - regmap_write(regmap, ATMEL_HSMC_CYCLE(cs), conf->cycle);
> - regmap_write(regmap, ATMEL_HSMC_TIMINGS(cs), conf->timings);
> - regmap_write(regmap, ATMEL_HSMC_MODE(cs), conf->mode);
> + regmap_write(regmap, ATMEL_HSMC_SETUP(layout, cs), conf->setup);
> + regmap_write(regmap, ATMEL_HSMC_PULSE(layout, cs), conf->pulse);
> + regmap_write(regmap, ATMEL_HSMC_CYCLE(layout, cs), conf->cycle);
> + regmap_write(regmap, ATMEL_HSMC_TIMINGS(layout, cs), conf->timings);
> + regmap_write(regmap, ATMEL_HSMC_MODE(layout, cs), conf->mode);
> }
> EXPORT_SYMBOL_GPL(atmel_hsmc_cs_conf_apply);
>
> @@ -297,18 +299,55 @@ EXPORT_SYMBOL_GPL(atmel_smc_cs_conf_get);
> * atmel_hsmc_cs_conf_get - retrieve the current SMC CS conf
> * @regmap: the HSMC regmap
> * @cs: the CS id
> + * @layout: the layout of registers
> * @conf: the SMC CS conf object to store the current conf
> *
> * Retrieve the SMC CS configuration.
> * Only valid on post-sama5 SoCs.
> */
> -void atmel_hsmc_cs_conf_get(struct regmap *regmap, int cs,
> - struct atmel_smc_cs_conf *conf)
> +void atmel_hsmc_cs_conf_get(struct regmap *regmap,
> + const struct atmel_hsmc_reg_layout
Re: [PATCH 3/3] ARM: dts: at91: sama5d2: use sama5d2 compatible string for SMC
On Tue, Jul 11, 2017 at 09:58:29AM +0200, Ludovic Desroches wrote:
> On Tue, Jul 11, 2017 at 09:52:58AM +0200, Nicolas Ferre wrote:
> > On 11/07/2017 at 09:40, Ludovic Desroches wrote:
> > > A new compatible string has been introduced for sama5d2 SMC to allow to
> > > manage the registers mapping change.
> > >
> > > Signed-off-by: Ludovic Desroches
> > > ---
> > > arch/arm/boot/dts/sama5d2.dtsi | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/boot/dts/sama5d2.dtsi
> > > b/arch/arm/boot/dts/sama5d2.dtsi
> > > index 4fcd5bb219e3..60e69aeacbdb 100644
> > > --- a/arch/arm/boot/dts/sama5d2.dtsi
> > > +++ b/arch/arm/boot/dts/sama5d2.dtsi
> > > @@ -1048,7 +1048,7 @@
> > > };
> > >
> > > hsmc: hsmc@f8014000 {
> > > - compatible = "atmel,sama5d3-smc", "syscon",
> > > "simple-mfd";
> > > + compatible = "atmel,sama5d2-smc", "syscon",
> > > "simple-mfd";
> >
> > You'd better use something like:
> > compatible = "atmel,sama5d2-smc", "atmel,sama5d3-smc", "syscon",
> > "simple-mfd";
>
> But it's not true, during data interface setup, we will write values in
> the wrong place if we fallback on "atmel,sama5d3-smc".
>
> Regards
>
> Ludovic
>
> >
> > So that this patch is independent from the rest of the series and
> > we can avoid to having to synchronize with mfd or mtd/nand for this part.
If Lee and Boris agree, maybe the series can go through AT91.
Regards
Ludovic
> >
> > Regards,
> >
> > > reg = <0xf8014000 0x1000>;
> > > interrupts = <17 IRQ_TYPE_LEVEL_HIGH 6>;
> > > clocks = <_clk>;
> > >
> >
> >
> > --
> > Nicolas Ferre
Re: [PATCH v4 03/14] drm/fb-helper: separate the fb_setcmap helper into atomic and legacy paths
On Thu, Jul 06, 2017 at 02:20:37PM +0200, Peter Rosin wrote:
> The legacy path implements setcmap in terms of crtc .gamma_set.
>
> The atomic path implements setcmap by directly updating the crtc gamma_lut
> property.
>
> This has a couple of benefits:
> - it makes the redundant fb helpers .load_lut, .gamma_set and .gamma_get
> completely obsolete. They are now unused and subject for removal.
> - atomic drivers that support clut modes get fbdev support for those from
> the drm core. This includes atmel-hlcdc, but perhaps others as well?
>
> Signed-off-by: Peter Rosin
> ---
> drivers/gpu/drm/drm_fb_helper.c | 232
>
> 1 file changed, 161 insertions(+), 71 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
> index 721511d..32d6ea1 100644
> --- a/drivers/gpu/drm/drm_fb_helper.c
> +++ b/drivers/gpu/drm/drm_fb_helper.c
> @@ -1195,27 +1195,6 @@ void drm_fb_helper_set_suspend_unlocked(struct
> drm_fb_helper *fb_helper,
> }
> EXPORT_SYMBOL(drm_fb_helper_set_suspend_unlocked);
>
> -static int setcolreg(struct drm_crtc *crtc, u16 red, u16 green,
> - u16 blue, u16 regno, struct fb_info *info)
> -{
> - struct drm_fb_helper *fb_helper = info->par;
> - struct drm_framebuffer *fb = fb_helper->fb;
> -
> - /*
> - * The driver really shouldn't advertise pseudo/directcolor
> - * visuals if it can't deal with the palette.
> - */
> - if (WARN_ON(!fb_helper->funcs->gamma_set ||
> - !fb_helper->funcs->gamma_get))
> - return -EINVAL;
> -
> - WARN_ON(fb->format->cpp[0] != 1);
> -
> - fb_helper->funcs->gamma_set(crtc, red, green, blue, regno);
> -
> - return 0;
> -}
> -
> static int setcmap_pseudo_palette(struct fb_cmap *cmap, struct fb_info *info)
> {
> u32 *palette = (u32 *)info->pseudo_palette;
> @@ -1248,57 +1227,140 @@ static int setcmap_pseudo_palette(struct fb_cmap
> *cmap, struct fb_info *info)
> return 0;
> }
>
> -/**
> - * drm_fb_helper_setcmap - implementation for _ops.fb_setcmap
> - * @cmap: cmap to set
> - * @info: fbdev registered by the helper
> - */
> -int drm_fb_helper_setcmap(struct fb_cmap *cmap, struct fb_info *info)
> +static int setcmap_legacy(struct fb_cmap *cmap, struct fb_info *info)
> {
> struct drm_fb_helper *fb_helper = info->par;
> - struct drm_device *dev = fb_helper->dev;
> - const struct drm_crtc_helper_funcs *crtc_funcs;
> - u16 *red, *green, *blue, *transp;
> struct drm_crtc *crtc;
> u16 *r, *g, *b;
> - int i, j, rc = 0;
> - int start;
> + int i, ret = 0;
>
> - if (oops_in_progress)
> - return -EBUSY;
> + drm_modeset_lock_all(fb_helper->dev);
> + for (i = 0; i < fb_helper->crtc_count; i++) {
> + crtc = fb_helper->crtc_info[i].mode_set.crtc;
> + if (!crtc->funcs->gamma_set || !crtc->gamma_size)
> + return -EINVAL;
>
> - mutex_lock(_helper->lock);
> - if (!drm_fb_helper_is_bound(fb_helper)) {
> - mutex_unlock(_helper->lock);
> - return -EBUSY;
> + if (cmap->start + cmap->len > crtc->gamma_size)
> + return -EINVAL;
> +
> + r = crtc->gamma_store;
> + g = r + crtc->gamma_size;
> + b = g + crtc->gamma_size;
> +
> + memcpy(r + cmap->start, cmap->red, cmap->len * sizeof(*r));
> + memcpy(g + cmap->start, cmap->green, cmap->len * sizeof(*g));
> + memcpy(b + cmap->start, cmap->blue, cmap->len * sizeof(*b));
> +
> + ret = crtc->funcs->gamma_set(crtc, r, g, b,
> + crtc->gamma_size, NULL);
> + if (ret)
> + return ret;
> }
> + drm_modeset_unlock_all(fb_helper->dev);
>
> - drm_modeset_lock_all(dev);
> - if (info->fix.visual == FB_VISUAL_TRUECOLOR) {
> - rc = setcmap_pseudo_palette(cmap, info);
> - goto out;
> + return ret;
> +}
> +
> +static struct drm_property_blob *setcmap_new_gamma_lut(struct drm_crtc *crtc,
> +struct fb_cmap *cmap)
> +{
> + struct drm_device *dev = crtc->dev;
> + struct drm_property_blob *gamma_lut;
> + struct drm_color_lut *lut;
> + int size = crtc->gamma_size;
> + int i;
> +
> + if (!size || cmap->start + cmap->len > size)
> + return ERR_PTR(-EINVAL);
> +
> + gamma_lut = drm_property_create_blob(dev, sizeof(*lut) * size, NULL);
> + if (IS_ERR(gamma_lut))
> + return gamma_lut;
> +
> + lut = (struct drm_color_lut *)gamma_lut->data;
> + if (cmap->start || cmap->len != size) {
> + u16 *r = crtc->gamma_store;
> + u16 *g = r + crtc->gamma_size;
> + u16 *b = g + crtc->gamma_size;
> +
> + for (i = 0; i < cmap->start; i++) {
> +
Re: [PATCH] usb: isp1760: compress return logic into one line
Am Montag, den 10.07.2017, 14:58 -0500 schrieb Gustavo A. R. Silva : > Hi Oliver, > > Quoting Oliver Neukum : > > > > > Am Sonntag, den 09.07.2017, 21:00 -0500 schrieb Gustavo A. R. Silva : > > > > > > Simplify return logic to avoid unnecessary variable assignment. > > > > > > This issue was detected using Coccinelle and the following > > > semantic patch: > > > > > > > Hi, > > > > I need to ask: Where is the improvement? The compiler does not bother > > and for the human reader you do not do anything obvious and you > > decreased grepability. > > > > The declaration of local variable _retval_ was removed also. > So both, variable declaration and assignment removal are the improvements. Yeah, but a variable called "retval" has an extremely clear function. Simplifying code is an improvement. Making it clearer is an improvement. I am sorry but the proposed change is almost like removing blank lines to make it more compact > Regarding the greability, I think that depends on the context. "retval" has a clear function and is unique. Regards Oliver
Re: [PATCH v4 14/14] drm: remove unused and redundant callbacks
On Thu, Jul 06, 2017 at 02:20:48PM +0200, Peter Rosin wrote:
> Drivers no longer have any need for these callbacks, and there are no
> users. Zap. Zap-zap-zzzap-p-pp-p.
>
> Signed-off-by: Peter Rosin
On patches 4-14: Acked-by: Daniel Vetter
I'll try to haggle for a few more reviews by maintainers as soon as the
first 3 patches have landed, but I think I'll pull them all in about a
month or so latest. Please remind me in case I forget to do that (which is
likely ...).
Thanks a lot for doing this.
-Daniel
> ---
> include/drm/drm_crtc.h | 8
> include/drm/drm_fb_helper.h | 32
>
> include/drm/drm_modeset_helper_vtables.h | 16
> 3 files changed, 56 deletions(-)
>
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 3a911a6..0cc8962 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -358,14 +358,6 @@ struct drm_crtc_funcs {
>* drm_crtc_enable_color_mgmt(), which then supports the legacy gamma
>* interface through the drm_atomic_helper_legacy_gamma_set()
>* compatibility implementation.
> - *
> - * NOTE:
> - *
> - * Drivers that support gamma tables and also fbdev emulation through
> - * the provided helper library need to take care to fill out the gamma
> - * hooks for both. Currently there's a bit an unfortunate duplication
> - * going on, which should eventually be unified to just one set of
> - * hooks.
>*/
> int (*gamma_set)(struct drm_crtc *crtc, u16 *r, u16 *g, u16 *b,
>uint32_t size,
> diff --git a/include/drm/drm_fb_helper.h b/include/drm/drm_fb_helper.h
> index ea170b9..21c5630 100644
> --- a/include/drm/drm_fb_helper.h
> +++ b/include/drm/drm_fb_helper.h
> @@ -85,38 +85,6 @@ struct drm_fb_helper_surface_size {
> */
> struct drm_fb_helper_funcs {
> /**
> - * @gamma_set:
> - *
> - * Set the given gamma LUT register on the given CRTC.
> - *
> - * This callback is optional.
> - *
> - * FIXME:
> - *
> - * This callback is functionally redundant with the core gamma table
> - * support and simply exists because the fbdev hasn't yet been
> - * refactored to use the core gamma table interfaces.
> - */
> - void (*gamma_set)(struct drm_crtc *crtc, u16 red, u16 green,
> - u16 blue, int regno);
> - /**
> - * @gamma_get:
> - *
> - * Read the given gamma LUT register on the given CRTC, used to save the
> - * current LUT when force-restoring the fbdev for e.g. kdbg.
> - *
> - * This callback is optional.
> - *
> - * FIXME:
> - *
> - * This callback is functionally redundant with the core gamma table
> - * support and simply exists because the fbdev hasn't yet been
> - * refactored to use the core gamma table interfaces.
> - */
> - void (*gamma_get)(struct drm_crtc *crtc, u16 *red, u16 *green,
> - u16 *blue, int regno);
> -
> - /**
>* @fb_probe:
>*
>* Driver callback to allocate and initialize the fbdev info structure.
> diff --git a/include/drm/drm_modeset_helper_vtables.h
> b/include/drm/drm_modeset_helper_vtables.h
> index 0656984..6cdcb42 100644
> --- a/include/drm/drm_modeset_helper_vtables.h
> +++ b/include/drm/drm_modeset_helper_vtables.h
> @@ -267,22 +267,6 @@ struct drm_crtc_helper_funcs {
> enum mode_set_atomic);
>
> /**
> - * @load_lut:
> - *
> - * Load a LUT prepared with the _fb_helper_funcs.gamma_set vfunc.
> - *
> - * This callback is optional and is only used by the fbdev emulation
> - * helpers.
> - *
> - * FIXME:
> - *
> - * This callback is functionally redundant with the core gamma table
> - * support and simply exists because the fbdev hasn't yet been
> - * refactored to use the core gamma table interfaces.
> - */
> - void (*load_lut)(struct drm_crtc *crtc);
> -
> - /**
>* @disable:
>*
>* This callback should be used to disable the CRTC. With the atomic
> --
> 2.1.4
>
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [GIT PULL 0/3] perf/urgent fixes
* Arnaldo Carvalho de Melo wrote: > Hi Ingo, > > Please consider pulling, the exclude_kernel one is a nasty brown paper > bag one, I'd say :-\ > > - Arnaldo > > Test results at the end of this message, as usual. > > The following changes since commit dbf580623d5fee785218d1a47a2bcdf36d85c0e9: > > kprobes: Ensure that jprobe probepoints are at function entry (2017-07-08 > 11:05:35 +0200) > > are available in the git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git > tags/perf-urgent-for-mingo-4.13-20170710 > > for you to fetch changes up to 80f62589fa52f530cffc50e78c0b5a2ae572d61e: > > perf annotate: Fix broken arrow at row 0 connecting jmp instruction to its > target (2017-07-10 16:36:40 -0300) > > > perf/urgent fixes: > > - Fix attr.exclude_kernel setting for default cycles:p (Arnaldo Carvalho de > Melo) > > - Fix broken arrow at row 0 connecting jmp instruction to its target in > the annotate TUI (Jin Yao) > > Signed-off-by: Arnaldo Carvalho de Melo > > > Arnaldo Carvalho de Melo (2): > perf evsel: Fix attr.exclude_kernel setting for default cycles:p > perf evsel: State in the default event name if attr.exclude_kernel is > set > > Jin Yao (1): > perf annotate: Fix broken arrow at row 0 connecting jmp instruction to > its target > > tools/perf/ui/browser.c | 2 +- > tools/perf/util/evsel.c | 8 +--- > 2 files changed, 6 insertions(+), 4 deletions(-) Pulled, thanks a lot Arnaldo! Ingo
Re: [PATCH] KVM: s390: Fix KVM_S390_GET_CMMA_BITS ioctl definition
On 07/10/2017 11:23 PM, Gleb Fotengauer-Malinovskiy wrote:
> On Mon, Jul 10, 2017 at 08:43:12PM +0200, Christian Borntraeger wrote:
>> On 07/10/2017 04:44 PM, Gleb Fotengauer-Malinovskiy wrote:
>>> This ioctl actually writes to parameter too.
>>
>> Maybe rephrase that to:
>> The kernel does not only read struct kvm_s390_cmma_log for
>> KVM_S390_GET_CMMA_BITS,
>> it also writes back a return value making this _IOWR instead of _IOW.
>
> Ok, see v2.
>
>>> Fixes: 4036e387 ("KVM: s390: ioctls to get and set guest storage
>>> attributes")
>>> Signed-off-by: Gleb Fotengauer-Malinovskiy
>> Acked-by: Christian Borntraeger
>>
>>
>> Out of curiosity, how did you notice that?
>
> I regenerated strace's ioctl lists. It was obvious from the diff that
> *GET* and *SET* could not be both _IOC_WRITE.
>
In fact we do have multiple GET/SET ioctls in KVM, where both provide a control
block that is _IOC_WRITE only. That control block then has an address that will
be read/written to depending on get/set.
E.g. look at
#define KVM_SET_DEVICE_ATTR _IOW(KVMIO, 0xe1, struct kvm_device_attr)
#define KVM_GET_DEVICE_ATTR _IOW(KVMIO, 0xe2, struct kvm_device_attr)
but as far as I understand, the direction hints only qualify the referenced
struct and not the side effects. So for KVM_*_DEVICE_ATTR it is correct to have
IOW for both cases.
But for GET_CMMA we do indeed write back data.
Paolo, Radim,
if we want to fix the direction hint, it would be good to merge this in as soon
as possible. The new interface was added during this merge window.
Re: A udev rule to serve the change event of ACPI container?
On Mon 26-06-17 10:59:07, Michal Hocko wrote: > On Mon 26-06-17 14:26:57, Joey Lee wrote: > > Hi all, > > > > If ACPI received ejection request for a ACPI container, kernel > > emits KOBJ_CHANGE uevent when it found online children devices > > below the acpi container. > > > > Base on the description of caa73ea15 kernel patch, user space > > is expected to offline all devices below the container and the > > container itself. Then, user space can finalize the removal of > > the container with the help of its ACPI device object's eject > > attribute in sysfs. > > > > That means that kernel relies on users space to peform the offline > > and ejection jobs to acpi container and children devices. The > > discussion is here: > > https://lkml.org/lkml/2013/11/28/520 > > > > The mail loop didn't explain why the userspace is responsible for > > the whole container offlining. Is it possible to do that transparently > > from the kernel? What's the difference between offlining memory and > > processors which happends without any cleanup and container which > > does essentially the same except it happens at once? > > > > - After a couple of years, can we let the container hot-remove > >process transparently? > > - Except udev rule, does there have any other mechanism to trigger > >auto offline/ejection? > > I would be also interested whether the kernel can simply send an udev event > to all devices in the container. Any opinion on this? -- Michal Hocko SUSE Labs
Re: [PATCH] serial: 8250_of: Fix regression in reset support
Hi Linus,
On Mon, 2017-07-10 at 17:24 +0200, Linus Walleij wrote:
> commit e2860e1f62f2 ("serial: 8250_of: Add reset support")
> introduced reset support for the 8250_of driver.
>
> However it unconditionally uses the assert/deassert pair to
> deassert reset on the device at probe and assert it at
> remove. This does not work with systems that have a
> self-deasserting reset controller, such as Gemini, that
> recently added a reset controller.
Self-deasserting resets are fundamentally incompatible with
assert/deassert in the current shared reset model (as there is no
framework for deferring or or vetoing reset assertions while one of the
devices sharing the reset line could be negatively affected).
The way it is implemented right now, after reset() is triggered once,
assert/deassert will return -EINVAL for the lifetime of the reset
control instance. Also, reset() returns -EINVAL between another users'
deassert() and assert() calls.
> As a result, the console will not probe on the Gemini with
> this message:
>
> Serial: 8250/16550 driver, 1 ports, IRQ sharing disabled
> of_serial: probe of 4200.serial failed with error -524
>
> This (-ENOTSUPP) is the error code returned by the
> deassert() operation on self-deasserting reset controllers.
One easy workaround would be to implement deassert() to be a no-op (or
to divert the call to reset()) in your reset controller driver. A more
intrusive variant would be to do the same in the core, as you suggest
below.
> Add some code and comments that will:
>
> - In probe() avoid to bail out if -ENOTSUPP is returned
> from the reset_deassert() call.
I think it would be better not to put this burden on the drivers.
> - If reset_assert() bails out with -ENOTSUPP on remove(),
> try the self-deasserting method as a fallback.
I don't understand the purpose of this, see below.
> Cc: Joel Stanley
> Cc: Philipp Zabel
> Cc: Greg Kroah-Hartman
> Fixes: e2860e1f62f2 ("serial: 8250_of: Add reset support")
> Signed-off-by: Linus Walleij
> ---
> Philipp: please comment on this or ACK if it is the right
> approach. It sort of sets a precedent for handling
> different reset controllers from the consumer side.
>
> Another possibility is to modify the reset core such
> that .deassert() bails out silently if the controller only
> has .reset(), and .assert() just calls .reset() if the
> controller does not have .assert().
>
> Actually I think the latter is more intuitive but it is
> also more intrusive.
I have been thinking about this. Due to the diversity of reset
controller implementations, I'm hesitant to do this in the core.
For starters, why would reset() be called during assert() and not during
deassert()? Can we be sure that this is the right thing for all reset
controllers?
The deassert() call guarantees that the reset line is deasserted after
the call (and as long as deassert_count > 0). What if somebody ends up
building a self-deasserting reset controller that has the reset line
initially asserted?
Also it is implied that after the call to deassert() the device is in a
working state, obviously. This might only be the case after the first
reset() for some devices.
> ---
> drivers/tty/serial/8250/8250_of.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/tty/serial/8250/8250_of.c
> b/drivers/tty/serial/8250/8250_of.c
> index 0cf95fddccfc..927ee8561c8d 100644
> --- a/drivers/tty/serial/8250/8250_of.c
> +++ b/drivers/tty/serial/8250/8250_of.c
> @@ -138,7 +138,12 @@ static int of_platform_serial_setup(struct
> platform_device *ofdev,
> if (IS_ERR(info->rst))
> goto out;
> ret = reset_control_deassert(info->rst);
> - if (ret)
> + /*
> + * If the deassert operation is not supported, this could be because
> + * the reset controller is self-deasserting and onlt supports the
> + * .reset() operation, so this is not a probe error.
> + */
> + if (ret && (ret != -ENOTSUPP))
I don't like to have to do this in the drivers.
The driver just requests the reset line to be deasserted after the call,
if there is one. If the reset controller driver can guarantee that,
deassert() should not return an error.
> goto out;
>
> port->type = type;
> @@ -235,10 +240,14 @@ static int of_platform_serial_probe(struct
> platform_device *ofdev)
> static int of_platform_serial_remove(struct platform_device *ofdev)
> {
> struct of_serial_info *info = platform_get_drvdata(ofdev);
> + int ret;
>
> serial8250_unregister_port(info->line);
>
> - reset_control_assert(info->rst);
> + ret = reset_control_assert(info->rst);
> + /* If the assert operation is not supported, use pure reset() */
> + if (ret == -ENOTSUPP)
> + reset_control_reset(info->rst);
> if (info->clk)
> clk_disable_unprepare(info->clk);
> kfree(info);
What is the purpose of triggering the assert during remove?
I would
Re: Potential scheduler regression
* Ben Guthro wrote: > > If people have experience with these in the "enterprise" distros, or any > > other > > tree, and want to provide me with backported, and tested, patches, I'll be > > glad to consider them for stable kernels. > > > > thanks, > > > > greg k-h > > I tried to do a simple cherry-pick of the suggested patches - but they > apply against files that don't exist in the 4.9 series. I think there are only two strategies to maintain a backport which work in the long run: - insist on the simplest fixes and pure cherry-picks - or pick up _everything_ to sync up the two versions. The latter would mean a lot of commits - and I'm afraid it would also involve the scheduler header split-up, which literally involves hundreds of files plus perpetual build-breakage risk, so it's a no-no. > In my release of 4.9 - I'm planning on doing the simpler revert of 1b568f0aab > that introduced the performance degradation, rather than pulling in lots of > code > from newer kernels. That sounds much saner - I'd even Ack that approach for -stable as a special exception, than to complicate things with excessive backports. Thanks, Ingo
Re: [PATCH v2] printk: Modify operators of printed_len and text_len
On Tue 2017-07-11 14:40:55, Pierre Kuo wrote:
> With commit ("printk: report lost messages in printk
> safe/nmi contexts") and commit <8b1742c9c207> ("printk: remove zap_locks()
> function"), it seems we can remove initialization, "=0", of text_len and
> directly assign result of log_output to printed_len.
Just one small thing. The <> brackets should not be there around
the sha1 commit id. Otherwise, the patch looks fine to me.
Acked-by: Petr Mladek
You do not need to resend the patch. I'll remove the brackets
when applying the patch. I'll wait few more days just in case
anyone has more comments on this patch.
Best Regards,
Petr
Re: [PATCH v9 07/38] x86/mm: Remove phys_to_virt() usage in ioremap()
On Tue, Jul 11, 2017 at 6:58 AM, Brian Gerst wrote:
> On Mon, Jul 10, 2017 at 3:50 PM, Tom Lendacky wrote:
>> On 7/8/2017 7:57 AM, Brian Gerst wrote:
>>> On Fri, Jul 7, 2017 at 9:39 AM, Tom Lendacky
>>
>> I originally had a check for SME here in a previous version of the
>> patch. Thomas Gleixner recommended removing the check so that the code
>> path was always exercised regardless of the state of SME in order to
>> better detect issues:
>>
>> http://marc.info/?l=linux-kernel=149803067811436=2
>
> Looking a bit closer, this shortcut doesn't set the caching
> attributes. So it's probably best to get rid of it anyways. Also
> note, there is a corresponding check in iounmap().
Could that cause regressions if a driver relies on (write-through)
cacheable access to the VGA frame buffer RAM or an read-only
cached access to an option ROM but now gets uncached access?
I also tried to find out whether we can stop mapping the ISA MMIO
area into the linear mapping, but at least the VGA code uses
VGA_MAP_MEM() to get access to the same pointers. I'm pretty
sure this got copied incorrectly into most other architectures, but
it is definitely still used on x86 with vga16fb/vgacon/mdacon.
On the plus side, I see that removing this code path will end up
restoring MMIOTRACE support for the ISA MMIO range that was
apparently removed by accident in commit d61fc44853f4
("x86: mmiotrace, preview 2") in linux-2.6.27.
Arnd
Re: [PATCH v4 3/3] KVM: nVMX: Emulate EPTP switching for the L1 hypervisor
On 11/07/2017 09:51, David Hildenbrand wrote:
>> +static inline bool nested_cpu_has_eptp_switching(struct vmcs12 *vmcs12)
>> +{
>> +return nested_cpu_has_vmfunc(vmcs12) &&
>> +(vmcs12->vm_function_control &
>
> I wonder if it makes sense to rename vm_function_control to
> - vmfunc_control
> - vmfunc_controls (so it matches nested_vmx_vmfunc_controls)
> - vmfunc_ctrl
Blame Intel for this. :) They use full English names for VMCS fields (so
"VM-function control") and uppercase names for MSRs (so "IA32_VMX_VMFUNC").
"nested_vmx_vmfunc_controls" could be renamed to "nested_vmx_vmfunc" for
consistency, but I like the longer name too.
Paolo
Re: [PATCH v2 4/8] objtool: add undwarf debuginfo generation
* Josh Poimboeuf wrote: > Anyway, I used some linker magic to temporarily move the unwinder code to the > end of .text, so that unwinder changes don't add unexpected side effects to > the > microbenchmark behavior. Now I'm getting more consistent results: the packed > struct is measuring ~2% slower. The slight slowdown might just be explained > by > the fact that GCC generates some extra instructions for extracting the fields > out of the packed struct. Yeah, the 16-bit field accesses versus a zero-extended 32-bit field are more complex to access even on x86 that has a fair amount of 16-bit legacy. > In the meantime, I found a ~10% speedup by making the "fast lookup table" > block > size a power-of-two (256) to get rid of the need for a slow 'div' instruction. > > I think I'm done performance tweaking for now. I'll keep the packed struct, > and > add the code for the 'div' removal, and hope to submit v3 soon. Sounds good to me! ~2% slowdown for ~30% RAM savings for a debug data structure that is about as large as a typical kernel's total .text is a decent trade-off. Thanks, Ingo
Re: [PATCH] irqchip: gicv3-its: Use NUMA aware memory allocation for ITS tables
Hi Marc, On Mon, Jul 10, 2017 at 04:15:28PM +0100, Marc Zyngier wrote: > On 10/07/17 15:57, Shanker Donthineni wrote: > > Hi Marc, > > > > On 07/10/2017 08:50 AM, Marc Zyngier wrote: > >> On 10/07/17 11:21, Ganapatrao Kulkarni wrote: > >>> Hi Marc, > >>> > >>> On Mon, Jul 10, 2017 at 2:53 PM, Marc Zyngier > >>> wrote: > On 10/07/17 10:08, Ganapatrao Kulkarni wrote: > > On Mon, Jul 10, 2017 at 2:36 PM, Marc Zyngier > > wrote: > >> On 10/07/17 09:48, Ganapatrao Kulkarni wrote: > >>> Hi Marc, > >>> > >>> On Mon, Jul 3, 2017 at 8:23 PM, Marc Zyngier > >>> wrote: > Hi Shanker, > > On 03/07/17 15:24, Shanker Donthineni wrote: > > Hi Marc, > > > > On 06/30/2017 03:51 AM, Marc Zyngier wrote: > >> On 30/06/17 04:01, Ganapatrao Kulkarni wrote: > >>> On Fri, Jun 30, 2017 at 8:04 AM, Ganapatrao Kulkarni > >>> wrote: > Hi Shanker, > > On Sun, Jun 25, 2017 at 9:16 PM, Shanker Donthineni > wrote: > > The NUMA node information is visible to ITS driver but not > > being used > > other than handling errata. This patch allocates the memory for > > ITS > > tables from the corresponding NUMA node using the appropriate > > NUMA > > aware functions. > >>> > >>> IMHO, the description would have been more constructive? > >>> > >>> "All ITS tables are mapped by default to NODE 0 memory. > >>> Adding changes to allocate memory from respective NUMA NODES of > >>> ITS devices. > >>> This will optimize tables access and avoids unnecessary > >>> inter-node traffic." > >> > >> But more importantly, I'd like to see figures showing the actual > >> benefit > >> of this per-node allocation. Given that both of you guys have > >> access to > >> such platforms, please show me the numbers! > >> > > > > I'll share the actual results which shows the improvement whenever > > available on our next chips. Current version of Qualcomm qdf2400 > > doesn't > > support multi socket configuration to capture results and share > > with you. > > > > Do you see any other issues with this patch apart from the > > performance > > improvements. I strongly believe this brings the noticeable > > improvement > > in numbers on systems where it has multi node memory/CPU > > configuration. > > I agree that it *could* show an improvement, but it very much > depends on > how often the ITS misses in its caches. For this kind of patches, I > want > to see two things: > > 1) It brings a measurable benefit on NUMA platforms > >>> > >>> Did some measurement of interrupt response time for LPIs and we don't > >>> see any major > >>> improvement due to caching of Tables. However, we have seen > >>> improvements of around 5%. > >> > >> An improvement of what exactly? > > > > interrupt response time. > > Measured how? On which HW? Using which benchmark? > >>> > >>> This has been tested on ThunderX2. > >>> We have instrumented gic-v3-its driver code to create dummy LPI device > >>> with few vectors. > >>> The LPI is induced from dummy device(through sysfs by writing to > >>> TRANSLATOR reg). > >>> The ISR routine(gic_handle_irq) being called to handle the induced LPI. > >>> NODE 1 cpu is used to induce LPI and NODE 1 cpu/collection is mapped > >>> in ITT to route this LPI. > >>> > >>> CPU timer counter are sampled at the time LPI is Induced and in ISR > >>> routine to calculate interrupt response time. > >>> the result shown improvement of 5% with this patch. > >> > >> And you call that a realistic measurement of the latency? Really? Sorry, > >> but I cannot take you seriously here. > >> > >>> Do you have any recommended benchmarks to test the same? > >> > >> Run a standard benchmark such as netperf, post the result with and > >> without that patch. The above is just plain ridiculous. > >> > > > > The whole purpose of ACPI subtable "GIC Interrupt Translation Service (ITS) > > Affinity structure" > > is to provide the proximity information to OS so that software will take > > advantage of NUMA > > aware allocations to improve the read latency of ITS/GICR tables, not just > > for implementing > > software workarounds. > > > > > > I believe ITS driver should provide NUMA aware allocations just like x86 > > Linux drivers. How much > > performance improvement we observer is based on the individual SOC > > implementation, inter NODE > > latency, inter node traffic, cache capacity, and type of the test used to > > measure results. > > > > Please consider this patch
Re: [PATCH] gpu: host1x: Free the IOMMU domain when there is no device to attach
On Mon, 2017-07-10 at 21:33 +0200, Paul Kocialkowski wrote:
> When there is no device to attach to the IOMMU domain, as may be the
> case when the device-tree does not contain the proper iommu node, it
> is
> best to keep going without IOMMU support rather than failing.
> This allows the driver to probe and function instead of taking down
> all of the tegra drm driver, leading to missing display support.
Fixes: 404bfb78daf3 ("gpu: host1x: Add IOMMU support")
> Signed-off-by: Paul Kocialkowski
> ---
> drivers/gpu/host1x/dev.c | 8 +++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c
> index ac65f52850a6..f296738d0de8 100644
> --- a/drivers/gpu/host1x/dev.c
> +++ b/drivers/gpu/host1x/dev.c
> @@ -186,8 +186,13 @@ static int host1x_probe(struct platform_device
> *pdev)
> return -ENOMEM;
>
> err = iommu_attach_device(host->domain, >dev);
> - if (err)
> + if (err == -ENODEV) {
> + iommu_domain_free(host->domain);
> + host->domain = NULL;
> + goto iommu_skip;
> + } else if (err) {
> goto fail_free_domain;
> + }
>
> geometry = >domain->geometry;
>
> @@ -198,6 +203,7 @@ static int host1x_probe(struct platform_device
> *pdev)
> host->iova_end = geometry->aperture_end;
> }
>
> +iommu_skip:
> err = host1x_channel_list_init(host);
> if (err) {
> dev_err(>dev, "failed to initialize channel
> list\n");
--
Paul Kocialkowski,
developer of free digital technology and hardware support
Website: https://www.paulk.fr/
Coding blog: https://code.paulk.fr/
Git repositories: https://git.paulk.fr/ https://git.code.paulk.fr/
