Re: [RFC v2] ARC: allow to use IOC and non-IOC DMA devices simultaneously
On Thu, Jun 28, 2018 at 05:14:52PM +0300, Eugeniy Paltsev wrote:
> And if we get DMA buffer from ZONE_HIGHMEM memory we need to
> do real flush/invalidate operations on that buffer, which is obviously
> not done by "dma_direct_ops".
>
> So I am not sure about "dma_direct_ops" using - probably we need to
> create our special cache ops like "arc_ioc_ops" which will handle
> ZONE_HIGHMEM case.
FYI, I have a plan to merge dma_direct_ops and dma_noncoherent_ops
and make the decision to use cache flushing run time controllable
with a flag in struct device based on existing dynamic selection
in e.g. arm, arm64 and mips. It will probably take a few more merge
windows to get there, but once it is done you should be able to take
easy advantage of it.
> +++ b/arch/arc/mm/dma.c
> @@ -19,6 +19,7 @@
> #include
> #include
> #include
> +#include
dma-noncoherent.h already includes dma-mapping.h, so this is not
required.
> - if ((is_isa_arcv2() && ioc_enable) ||
> - (attrs & DMA_ATTR_NON_CONSISTENT))
> + if (attrs & DMA_ATTR_NON_CONSISTENT)
> need_coh = 0;
need_coh is only used twice, it might be cleaner to remove the
variable and just open code the DMA_ATTR_NON_CONSISTENT check.
And for extra points remove the need_kvaddr variable as well.
Also you probably want to do the equivalent change in arch_dma_free
as well.
> +void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> + const struct iommu_ops *iommu, bool coherent)
> +{
> + /*
> + * IOC hardware snoops all DMA traffic keeping the caches consistent
> + * with memory - eliding need for any explicit cache maintenance of
> + * DMA buffers - so we can use dma_direct cache ops.
> + */
> + if (is_isa_arcv2() && ioc_enable && coherent) {
> + set_dma_ops(dev, &dma_direct_ops);
> + dev_info(dev, "use dma_direct_ops cache ops\n");
> + } else {
> + set_dma_ops(dev, &dma_noncoherent_ops);
> + dev_info(dev, "use dma_noncoherent_ops cache ops\n");
> + }
> +}
Note that due to your use of asm-generic/dma-mapping.h we already
default to dma_noncoherent_ops if no per-device ops is set. So we
could skip the else branch here I think.
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [RFC v2] ARC: allow to use IOC and non-IOC DMA devices simultaneously
On 06/28/2018 07:14 AM, Eugeniy Paltsev wrote:
> The ARC HS processor provides an IOC port (I/O coherency bus
> interface) that allows external devices such as DMA devices
> to access memory through the cache hierarchy, providing
> coherency between I/O transactions and the complete memory
> hierarchy.
You mention IOC port in first line, but the change has nothing to do with it -
this is confusing.
Make this less verbose and just expand on the subject you have - global vs. per
device dma settings, without going into all the internal details.
And again going back to my original point, you are changing the semantics (ioc
and
non ioc to co-exist) but everything still goes thru IOC port. That is different
that what we had before - everything goes through IOC port and ioc dma ops ONLY
if
IOC existed.
I understand the need to isolate the 2 things and that is fine. But then what
you
should do is
1a. introduce the per ioc dma settings in ARC - and not touch the DTBs.
1b. introduce the IO port settings in platform code
2. Now switch the DT binding to use the per device ioc
And any panic etc for IOC + highmem etc should be a separate patch - so we can
easily revert it when time is right !
>
> Some recent SoC with ARC HS (like HSDK) allow to select bus
> port (IOC or non-IOC port) for connecting DMA devices in runtime.
>
> With this patch we can use both HW-coherent and regular DMA
> peripherals simultaneously.
>
> For example we can connect USB and SDIO controllers through IOC port
> (so we don't need to need to maintain cache coherency for these
> devices manualy. All cache sync ops will be nop)
> And we can connect Ethernet directly to RAM port (so we had to
> maintain cache coherency manualy. Cache sync ops will be real
> flush/invalidate operations)
I presume this has been tested with some stress testing bonie++ etc.
> +void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> + const struct iommu_ops *iommu, bool coherent)
> +{
> + /*
> + * IOC hardware snoops all DMA traffic keeping the caches consistent
> + * with memory - eliding need for any explicit cache maintenance of
> + * DMA buffers - so we can use dma_direct cache ops.
> + */
> + if (is_isa_arcv2() && ioc_enable && coherent) {
> + set_dma_ops(dev, &dma_direct_ops);
> + dev_info(dev, "use dma_direct_ops cache ops\n");
> + } else {
> + set_dma_ops(dev, &dma_noncoherent_ops);
> + dev_info(dev, "use dma_noncoherent_ops cache ops\n");
> + }
> +}
And further to Christoph's comment, if we decide to skip the branch, please add
a
comment that it is already set in generic code etc.
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH] ARC: Improve handling of fatal signals in do_page_fault()
This was triggered by investigation of a deadlock after OOM killer invocation,
see [1] for more details.
Looks like our handling of fatal signal in do_page_fault() has some issues:
1. We only want to do special (read "early") handling of fatal signal
if handle_mm_fault() returned VM_FAULT_RETRY so that we don't loop
in retry loop endlessly, otherwise we'll handle that signal normally
on exit from exception handler.
2. up_read() is not needed as indeed it will be done in __lock_page_or_retry()
in mm/filemap.c.
With above comments in mind simplified version should be like that:
--->8---
if (fatal_signal_pending(current)
if (fault & VM_FAULT_RETRY)
if (user_mode(regs))
return;
--->8---
But looks like there's a room for improvement, see [2].
Instead of proceeding forward and then inevitably hitting retry path we
short-cut right to kernel fix-up code in no_context.
[1]
http://lists.infradead.org/pipermail/linux-snps-arc/2018-February/003403.html
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=746a272e44141af24a02f6c9b0f65f4c4598ed42
Signed-off-by: Alexey Brodkin
---
arch/arc/mm/fault.c | 12
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/arc/mm/fault.c b/arch/arc/mm/fault.c
index a0b7bd6d030d..17ed78e2f5eb 100644
--- a/arch/arc/mm/fault.c
+++ b/arch/arc/mm/fault.c
@@ -139,12 +139,16 @@ void do_page_fault(unsigned long address, struct pt_regs
*regs)
*/
fault = handle_mm_fault(vma, address, flags);
- /* If Pagefault was interrupted by SIGKILL, exit page fault "early" */
+ /* If we need to retry but a fatal signal is pending, handle the
+* signal first. We do not need to release the mmap_sem because
+* it would already be released in __lock_page_or_retry in
+* mm/filemap.c. */
if (unlikely(fatal_signal_pending(current))) {
- if ((fault & VM_FAULT_ERROR) && !(fault & VM_FAULT_RETRY))
- up_read(&mm->mmap_sem);
- if (user_mode(regs))
+ if (fault & VM_FAULT_RETRY) {
+ if (!user_mode(regs))
+ goto no_context;
return;
+ }
}
perf_sw_event(PERF_COUNT_SW_PAGE_FAULTS, 1, regs, address);
--
2.17.1
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
[PATCH] ARC: prevent showing irrelevant exception info in signal message
We process signals in the end of syscall/exception handler.
It the signal is fatal we print register's content using
show_regs function. show_regs() also prints information about
last exception happened.
In case of multicore system we can catch the situation when we
will print wrong information about exception. See the example:
__
CPU-0: started to handle page fault
CPU-1: sent signal to process, which is executed on CPU-0
CPU-0: ended page fault handle. Started to process signal before
returnig to userspace. Process signal, which is send from
CPU-0. As th signal is fatal we call show_regs().
show_regs() will show information about last exception
which is *page fault* (instead of "trap" which is used for
signals and happened on CPU-0)
So we will get message like this:
/home/waitpid02
potentially unexpected fatal signal 8.
Path: /home/waitpid02
CPU: 0 PID: 100 Comm: waitpid02 Not tainted 4.10.0-rc4 #2
task: 9f11c200 task.stack: 9f3ae000
[ECR ]: 0x00050200 => Invalid Write @ 0x by insn @ 0x000123ec
[EFA ]: 0x
[BLINK ]: 0x123ea
[ERET ]: 0x123ec
@off 0x123ec in [/home/waitpid02]
VMA: 0x0001 to 0x00016000
[STAT32]: 0x80080882 : IE U
BTA: 0x000123ea SP: 0x5ffd3db0 FP: 0x
LPS: 0x20031684 LPE: 0x2003169a LPC: 0x0006
[-other-info-]
This message is confusing because it show information about page fault
( [ECR ]: 0x00050200 => Invalid Write ) which is absolutely irrelevant
to signal.
This situation was reproduced with waitpid02 LTP test.
_
So remove printing information about exceptions from show_regs()
to avoid confusing messages. Print information about exceptions
only in required places instead of show_regs()
Now we don't print information about exceptions if signal is simply
send by another userspace app. So in case of waitpid02 we will print
next message:
_
./waitpid02
potentially unexpected fatal signal 8.
[STAT32]: 0x80080082 : IE U
BTA: 0x2fc4SP: 0x5ff8bd64 FP: 0x
LPS: 0x200524a0 LPE: 0x200524b6 LPC: 0x0006
[-other-info-]
_
This patch fix
STAR 9001146055: waitpid02: Invalid Write @ 0x by insn @ 0x000123ec
Signed-off-by: Eugeniy Paltsev
---
arch/arc/include/asm/bug.h | 1 +
arch/arc/kernel/traps.c| 1 +
arch/arc/kernel/troubleshoot.c | 27 ---
arch/arc/mm/fault.c| 2 ++
4 files changed, 24 insertions(+), 7 deletions(-)
diff --git a/arch/arc/include/asm/bug.h b/arch/arc/include/asm/bug.h
index 21ec82466d62..b9fd60f7d36b 100644
--- a/arch/arc/include/asm/bug.h
+++ b/arch/arc/include/asm/bug.h
@@ -16,6 +16,7 @@
struct task_struct;
void show_regs(struct pt_regs *regs);
+void show_exception_regs(struct pt_regs *regs);
void show_stacktrace(struct task_struct *tsk, struct pt_regs *regs);
void show_kernel_fault_diag(const char *str, struct pt_regs *regs,
unsigned long address);
diff --git a/arch/arc/kernel/traps.c b/arch/arc/kernel/traps.c
index b123558bf0bb..fe41f0d9488f 100644
--- a/arch/arc/kernel/traps.c
+++ b/arch/arc/kernel/traps.c
@@ -50,6 +50,7 @@ unhandled_exception(const char *str, struct pt_regs *regs,
siginfo_t *info)
tsk->thread.fault_address = (__force unsigned int)info->si_addr;
force_sig_info(info->si_signo, info, tsk);
+ show_exception_regs(regs);
} else {
/* If not due to copy_(to|from)_user, we are doomed */
diff --git a/arch/arc/kernel/troubleshoot.c b/arch/arc/kernel/troubleshoot.c
index 783b20354f8b..313aea25e638 100644
--- a/arch/arc/kernel/troubleshoot.c
+++ b/arch/arc/kernel/troubleshoot.c
@@ -170,14 +170,9 @@ static void show_ecr_verbose(struct pt_regs *regs)
}
}
-/
- * API called by rest of kernel
- ***/
-
-void show_regs(struct pt_regs *regs)
+void show_exception_regs(struct pt_regs *regs)
{
struct task_struct *tsk = current;
- struct callee_regs *cregs;
char *buf;
buf = (char *)__get_free_page(GFP_KERNEL);
@@ -190,12 +185,28 @@ void show_regs(struct pt_regs *regs)
show_ecr_verbose(regs);
pr_info("[EFA ]: 0x%08lx\n[BLINK ]: %pS\n[ERET ]: %pS\n",
- current->thread.fault_address,
+ tsk->thread.fault_address,
(void *)regs->blink, (void *)regs->ret);
if (user_mode(regs))
show_faulting_vma(regs->ret, buf); /* faulting code, not data */
+ free_page((unsigned long)buf);
+}
+
+/
+ * API called by rest of kernel
+ ***/
+
+void
Re: [PATCH] arc/Kconfig: include mm/Kconfig inside "ARC system configuration"
On 06/29/2018 06:42 AM, Mike Rapoport wrote: > Otherwise mm configuration options show up in the top level menu. > > Signed-off-by: Mike Rapoport > LGTM ! Added to for-curr Thx, -Vineet ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] arc/Kconfig: include mm/Kconfig inside "ARC system configuration"
On 06/29/2018 01:57 PM, Mike Rapoport wrote: > I think that would be something like this: > diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig > index e81bcd271be7..44a20141639a 100644 > --- a/arch/arc/Kconfig > +++ b/arch/arc/Kconfig > @@ -556,12 +556,14 @@ endmenu > > endmenu # "ARC Architecture Configuration" > > +menu "Memory Management" > source "mm/Kconfig" > > config FORCE_MAX_ZONEORDER > int "Maximum zone order" > default "12" if ARC_HUGEPAGE_16M > default "11" > +endmenu # "Memory Management" > > source "net/Kconfig" > source "drivers/Kconfig" > Awesome, this is exactly we want - care to send a formal patch ? -Vineet ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
Re: [PATCH] arc/Kconfig: include mm/Kconfig inside "ARC system configuration"
On 06/29/2018 02:12 PM, Vineet Gupta wrote: > On 06/29/2018 01:57 PM, Mike Rapoport wrote: >> I think that would be something like this: >> diff --git a/arch/arc/Kconfig b/arch/arc/Kconfig >> index e81bcd271be7..44a20141639a 100644 >> --- a/arch/arc/Kconfig >> +++ b/arch/arc/Kconfig >> @@ -556,12 +556,14 @@ endmenu >> >> endmenu # "ARC Architecture Configuration" >> >> +menu "Memory Management" >> source "mm/Kconfig" >> >> config FORCE_MAX_ZONEORDER >> int "Maximum zone order" >> default "12" if ARC_HUGEPAGE_16M >> default "11" >> +endmenu # "Memory Management" >> >> source "net/Kconfig" >> source "drivers/Kconfig" >> > > Awesome, this is exactly we want - care to send a formal patch ? > > -Vineet Mike, I interpret this as a reply to you. And you can add: Tested-by: Randy Dunlap Acked-by: Randy Dunlap and possibly: Suggested-by: Randy Dunlap thanks, -- ~Randy ___ linux-snps-arc mailing list linux-snps-arc@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-snps-arc
