Re: [PATCH] ARC: adjust memblock_reserve of kernel memory

[Vineet Gupta]

On 12/19/18 8:16 AM, Eugeniy Paltsev wrote:
> In setup_arch_memory we reserve the memory area wherein the kernel
> is located. Current implementation may reserve more memory than
> it actually required in case of CONFIG_LINUX_LINK_BASE is not
> equal to CONFIG_LINUX_RAM_BASE. This happens because we calculate
> start of the reserved region relatively to the CONFIG_LINUX_RAM_BASE
> and end of the region relatively to the CONFIG_LINUX_RAM_BASE.
>
> For example in case of HSDK board we wasted 256MiB of physical memory:
> --->8--
> Memory: 770416K/1048576K available (5496K kernel code,
> 240K rwdata, 1064K rodata, 2200K init, 275K bss,
> 278160K reserved, 0K cma-reserved)
> --->8--
>
> Fix that.
>
> Cc: sta...@vger.kernel.org

For backports, please also check how far back this needs to be applied (else we
get bot emails specifying they don't apply to ver x, y z)

> Signed-off-by: Eugeniy Paltsev 
> ---
>  arch/arc/mm/init.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arc/mm/init.c b/arch/arc/mm/init.c
> index f8fe5668b30f..a56e6a8ed259 100644
> --- a/arch/arc/mm/init.c
> +++ b/arch/arc/mm/init.c
> @@ -137,7 +137,8 @@ void __init setup_arch_memory(void)
>*/
>  
>   memblock_add_node(low_mem_start, low_mem_sz, 0);
> - memblock_reserve(low_mem_start, __pa(_end) - low_mem_start);
> + memblock_reserve(CONFIG_LINUX_LINK_BASE,
> +  __pa(_end) - CONFIG_LINUX_LINK_BASE);
>  
>  #ifdef CONFIG_BLK_DEV_INITRD
>   if (initrd_start)

So I looked into the history for restricting stable, and it seems this was
introduced with our earlier work for HSDK.

Before commit 9ed68785f7f this code was ok, since low_mem_start =
CONFIG_LINUX_LINK_BASE.
With the patch we changed low_mem_start to CONFIG_LINUX_RAM_BASE and missed 
this.

-Vineet

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH] ARC: adjust memblock_reserve of kernel memory

[Vineet Gupta]

On 12/19/18 8:16 AM, Eugeniy Paltsev wrote:
> In setup_arch_memory we reserve the memory area wherein the kernel
> is located. Current implementation may reserve more memory than
> it actually required in case of CONFIG_LINUX_LINK_BASE is not
> equal to CONFIG_LINUX_RAM_BASE. This happens because we calculate
> start of the reserved region relatively to the CONFIG_LINUX_RAM_BASE
> and end of the region relatively to the CONFIG_LINUX_RAM_BASE.
>
> For example in case of HSDK board we wasted 256MiB of physical memory:
> --->8--
> Memory: 770416K/1048576K available (5496K kernel code,
> 240K rwdata, 1064K rodata, 2200K init, 275K bss,
> 278160K reserved, 0K cma-reserved)
> --->8--
>
> Fix that.
>
> Cc: sta...@vger.kernel.org
> Signed-off-by: Eugeniy Paltsev 

LGTM. I presume you have booted HSDK with it and done some smoke testing.

-Vineet

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 1/2] ARC: show_regs: avoid page allocator

[Vineet Gupta]

On 12/19/18 9:04 AM, Eugeniy Paltsev wrote:
> As I can see x86 use print_vma_addr() in their show_signal_msg()
> function which allocate page with __get_free_page(GFP_NOWAIT);

Indeed with that the __get_free_page() lockdep splat is gone.

There's a different one now hence my other patch.

| [ARCLinux]# ./segv-null-ptr
| potentially unexpected fatal signal 11.
| BUG: sleeping function called from invalid context at kernel/fork.c:1011
| in_atomic(): 1, irqs_disabled(): 0, pid: 70, name: segv-null-ptr
| no locks held by segv-null-ptr/70.
| CPU: 0 PID: 70 Comm: segv-null-ptr Not tainted 4.18.0+ #69
|
| Stack Trace:
|  arc_unwind_core+0xcc/0x100
|  ___might_sleep+0x17a/0x190
|  mmput+0x16/0xb8
|  show_regs+0x52/0x310
|  get_signal+0x5ee/0x610
|  do_signal+0x2c/0x218
|  resume_user_mode_begin+0x90/0xd8


___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 00/21] glibc port to ARC processors

[Vineet Gupta]

On 12/19/18 4:14 PM, Joseph Myers wrote:
> On Wed, 19 Dec 2018, Vineet Gupta wrote:
> 
>> On 12/18/18 3:11 PM, Joseph Myers wrote:
>>> Another general point: when posting a new port, could you include pointers 
>>> to architecture and ABI reference manuals in case those are of relevance 
>>> to the review?  (URLs going directly to PDFs of those manuals are 
>>> preferred.)
>>
>> The PRM (Programmer's Reference Manual) is not open source and per corporate
>> policy requires license agreement, signing NDA...
> 
> It's very questionable whether we should consider a port for inclusion in 
> glibc at all without public architecture documentation 

IMHO this seems excessive. We've successfully open-sourced significant ports 
such
as Linux kernel, gcc, binutils etc. gcc atleast deals with instruction semantics
at a much closer level than glibc perse and that didn't get in the way then.

Being an open source developer myself I agree with your reservations, but the
reality is what Ted Tso so nicely put in at [1]

| "It's something I do worry about; and I do share your concern.  At the
| same time, the reality is that we are a little like the Old Dutch
| Masters, who had take into account the preference of their patrons
| (i.e., in our case, those who pay our paychecks :-)"

[1] https://lwn.net/Articles/446626/

> (that is, 
> documentation of instruction semantics; not necessarily documentation of 
> microarchitectural performance characteristics etc.). 

I understand the intent is all good faith and justified...

> Various kinds of 
> changes can require a developer to refer to documentation across the range 
> of architectures supported by glibc, if something requires assembly 
> implementations across architectures (e.g. some of Adhemerval's changes to 
> thread cancellation; or when I added fegetmode / fesetmode, that required 
> referring to various architecture documentation to identify what bits 
> should be considered mode bits, on each architecture where floating-point 
> status and control bits occupy a single register).

To be honest folks on lkml do sweeping arch changes, PeterZ is one good example
and he has infact changed ARC assembly at times w/o access to PRM. Given the
arrangement we have now, perhaps such changes can call in for review from port
maintainer etc.

> (Non-NDA click-throughs, like the Arm one agreeing not to use the manual 
> to find if Arm implementations violate any patents, are OK

No that is not possible: I've discussed this with power may-be in the past and
again today and this is not going to happen.

Thx,
-Vineet

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 00/21] glibc port to ARC processors

[Joseph Myers]

On Wed, 19 Dec 2018, Vineet Gupta wrote:

> On 12/18/18 3:11 PM, Joseph Myers wrote:
> > Another general point: when posting a new port, could you include pointers 
> > to architecture and ABI reference manuals in case those are of relevance 
> > to the review?  (URLs going directly to PDFs of those manuals are 
> > preferred.)
> 
> The PRM (Programmer's Reference Manual) is not open source and per corporate
> policy requires license agreement, signing NDA...

It's very questionable whether we should consider a port for inclusion in 
glibc at all without public architecture documentation (that is, 
documentation of instruction semantics; not necessarily documentation of 
microarchitectural performance characteristics etc.).  Various kinds of 
changes can require a developer to refer to documentation across the range 
of architectures supported by glibc, if something requires assembly 
implementations across architectures (e.g. some of Adhemerval's changes to 
thread cancellation; or when I added fegetmode / fesetmode, that required 
referring to various architecture documentation to identify what bits 
should be considered mode bits, on each architecture where floating-point 
status and control bits occupy a single register).

(Non-NDA click-throughs, like the Arm one agreeing not to use the manual 
to find if Arm implementations violate any patents, are OK, presuming 
there is nothing to restrict public discussion of the contents and using 
the information therein to develop free software, although having a direct 
URL to the manual without needing such a click-through is certainly 
preferred.)

-- 
Joseph S. Myers
jos...@codesourcery.com

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 00/21] glibc port to ARC processors

[Vineet Gupta]

On 12/18/18 3:11 PM, Joseph Myers wrote:
> Another general point: when posting a new port, could you include pointers 
> to architecture and ABI reference manuals in case those are of relevance 
> to the review?  (URLs going directly to PDFs of those manuals are 
> preferred.)

The PRM (Programmer's Reference Manual) is not open source and per corporate
policy requires license agreement, signing NDA...

The ABI doc as of today is not open as well, although I'm checking if we could
share it and if not create equivalent content like some of the other guys on
github etc.

Thx for your understanding.

-Vineet

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)

[Vineet Gupta]

On 12/19/18 2:00 PM, Adhemerval Zanella wrote:
> 
> 
> One possibility is to define an arch-specific __sigset_t.h with a custom 
> _SIGSET_NWORDS value and add an optimization on Linux sigaction.c to check
> if both kernel_sigaction and glibc sigaction share same size and internal
> layout to use the struct as-is for syscall instead of copy to a temporary
> value (similar to what we used to have on getdents).  ARC would still have
> arch-specific code and would be the only ABI to have a different sigset_t
> though.

I don't like ARC to single out either. But as Joseph suggests could this be
starting point for arches of future. Assuming it does, would rather see this or
the approach Joseph alluded to earlier [1]

[1] 
http://lists.infradead.org/pipermail/linux-snps-arc/2018-December/005122.html

> 
> However I *hardly* think sigaction is a hotspot in real world cases, usually
> the signal action is defined once or in a very limited number of times.  I am
> not considering synthetic benchmarks, specially lmbench which in most cases
> does not measure any useful metric. 

I tend to disagree. Coming from embedded linux background, I've found it 
immensely
useful to compare 2 minimal systems: especially when distos, out-of-box 
packages,
fancy benchmarks don't even exist.

At any rate, my disagreement with status quo is not so much of optimize for ARC,
but rather pointless spending of electrons. When we know that there are 64 
signals
at max, which need 64-bits, why bother shuffling around 1k bits, specially when
one is starting afresh and there's no legacy crap getting in the way.

The case of adding more signals in future is indeed theoretically possible but
that will be an ABI change anyways.

> Even for other sigset_t usage case I still
> think an arch-specific definition should not make much difference:
> 
>   1. setcontext: it would incur just in larger ucontext_t (kernel get/set
>  mask is done using kernel expected size).  Also, taking in consideration
>  these interfaces were removed from POSIX.1-2008, the inherent performance
>  issues (signal set/restore will most likely dominate the overhead), and
>  some implementation issues (BZ#18135 for instance), I would say to not
>  bother to optimize it.
> 
>   2. pselect, ppoll, epoll_pwait, posix_spawn (posix_spawnattr_t), sig*: 
>  for functions that accept sigset as an argument it would incur in just
>  larger memory utilization without much performance overhead. Again,
>  runtime for these calls would be mostly dominate by syscall overhead
>  or kernel wait-time for events.
> 
>   3. raise, etc: for function that might allocate a sigset_t internally it
>  will similar to 2.

I agree that that in libc, pretty much anything will be dominated by syscall
overhead, but still...


> Now, if ARC intention is just to follow generic glibc linux ABI definitions,
> it could just define its sigaction as (not tested):

Indeed the ABI is etched in stone and I have a very similar code now, with 
slight
difference.

> * sysdeps/unix/sysv/linux/arc/sigaction.c
> 
> ---
> #define SA_RESTORER 0x0400
> #include 
> 
> extern void restore_rt (void) asm ("__restore_rt") attribute_hidden;
> 
> #define SET_SA_RESTORER(kact, act)  \
>   (kact)->sa_flags = (act)->sa_flags | SA_RESTORER; \
>   (kact)->sa_restorer = &__default_rt_sa_restorer

+#define SET_SA_RESTORER(kact, act) \
+ ({\
+   if (!((kact)->sa_flags & SA_RESTORER))  \
+ { \
+   (kact)->sa_restorer = __default_rt_sa_restorer; \
+   (kact)->sa_flags |= SA_RESTORER;\
+ } \
+   else\
+ (kact)->sa_restorer = (act)->sa_restorer; \
+ })

> 
> #define RESET_SA_RESTORER(act, kact)\
>   (act)->sa_restorer = (kact)->sa_restorer
> 
> static void __default_rt_sa_restorer(void)
> {
>   INTERNAL_SYSCALL_DECL (err);
>   INTERNAL_SYSCALL_CALL (__NR_rt_sigreturn, err);
> }
> 
> #include 



___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 13/21] ARC: Build Infrastructure

[Joseph Myers]

On Wed, 19 Dec 2018, Vineet Gupta wrote:

> Indeed with current glibc port, we do build/test/run a bunch of packages 
> relying on PIE (dumpcap, openssh come to mind) successfully. So yes PIE 
> should work in general for ARC now. FWIW this could also explain the 
> following current results.

I should note a separate issue to consider: that of *static* PIE support.  
That doesn't need to be working, but if it's not working, or if you don't 
know whether it's working, when the port goes in you need to update the 
list at  accordingly.  (Port 
maintainers need to make sure 
 and 
 are up to date for their 
ports, as well as posting test results on the per-release wiki page during 
each release cycle's freeze period.)

-- 
Joseph S. Myers
jos...@codesourcery.com

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)

[Adhemerval Zanella]

On 19/12/2018 15:58, Vineet Gupta wrote:
> On 12/18/18 6:39 PM, Vineet Gupta wrote:
 diff --git a/sysdeps/unix/sysv/linux/arc/sigaction.c 
 b/sysdeps/unix/sysv/linux/arc/sigaction.c
>>> Why do you need this, rather than using the unified version (possibly with 
>>> a few macros defined first)?
>>
>> The only syscall ABI requirement is that we pass our our own SA_RESTORER stub
>> (rather than inject one in kernel, and deal with cache sync etc etc). Indeed 
>> the
>> common code can be used - likely was not the case when I started with ARC 
>> port, or
>> more likely the port that I started ARC port from ;-)
>>
>> I'll update this.
> 
> I took a stab at this but not really happy with taking this approach.
> 
> (1). Common code assumes disparate kernel and userland sigaction struct even
> though there's no reason for a *new* port to be: its not like all glibc code 
> is
> shared/common although I agree it is best to do so as much as possible
> So this requires explicit copy over of 1 struct into other, when it could have
> been avoided altogether for some cases atleast (!SA_RESTORER).
> 
> (2)  Linux kernel asm-generic syscall ABI expects sigset_t to be 2 words
> 
> | kernel: include/uapi/asm-generic/signal.h
> |
> | #define _NSIG   64
> | typedef struct {
> | unsigned long sig[_NSIG_WORDS];
> | } sigset_t;
> 
> And that is what we pretend at the time of syscall itself, e.g. snippet below 
> from
> generic sigaction()
> 
> | /* XXX The size argument hopefully will have to be changed to the
> | real size of the user-level sigset_t.  */
> |   result = INLINE_SYSCALL_CALL (rt_sigaction, sig,
> | act ?  : NULL,
> | oact ?  : NULL, STUB(act) _NSIG / 8);
> ^
> 
> However glibc assumes sigset_to to be 128 words which is fine, however the 
> memcpy
> for 256 words seems pointless when kernel is not supposed to be even looking 
> at
> beyond 2nd word (although I realize my SA_RESTORE case was doing the implicit 
> copy
> as well)
> 
> (3) Consider me a micro-optimization freak :-) but this memcpy seems waste of
> cycles and will atleast show up LMBench lat_sig  micro-benchmarks.
> 
> 
> I don't have strong feelings either ways, but wanted to express my concerns 
> anyways.
> 

One possibility is to define an arch-specific __sigset_t.h with a custom 
_SIGSET_NWORDS value and add an optimization on Linux sigaction.c to check
if both kernel_sigaction and glibc sigaction share same size and internal
layout to use the struct as-is for syscall instead of copy to a temporary
value (similar to what we used to have on getdents).  ARC would still have
arch-specific code and would be the only ABI to have a different sigset_t
though.

However I *hardly* think sigaction is a hotspot in real world cases, usually
the signal action is defined once or in a very limited number of times.  I am
not considering synthetic benchmarks, specially lmbench which in most cases
does not measure any useful metric. Even for other sigset_t usage case I still
think an arch-specific definition should not make much difference:

  1. setcontext: it would incur just in larger ucontext_t (kernel get/set
 mask is done using kernel expected size).  Also, taking in consideration
 these interfaces were removed from POSIX.1-2008, the inherent performance
 issues (signal set/restore will most likely dominate the overhead), and
 some implementation issues (BZ#18135 for instance), I would say to not
 bother to optimize it.

  2. pselect, ppoll, epoll_pwait, posix_spawn (posix_spawnattr_t), sig*: 
 for functions that accept sigset as an argument it would incur in just
 larger memory utilization without much performance overhead. Again,
 runtime for these calls would be mostly dominate by syscall overhead
 or kernel wait-time for events.

  3. raise, etc: for function that might allocate a sigset_t internally it
 will similar to 2.

Now, if ARC intention is just to follow generic glibc linux ABI definitions,
it could just define its sigaction as (not tested):

* sysdeps/unix/sysv/linux/arc/sigaction.c

---
#define SA_RESTORER 0x0400
#include 

extern void restore_rt (void) asm ("__restore_rt") attribute_hidden;

#define SET_SA_RESTORER(kact, act)  \
  (kact)->sa_flags = (act)->sa_flags | SA_RESTORER; \
  (kact)->sa_restorer = &__default_rt_sa_restorer

#define RESET_SA_RESTORER(act, kact)\
  (act)->sa_restorer = (kact)->sa_restorer

static void __default_rt_sa_restorer(void)
{
  INTERNAL_SYSCALL_DECL (err);
  INTERNAL_SYSCALL_CALL (__NR_rt_sigreturn, err);
}

#include 
---


___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 13/21] ARC: Build Infrastructure

[Vineet Gupta]

On 12/18/18 3:44 PM, Joseph Myers wrote:
> On Tue, 18 Dec 2018, Vineet Gupta wrote:
> 
>> +libc {
>> +  GLIBC_2.29 {
>> +__adddf3; __addsf3; __divdf3; __divsf3; __eqdf2; __eqsf2; __extendsfdf2;
>> +__fixdfdi; __fixdfsi; __fixsfdi; __fixsfsi;
>> +__fixunsdfdi; __fixunsdfsi; __fixunssfdi; __fixunssfsi;
>> +__floatdidf; __floatdisf; __floatsidf; __floatsisf;
>> +__floatundidf; __floatundisf; __floatunsidf; __floatunsisf;
>> +__gedf2; __gesf2; __gtdf2; __gtsf2; __ledf2; __lesf2; __ltdf2; __ltsf2;
>> +__muldf3; __mulsf3; __nedf2; __nesf2; __negdf2; __negsf2;
>> +__subdf3; __subsf3; __truncdfsf2; __unorddf2; __unordsf2;
>
> Exporting soft-fp symbols from glibc is only appropriate if you are also 
> building soft-fp in glibc (as opposed to building it in libgcc) - which 
> might be done with a view to possibly enabling support for exceptions and 
> rounding modes for soft-float, as on powerpc soft-float, for example.  I 
> don't see a Subdirs files containing "soft-fp" in this patch series.  If 
> you're just linking with libgcc, you should not be re-exporting these 
> functions from libc.so (and indeed I don't see them in your ABI baselines 
> - libgcc.a has them as hidden symbols to prevent such re-exporting).

Removed now !

>> +libc_cv_fpie=no
> Why? 

This likely goes back to my original work on glibc (started somewhere in 2015),
and at the time binutils support for PIE was really shaky (to speak mildly). It
was only in ARC GNU 2016.09 release did we have serious pie fixes due to 
essential
rewrite of binutils backend by Cuper and Claudiu.

> I'd expect -fpie to be working for all glibc architectures with any 
> modern GCC version (I wouldn't expect it to need extra GCC back-end 
> support beyond -fpic).

Well at the time there were multiple PIE related issues in binutils rather than 
gcc.

Indeed with current glibc port, we do build/test/run a bunch of packages relying
on PIE (dumpcap, openssh come to mind) successfully. So yes PIE should work in
general for ARC now. FWIW this could also explain the following current results.

PASS: gmon/tst-gmon-gprof
FAIL: gmon/tst-gmon-pie-gprof

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 1/2] ARC: show_regs: avoid page allocator

[Vineet Gupta]

On 12/19/18 12:46 PM, William Kucharski wrote:
> I would rather see 256 as a #define somewhere rather than a magic number 
> sprinkled
> around arch/arc/kernel/troubleshoot.c.

That bothered me as well, but I was too lazy to define one and the existing ones
don't apply. PATH_MAX is 4K which will blow up the stack usage.
> 
> Still, that's what the existing code does, so I suppose it's OK.

I'll define one locally.

> Otherwise the change looks good.

Thx for taking a look.

> Reviewed-by: William Kucharski 

I'll add this to the patch.

Thx,
-Vineet

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 1/2] ARC: show_regs: avoid page allocator

[William Kucharski]

> On Dec 18, 2018, at 11:53 AM, Vineet Gupta  wrote:
> 
> Use on-stack smaller buffers instead of dynamic pages.
> 
> The motivation for this change was to address lockdep splat when
> signal handling code calls show_regs (with preemption disabled) and
> ARC show_regs calls into sleepable page allocator.
> 
> | potentially unexpected fatal signal 11.
> | BUG: sleeping function called from invalid context at 
> ../mm/page_alloc.c:4317
> | in_atomic(): 1, irqs_disabled(): 0, pid: 57, name: segv
> | no locks held by segv/57.
> | Preemption disabled at:
> | [<8182f17e>] get_signal+0x4a6/0x7c4
> | CPU: 0 PID: 57 Comm: segv Not tainted 4.17.0+ #23
> |
> | Stack Trace:
> |  arc_unwind_core.constprop.1+0xd0/0xf4
> |  __might_sleep+0x1f6/0x234
> |  __get_free_pages+0x174/0xca0
> |  show_regs+0x22/0x330
> |  get_signal+0x4ac/0x7c4 # print_fatal_signals() -> preempt_disable()
> |  do_signal+0x30/0x224
> |  resume_user_mode_begin+0x90/0xd8
> 
> Despite this, lockdep still barfs (see next change), but this patch
> still has merit as in we use smaller/localized buffers now and there's
> less instructoh trace to sift thru when debugging pesky issues.
> 
> Signed-off-by: Vineet Gupta 

I would rather see 256 as a #define somewhere rather than a magic number 
sprinkled
around arch/arc/kernel/troubleshoot.c.

Still, that's what the existing code does, so I suppose it's OK.

Otherwise the change looks good.

Reviewed-by: William Kucharski 


___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 03/21] ARC: ABI Implementation

[Joseph Myers]

On Wed, 19 Dec 2018, Vineet Gupta wrote:

> On 12/19/18 9:40 AM, Joseph Myers wrote:
> > Architecture maintainers need wiki accounts in any case (to post test 
> > results for their architectures during every release freeze period), so 
> > you should create an account and post its name so someone can add it to 
> > EditorGroup so you can edit.
> 
> Created now: VineetGupta

Added to EditorGroup.

-- 
Joseph S. Myers
jos...@codesourcery.com

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 09/21] ARC: Linux ABI

[Joseph Myers]

On Wed, 19 Dec 2018, Vineet Gupta wrote:

> So perhaps we keep the ARC header above, with matching layout, while 
> switching to generic sigaction() still. OK ?

If the different userspace layout is better, it should be better for all 
future ports as well.

Duplicating all the SA_* constants in architecture-specific headers, even 
when those architectures don't actually have different values of those 
constants, is bad, especially since there is logic about which __USE_* 
macros get which SA_* defined which then needs to be duplicated.

So what this suggests to me is a preliminary patch series to unify 
bits/sigaction.h headers - there might be one header for SA_* constants 
(architecture-specific variants only when needed), one for SIG_* constants 
(architecture-specific variants only when needed), and one for the ways in 
which the layout of the userspace struct sigaction can vary (defining 
macros that the unified bits/sigaction.h then uses to control the layout - 
cf. the siginfo unification a while back).

Initially, the default version of that last header would correspond to the 
existing sysdeps/unix/sysv/linux/bits/sigaction.h header - however, if you 
get consensus for a different layout as the default for new architectures, 
the default would change and all existing architectures that use the 
current default layout would get copies of the new bits/ header configured 
to keep the old layout.  (Variants are possible to reduce the number of 
architectures needing architecture-specific copies as a result of the 
changed default.  For example, you could have the new default only in 
sysdeps/unix/sysv/linux/generic not sysdeps/unix/sysv/linux/ and then only 
existing asm-generic architectures (aarch64 nios2 riscv) would need to 
override back to the old default layout.)

-- 
Joseph S. Myers
jos...@codesourcery.com

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 14/21] ARC: Enable __start as entry point vs. canonical _start

[Vineet Gupta]

On 12/18/18 1:28 PM, Florian Weimer wrote:
> I think this change and the test suite fixes should be folded into the
> initial addition of these files.  Or is there a reason for not doing
> this?

As suggested by other reviewers too, I'm folding all fixes into initial files.

Thx,
-Vineet



___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 10/21] ARC: Linux Startup and Dynamic Loading

[Vineet Gupta]

On 12/18/18 3:49 PM, Joseph Myers wrote:
> On Tue, 18 Dec 2018, Vineet Gupta wrote:
> 
>> diff --git a/sysdeps/unix/sysv/linux/arc/ldconfig.h 
>> b/sysdeps/unix/sysv/linux/arc/ldconfig.h
>
> This header should only be needed if you support multiple ABIs 
> simultaneously on the same system; note most architectures don't have it.  
> Maybe it will be relevant when you add support for the hard-float ABI, but 
> not yet.

Ok removed.

>> +#define SYSDEP_KNOWN_INTERPRETER_NAMES \
>> +  { "/lib/ld-linux.so.2", FLAG_ELF_LIBC6 },
>
> That's not the dynamic linker name you're using elsewhere.

Indeed this is leftover from RFC last year. Per your suggestion we changed the
name from ld-linux.so.2 to ld-linux-*arc*.so.2 (reflected in shlib-versions) but
missed here. Fixed now.

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 03/21] ARC: ABI Implementation

[Vineet Gupta]

On 12/19/18 9:40 AM, Joseph Myers wrote:
> Architecture maintainers need wiki accounts in any case (to post test 
> results for their architectures during every release freeze period), so 
> you should create an account and post its name so someone can add it to 
> EditorGroup so you can edit.

Created now: VineetGupta

Thx,
-Vineet

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 09/21] ARC: Linux ABI

[Vineet Gupta]

On 12/18/18 3:38 PM, Joseph Myers wrote:
> On Tue, 18 Dec 2018, Vineet Gupta wrote:
> 
>> +typedef unsigned short int __pr_uid_t;
>> +typedef unsigned short int __pr_gid_t;
> 
> Are you sure?  

No I'm not :-) There were some interim sweeping changes in this area since when 
I
started so this might indeed be redundant as you say.

> I don't see an ARC-specific definition of __kernel_uid_t or 
> __kernel_gid_t in the Linux kernel at all (which would mean unsigned int 
> is actually used and you don't need this header at all).

I'll check.

>> diff --git a/sysdeps/unix/sysv/linux/arc/bits/sigaction.h 
>> b/sysdeps/unix/sysv/linux/arc/bits/sigaction.h
> 
> I wouldn't expect new architectures to have their own bits/sigaction.h.  
> Rather, I'd expect them to use the generic bits/sigaction.h and the 
> generic code to convert from the userspace struct sigaction to the kernel 
> version.

So this is further to the other thread about generic sigaction (sorry for
cross-post). Indeed with switch to generic sigaction this may not be required.
*However* the layouts of generic userspace sigaction are different from 
kernel's.
Just to rehash (notes for myself really)

1. user exported struct

struct sigaction
  {
__sighandler_t sa_handler;
__sigset_t sa_mask;
int sa_flags;
void (*sa_restorer) (void);
  };

2. Linux kernel UAPI struct sigaction / glibc kernel_sigaction

struct sigaction {
__sighandler_t sa_handler;
unsigned long sa_flags;
__sigrestore_t sa_restorer;
sigset_t sa_mask;  /* mask last for extensibility */
};

Since they don't match and we can't possibly change kernel_sigaction, a future
optimization implies changing #1 which has ABI implications at which point it 
has
diminished or NO returns.

So perhaps we keep the ARC header above, with matching layout, while switching 
to
generic sigaction() still. OK ?

>> +#ifdef __USE_MISC
>> +# define __ctx(fld) fld
>> +#else
>> +# define __ctx(fld) __ ## fld
>> +#endif
> 
> New ports should just use namespace-clean field names here 
> unconditionally.  The __ctx macros with __USE_MISC conditionals are purely 
> for maximum API-compatibility for existing ports that needed to be fixed 
> to make them namespace-clean.

OK, fixed this.

Thx,
-Vineet

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)

[Joseph Myers]

On Wed, 19 Dec 2018, Vineet Gupta wrote:

> I took a stab at this but not really happy with taking this approach.
> 
> (1). Common code assumes disparate kernel and userland sigaction struct even
> though there's no reason for a *new* port to be: its not like all glibc code 
> is
> shared/common although I agree it is best to do so as much as possible
> So this requires explicit copy over of 1 struct into other, when it could have
> been avoided altogether for some cases atleast (!SA_RESTORER).

So make the generic code optimize those cases based on appropriate 
conditionals (making sure to verify those conditionals are right for every 
architecture in glibc).

Any new architecture having much architecture-specific code for the kernel 
interface in glibc, beyond the basic definitions of how to call a syscall, 
is suspect, given that the kernel structures should be consistent across 
asm-generic architectures; we ought to make the defaults work so they are 
genuinely suitable as defaults for new architectures.  This may require 
changes to the sysdeps/unix/sysv/linux/ code if it's currently "generic" 
to old architectures but not so good for asm-generic ones.

-- 
Joseph S. Myers
jos...@codesourcery.com

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 21/21] NEWS: mention ARC port

[Vineet Gupta]

On 12/18/18 3:45 PM, Joseph Myers wrote:
> Obviously this belongs under 2.29, or whatever version gets the port, not 
> 2.26.

oops, left over from original code, now fixed and beefed up the details a bit.

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH] ARC: adjust memblock_reserve of kernel memory

[Sasha Levin]

Hi,

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: all

The bot has tested the following trees: v4.19.10, v4.14.89, v4.9.146, v4.4.168, 
v3.18.130, 

v4.19.10: Build OK!
v4.14.89: Build OK!
v4.9.146: Build OK!
v4.4.168: Failed to apply! Possible dependencies:
26f9d5fd82ca ("ARC: support HIGHMEM even without PAE40")
37eda9df5bd8 ("ARC: mm: Introduce explicit super page size support")
c2ff5cf2735c ("ARC: mm: Use virt_to_pfn() for addr >> PAGE_SHIFT pattern")

v3.18.130: Failed to apply! Possible dependencies:
09f3b37e4e3b ("ARC: entry.S: Introduce INTERRUPT_{PROLOGUE,EPILOGUE}")
1f6ccfff6314 ("ARCv2: Support for ARCv2 ISA and HS38x cores")
26f9d5fd82ca ("ARC: support HIGHMEM even without PAE40")
37eda9df5bd8 ("ARC: mm: Introduce explicit super page size support")
4bf4564b27db ("ARC: entry.S: comments cleanup")
565a9b497c51 ("ARC: RIP broken 64bit RTSC")
5793e273a134 ("ARC: intc: split into ARCompact ISA specific, common bits")
6d1a20b1d237 ("ARC: entry.S: split into ARCompact ISA specific, common 
bits")
820970a5aa3c ("ARCv2: [intc] HS38 core interrupt controller")
9b8c7d1e7107 ("ARC: entry.S: FAKE_RET_FROM_EXCPN can always use r9")
a44ec8bd2a55 ("ARC: Fix RTT boot printing")
a615b47dbf0d ("ARC: entry.S: confine EXCEPTION_* macros to one file")
a8717d280879 ("ARC: entry.S: Trap handler to use r10 for syscall vs. brkpt 
decision")
c10d6969b095 ("ARC: entry.S: Ensure that restore_regs is local to 
compilation unit")
dc9e234f91c7 ("ARC: cosmetic: Remove unused ECR bitfield masks")
f033737e77d9 ("ARC: entry.S: canonical'ize EXCEPTION_{PROLOGUE,EPILOGUE}")
fbfa26ae3b20 ("ARC: entry.S: common'ize scrtach reg freeup in intr + 
exceptions")


How should we proceed with this patch?

--
Thanks,
Sasha

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

ARC vs. generic sigaction (was Re: [PATCH 08/21] ARC: Linux Syscall Interface)

[Vineet Gupta]

On 12/18/18 6:39 PM, Vineet Gupta wrote:
>>> diff --git a/sysdeps/unix/sysv/linux/arc/sigaction.c 
>>> b/sysdeps/unix/sysv/linux/arc/sigaction.c
>> Why do you need this, rather than using the unified version (possibly with 
>> a few macros defined first)?
>
> The only syscall ABI requirement is that we pass our our own SA_RESTORER stub
> (rather than inject one in kernel, and deal with cache sync etc etc). Indeed 
> the
> common code can be used - likely was not the case when I started with ARC 
> port, or
> more likely the port that I started ARC port from ;-)
> 
> I'll update this.

I took a stab at this but not really happy with taking this approach.

(1). Common code assumes disparate kernel and userland sigaction struct even
though there's no reason for a *new* port to be: its not like all glibc code is
shared/common although I agree it is best to do so as much as possible
So this requires explicit copy over of 1 struct into other, when it could have
been avoided altogether for some cases atleast (!SA_RESTORER).

(2)  Linux kernel asm-generic syscall ABI expects sigset_t to be 2 words

| kernel: include/uapi/asm-generic/signal.h
|
| #define _NSIG 64
| typedef struct {
|   unsigned long sig[_NSIG_WORDS];
| } sigset_t;

And that is what we pretend at the time of syscall itself, e.g. snippet below 
from
generic sigaction()

| /* XXX The size argument hopefully will have to be changed to the
| real size of the user-level sigset_t.  */
|   result = INLINE_SYSCALL_CALL (rt_sigaction, sig,
|   act ?  : NULL,
|   oact ?  : NULL, STUB(act) _NSIG / 8);
^

However glibc assumes sigset_to to be 128 words which is fine, however the 
memcpy
for 256 words seems pointless when kernel is not supposed to be even looking at
beyond 2nd word (although I realize my SA_RESTORE case was doing the implicit 
copy
as well)

(3) Consider me a micro-optimization freak :-) but this memcpy seems waste of
cycles and will atleast show up LMBench lat_sig  micro-benchmarks.


I don't have strong feelings either ways, but wanted to express my concerns 
anyways.

Thx,
-Vineet

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 03/21] ARC: ABI Implementation

[Joseph Myers]

On Tue, 18 Dec 2018, Vineet Gupta wrote:

> P.S. Is there a lint file or are these style guidelines in wiki 
> somewhere. If not I can volunteer to capture them in a wiki/readme for 
> future port submissions.

In addition to the GNU Coding Standards (which cover many things such as 
comment formatting and spaces before '('), we have 
 which discusses 
various glibc-specific details.  If you find there are points of the style 
in glibc which aren't documented in either place, then, yes, it would be a 
good idea to add them to the wiki (I just added a few).

Architecture maintainers need wiki accounts in any case (to post test 
results for their architectures during every release freeze period), so 
you should create an account and post its name so someone can add it to 
EditorGroup so you can edit.

Where the formatting issues pointed out exist in some existing code in 
glibc, it might also make sense to fix that existing code to follow the 
standards more closely so it provides a better example for new code.  
However, such formatting changes to existing code may be controversial 
(especially for code that originally came from third-party sources using 
completely different formatting - though in my view, where such files are 
now maintained in glibc without ongoing updates from upstream, we ought to 
make the formatting consistent to simplify maintenance).

-- 
Joseph S. Myers
jos...@codesourcery.com

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 1/2] ARC: show_regs: avoid page allocator

[Vineet Gupta]

On 12/19/18 9:04 AM, Eugeniy Paltsev wrote:
> Just curious: isn't that enough to use GFP_NOWAIT instead
> of GFP_KERNEL when we allocate page in show_regs()?
>
> As I can see x86 use print_vma_addr() in their show_signal_msg()
> function which allocate page with __get_free_page(GFP_NOWAIT);

I'm not sure if lockdep will be happy with it still.

At any rate, as explained in changelog, this still has merit, since the buffer 
is
only needed for nested d_path calls, which are better served with a smaller
on-stack buffer. For cases such as kernel crash, we want lesser code/traces in
fault path to sift thru !

-Vineet

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 1/2] ARC: show_regs: avoid page allocator

[Eugeniy Paltsev]

Hi Vineet,

Just curious: isn't that enough to use GFP_NOWAIT instead
of GFP_KERNEL when we allocate page in show_regs()?

As I can see x86 use print_vma_addr() in their show_signal_msg()
function which allocate page with __get_free_page(GFP_NOWAIT);

On Tue, 2018-12-18 at 10:53 -0800, Vineet Gupta wrote:
> Use on-stack smaller buffers instead of dynamic pages.
> 
> The motivation for this change was to address lockdep splat when
> signal handling code calls show_regs (with preemption disabled) and
> ARC show_regs calls into sleepable page allocator.
> 
> > potentially unexpected fatal signal 11.
> > BUG: sleeping function called from invalid context at 
> > ../mm/page_alloc.c:4317
> > in_atomic(): 1, irqs_disabled(): 0, pid: 57, name: segv
> > no locks held by segv/57.
> > Preemption disabled at:
> > [<8182f17e>] get_signal+0x4a6/0x7c4
> > CPU: 0 PID: 57 Comm: segv Not tainted 4.17.0+ #23
> > 
> > Stack Trace:
> >  arc_unwind_core.constprop.1+0xd0/0xf4
> >  __might_sleep+0x1f6/0x234
> >  __get_free_pages+0x174/0xca0
> >  show_regs+0x22/0x330
> >  get_signal+0x4ac/0x7c4 # print_fatal_signals() -> preempt_disable()
> >  do_signal+0x30/0x224
> >  resume_user_mode_begin+0x90/0xd8
> 
> Despite this, lockdep still barfs (see next change), but this patch
> still has merit as in we use smaller/localized buffers now and there's
> less instructoh trace to sift thru when debugging pesky issues.
> 
> Signed-off-by: Vineet Gupta 
> ---
>  arch/arc/kernel/troubleshoot.c | 22 +-
>  1 file changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/arch/arc/kernel/troubleshoot.c b/arch/arc/kernel/troubleshoot.c
> index e8d9fb452346..2885bec71fb8 100644
> --- a/arch/arc/kernel/troubleshoot.c
> +++ b/arch/arc/kernel/troubleshoot.c
> @@ -58,11 +58,12 @@ static void show_callee_regs(struct callee_regs *cregs)
>   print_reg_file(&(cregs->r13), 13);
>  }
>  
> -static void print_task_path_n_nm(struct task_struct *tsk, char *buf)
> +static void print_task_path_n_nm(struct task_struct *tsk)
>  {
>   char *path_nm = NULL;
>   struct mm_struct *mm;
>   struct file *exe_file;
> + char buf[256];
>  
>   mm = get_task_mm(tsk);
>   if (!mm)
> @@ -80,10 +81,9 @@ static void print_task_path_n_nm(struct task_struct *tsk, 
> char *buf)
>   pr_info("Path: %s\n", !IS_ERR(path_nm) ? path_nm : "?");
>  }
>  
> -static void show_faulting_vma(unsigned long address, char *buf)
> +static void show_faulting_vma(unsigned long address)
>  {
>   struct vm_area_struct *vma;
> - char *nm = buf;
>   struct mm_struct *active_mm = current->active_mm;
>  
>   /* can't use print_vma_addr() yet as it doesn't check for
> @@ -96,8 +96,11 @@ static void show_faulting_vma(unsigned long address, char 
> *buf)
>* if the container VMA is not found
>*/
>   if (vma && (vma->vm_start <= address)) {
> + char buf[256];
> + char *nm = "?";
> +
>   if (vma->vm_file) {
> - nm = file_path(vma->vm_file, buf, PAGE_SIZE - 1);
> + nm = file_path(vma->vm_file, buf, 256-1);
>   if (IS_ERR(nm))
>   nm = "?";
>   }
> @@ -173,13 +176,8 @@ void show_regs(struct pt_regs *regs)
>  {
>   struct task_struct *tsk = current;
>   struct callee_regs *cregs;
> - char *buf;
> -
> - buf = (char *)__get_free_page(GFP_KERNEL);
> - if (!buf)
> - return;
>  
> - print_task_path_n_nm(tsk, buf);
> + print_task_path_n_nm(tsk);
>   show_regs_print_info(KERN_INFO);
>  
>   show_ecr_verbose(regs);
> @@ -189,7 +187,7 @@ void show_regs(struct pt_regs *regs)
>   (void *)regs->blink, (void *)regs->ret);
>  
>   if (user_mode(regs))
> - show_faulting_vma(regs->ret, buf); /* faulting code, not data */
> + show_faulting_vma(regs->ret); /* faulting code, not data */
>  
>   pr_info("[STAT32]: 0x%08lx", regs->status32);
>  
> @@ -221,8 +219,6 @@ void show_regs(struct pt_regs *regs)
>   cregs = (struct callee_regs *)current->thread.callee_reg;
>   if (cregs)
>   show_callee_regs(cregs);
> -
> - free_page((unsigned long)buf);
>  }
>  
>  void show_kernel_fault_diag(const char *str, struct pt_regs *regs,
-- 
 Eugeniy Paltsev
___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: ensure dma_alloc_coherent always returns zeroed memory

[Christoph Hellwig]

FYI, I've picked this up for dma-mapping for-next now.

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH v2 2/2] kgdb/treewide: constify struct kgdb_arch arch_kgdb_ops

[Daniel Thompson]

On Thu, Dec 06, 2018 at 08:07:40PM +, Christophe Leroy wrote:
> checkpatch.pl reports the following:
> 
>   WARNING: struct kgdb_arch should normally be const
>   #28: FILE: arch/mips/kernel/kgdb.c:397:
>   +struct kgdb_arch arch_kgdb_ops = {
> 
> This report makes sense, as all other ops struct, this
> one should also be const. This patch does the change.
> 
> Cc: Vineet Gupta 
> Cc: Russell King 
> Cc: Catalin Marinas 
> Cc: Will Deacon 
> Cc: Yoshinori Sato 
> Cc: Richard Kuo 
> Cc: Michal Simek 
> Cc: Ralf Baechle 
> Cc: Paul Burton 
> Cc: James Hogan 
> Cc: Ley Foon Tan 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: Michael Ellerman 
> Cc: Rich Felker 
> Cc: "David S. Miller" 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Cc: x...@kernel.org
> Acked-by: Daniel Thompson 
> Acked-by: Paul Burton 
> Signed-off-by: Christophe Leroy 

I've not heard any objections from the arch/ maintainers so...

Applied! Thanks.


> -
> ---
>  v2: Added CCs to all maintainers/supporters identified by get_maintainer.pl 
> and Acks from Daniel and Paul.
> 
>  arch/arc/kernel/kgdb.c| 2 +-
>  arch/arm/kernel/kgdb.c| 2 +-
>  arch/arm64/kernel/kgdb.c  | 2 +-
>  arch/h8300/kernel/kgdb.c  | 2 +-
>  arch/hexagon/kernel/kgdb.c| 2 +-
>  arch/microblaze/kernel/kgdb.c | 2 +-
>  arch/mips/kernel/kgdb.c   | 2 +-
>  arch/nios2/kernel/kgdb.c  | 2 +-
>  arch/powerpc/kernel/kgdb.c| 2 +-
>  arch/sh/kernel/kgdb.c | 2 +-
>  arch/sparc/kernel/kgdb_32.c   | 2 +-
>  arch/sparc/kernel/kgdb_64.c   | 2 +-
>  arch/x86/kernel/kgdb.c| 2 +-
>  include/linux/kgdb.h  | 2 +-
>  14 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c
> index 9a3c34af2ae8..bfd04b442e36 100644
> --- a/arch/arc/kernel/kgdb.c
> +++ b/arch/arc/kernel/kgdb.c
> @@ -204,7 +204,7 @@ void kgdb_roundup_cpus(unsigned long flags)
>   local_irq_disable();
>  }
>  
> -struct kgdb_arch arch_kgdb_ops = {
> +const struct kgdb_arch arch_kgdb_ops = {
>   /* breakpoint instruction: TRAP_S 0x3 */
>  #ifdef CONFIG_CPU_BIG_ENDIAN
>   .gdb_bpt_instr  = {0x78, 0x7e},
> diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c
> index caa0dbe3dc61..21a6d5958955 100644
> --- a/arch/arm/kernel/kgdb.c
> +++ b/arch/arm/kernel/kgdb.c
> @@ -274,7 +274,7 @@ int kgdb_arch_remove_breakpoint(struct kgdb_bkpt *bpt)
>   * and we handle the normal undef case within the do_undefinstr
>   * handler.
>   */
> -struct kgdb_arch arch_kgdb_ops = {
> +const struct kgdb_arch arch_kgdb_ops = {
>  #ifndef __ARMEB__
>   .gdb_bpt_instr  = {0xfe, 0xde, 0xff, 0xe7}
>  #else /* ! __ARMEB__ */
> diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
> index a20de58061a8..fe1d1f935b90 100644
> --- a/arch/arm64/kernel/kgdb.c
> +++ b/arch/arm64/kernel/kgdb.c
> @@ -357,7 +357,7 @@ void kgdb_arch_exit(void)
>   unregister_die_notifier(_notifier);
>  }
>  
> -struct kgdb_arch arch_kgdb_ops;
> +const struct kgdb_arch arch_kgdb_ops;
>  
>  int kgdb_arch_set_breakpoint(struct kgdb_bkpt *bpt)
>  {
> diff --git a/arch/h8300/kernel/kgdb.c b/arch/h8300/kernel/kgdb.c
> index 1a1d30cb0609..602e478afbd5 100644
> --- a/arch/h8300/kernel/kgdb.c
> +++ b/arch/h8300/kernel/kgdb.c
> @@ -129,7 +129,7 @@ void kgdb_arch_exit(void)
>   /* Nothing to do */
>  }
>  
> -struct kgdb_arch arch_kgdb_ops = {
> +const struct kgdb_arch arch_kgdb_ops = {
>   /* Breakpoint instruction: trapa #2 */
>   .gdb_bpt_instr = { 0x57, 0x20 },
>  };
> diff --git a/arch/hexagon/kernel/kgdb.c b/arch/hexagon/kernel/kgdb.c
> index 16c24b22d0b2..f1924d483e78 100644
> --- a/arch/hexagon/kernel/kgdb.c
> +++ b/arch/hexagon/kernel/kgdb.c
> @@ -83,7 +83,7 @@ struct dbg_reg_def_t dbg_reg_def[DBG_MAX_REG_NUM] = {
>   { "syscall_nr", GDB_SIZEOF_REG, offsetof(struct pt_regs, syscall_nr)},
>  };
>  
> -struct kgdb_arch arch_kgdb_ops = {
> +const struct kgdb_arch arch_kgdb_ops = {
>   /* trap0(#0xDB) 0x0cdb0054 */
>   .gdb_bpt_instr = {0x54, 0x00, 0xdb, 0x0c},
>  };
> diff --git a/arch/microblaze/kernel/kgdb.c b/arch/microblaze/kernel/kgdb.c
> index 6366f69d118e..130cd0f064ce 100644
> --- a/arch/microblaze/kernel/kgdb.c
> +++ b/arch/microblaze/kernel/kgdb.c
> @@ -143,7 +143,7 @@ void kgdb_arch_exit(void)
>  /*
>   * Global data
>   */
> -struct kgdb_arch arch_kgdb_ops = {
> +const struct kgdb_arch arch_kgdb_ops = {
>  #ifdef __MICROBLAZEEL__
>   .gdb_bpt_instr = {0x18, 0x00, 0x0c, 0xba}, /* brki r16, 0x18 */
>  #else
> diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c
> index 31eff1bec577..edfdc2ec2d16 100644
> --- a/arch/mips/kernel/kgdb.c
> +++ b/arch/mips/kernel/kgdb.c
> @@ -394,7 +394,7 @@ int kgdb_arch_handle_exception(int vector, int signo, int 
> err_code,
>   return -1;
>  }
>  
> -struct kgdb_arch arch_kgdb_ops = {
> +const struct kgdb_arch arch_kgdb_ops = {
>  #ifdef CONFIG_CPU_BIG_ENDIAN
>   .gdb_bpt_instr = { spec_op << 

Re: [PATCH v2 1/2] mips/kgdb: prepare arch_kgdb_ops for constness

[Daniel Thompson]

On Thu, Dec 06, 2018 at 08:07:38PM +, Christophe Leroy wrote:
> MIPS is the only architecture modifying arch_kgdb_ops during init.
> This patch makes the init static, so that it can be changed to
> const in following patch, as recommended by checkpatch.pl
> 
> Suggested-by: Paul Burton 
> Acked-by: Daniel Thompson 
> Acked-by: Paul Burton 
> Signed-off-by: Christophe Leroy 

Applied! Thanks.


> -
> ---
>  v2: Added acks from Daniel and Paul.
> 
>  arch/mips/kernel/kgdb.c | 16 +++-
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c
> index eb6c0d582626..31eff1bec577 100644
> --- a/arch/mips/kernel/kgdb.c
> +++ b/arch/mips/kernel/kgdb.c
> @@ -394,18 +394,16 @@ int kgdb_arch_handle_exception(int vector, int signo, 
> int err_code,
>   return -1;
>  }
>  
> -struct kgdb_arch arch_kgdb_ops;
> +struct kgdb_arch arch_kgdb_ops = {
> +#ifdef CONFIG_CPU_BIG_ENDIAN
> + .gdb_bpt_instr = { spec_op << 2, 0x00, 0x00, break_op },
> +#else
> + .gdb_bpt_instr = { break_op, 0x00, 0x00, spec_op << 2 },
> +#endif
> +};
>  
>  int kgdb_arch_init(void)
>  {
> - union mips_instruction insn = {
> - .r_format = {
> - .opcode = spec_op,
> - .func   = break_op,
> - }
> - };
> - memcpy(arch_kgdb_ops.gdb_bpt_instr, insn.byte, BREAK_INSTR_SIZE);
> -
>   register_die_notifier(_notifier);
>  
>   return 0;
> -- 
> 2.13.3
> 

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [REPOST PATCH v6 2/4] kgdb: Fix kgdb_roundup_cpus() for arches who used smp_call_function()

[Daniel Thompson]

On Tue, Dec 04, 2018 at 07:38:26PM -0800, Douglas Anderson wrote:
> When I had lockdep turned on and dropped into kgdb I got a nice splat
> on my system.  Specifically it hit:
>   DEBUG_LOCKS_WARN_ON(current->hardirq_context)
> 
> Specifically it looked like this:
>   sysrq: SysRq : DEBUG
>   [ cut here ]
>   DEBUG_LOCKS_WARN_ON(current->hardirq_context)
>   WARNING: CPU: 0 PID: 0 at .../kernel/locking/lockdep.c:2875 
> lockdep_hardirqs_on+0xf0/0x160
>   CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.19.0 #27
>   pstate: 604003c9 (nZCv DAIF +PAN -UAO)
>   pc : lockdep_hardirqs_on+0xf0/0x160
>   ...
>   Call trace:
>lockdep_hardirqs_on+0xf0/0x160
>trace_hardirqs_on+0x188/0x1ac
>kgdb_roundup_cpus+0x14/0x3c
>kgdb_cpu_enter+0x53c/0x5cc
>kgdb_handle_exception+0x180/0x1d4
>kgdb_compiled_brk_fn+0x30/0x3c
>brk_handler+0x134/0x178
>do_debug_exception+0xfc/0x178
>el1_dbg+0x18/0x78
>kgdb_breakpoint+0x34/0x58
>sysrq_handle_dbg+0x54/0x5c
>__handle_sysrq+0x114/0x21c
>handle_sysrq+0x30/0x3c
>qcom_geni_serial_isr+0x2dc/0x30c
>   ...
>   ...
>   irq event stamp: ...45
>   hardirqs last  enabled at (...44): [...] __do_softirq+0xd8/0x4e4
>   hardirqs last disabled at (...45): [...] el1_irq+0x74/0x130
>   softirqs last  enabled at (...42): [...] _local_bh_enable+0x2c/0x34
>   softirqs last disabled at (...43): [...] irq_exit+0xa8/0x100
>   ---[ end trace adf21f830c46e638 ]---
> 
> Looking closely at it, it seems like a really bad idea to be calling
> local_irq_enable() in kgdb_roundup_cpus().  If nothing else that seems
> like it could violate spinlock semantics and cause a deadlock.
> 
> Instead, let's use a private csd alongside
> smp_call_function_single_async() to round up the other CPUs.  Using
> smp_call_function_single_async() doesn't require interrupts to be
> enabled so we can remove the offending bit of code.
> 
> In order to avoid duplicating this across all the architectures that
> use the default kgdb_roundup_cpus(), we'll add a "weak" implementation
> to debug_core.c.
> 
> Looking at all the people who previously had copies of this code,
> there were a few variants.  I've attempted to keep the variants
> working like they used to.  Specifically:
> * For arch/arc we passed NULL to kgdb_nmicallback() instead of
>   get_irq_regs().
> * For arch/mips there was a bit of extra code around
>   kgdb_nmicallback()
> 
> NOTE: In this patch we will still get into trouble if we try to round
> up a CPU that failed to round up before.  We'll try to round it up
> again and potentially hang when we try to grab the csd lock.  That's
> not new behavior but we'll still try to do better in a future patch.
> 
> Suggested-by: Daniel Thompson 
> Signed-off-by: Douglas Anderson 
> Cc: Vineet Gupta 
> Cc: Russell King 
> Cc: Catalin Marinas 
> Cc: Will Deacon 
> Cc: Richard Kuo 
> Cc: Ralf Baechle 
> Cc: Paul Burton 
> Cc: James Hogan 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: Michael Ellerman 
> Cc: Yoshinori Sato 
> Cc: Rich Felker 
> Cc: "David S. Miller" 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Cc: "H. Peter Anvin" 
> Acked-by: Will Deacon 

I've not heard any objections from the arch/ maintainers so...

Applied! Thanks.


> -
> ---
> 
> Changes in v6:
> - Moved smp_call_function_single_async() error check to patch 3.
> 
> Changes in v5:
> - Add a comment about get_irq_regs().
> - get_cpu() => raw_smp_processor_id() in kgdb_roundup_cpus().
> - for_each_cpu() => for_each_online_cpu()
> - Error check smp_call_function_single_async()
> 
> Changes in v4: None
> Changes in v3:
> - No separate init call.
> - Don't round up the CPU that is doing the rounding up.
> - Add "#ifdef CONFIG_SMP" to match the rest of the file.
> - Updated desc saying we don't solve the "failed to roundup" case.
> - Document the ignored parameter.
> 
> Changes in v2:
> - Removing irq flags separated from fixing lockdep splat.
> - Don't use smp_call_function (Daniel).
> 
>  arch/arc/kernel/kgdb.c | 10 ++
>  arch/arm/kernel/kgdb.c | 12 ---
>  arch/arm64/kernel/kgdb.c   | 12 ---
>  arch/hexagon/kernel/kgdb.c | 27 -
>  arch/mips/kernel/kgdb.c|  9 +
>  arch/powerpc/kernel/kgdb.c |  4 ++--
>  arch/sh/kernel/kgdb.c  | 12 ---
>  include/linux/kgdb.h   | 15 --
>  kernel/debug/debug_core.c  | 41 ++
>  9 files changed, 59 insertions(+), 83 deletions(-)
> 
> diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c
> index 0932851028e0..68d9fe4b5aa7 100644
> --- a/arch/arc/kernel/kgdb.c
> +++ b/arch/arc/kernel/kgdb.c
> @@ -192,18 +192,12 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned 
> long ip)
>   instruction_pointer(regs) = ip;
>  }
>  
> -static void kgdb_call_nmi_hook(void *ignored)
> +void kgdb_call_nmi_hook(void *ignored)
>  {
> + /* Default implementation passes get_irq_regs() but we don't */
>   

Re: [REPOST PATCH v6 1/4] kgdb: Remove irq flags from roundup

[Daniel Thompson]

On Tue, Dec 04, 2018 at 07:38:25PM -0800, Douglas Anderson wrote:
> The function kgdb_roundup_cpus() was passed a parameter that was
> documented as:
> 
> > the flags that will be used when restoring the interrupts. There is
> > local_irq_save() call before kgdb_roundup_cpus().
> 
> Nobody used those flags.  Anyone who wanted to temporarily turn on
> interrupts just did local_irq_enable() and local_irq_disable() without
> looking at them.  So we can definitely remove the flags.
> 
> Signed-off-by: Douglas Anderson 
> Cc: Vineet Gupta 
> Cc: Russell King 
> Cc: Catalin Marinas 
> Cc: Will Deacon 
> Cc: Richard Kuo 
> Cc: Ralf Baechle 
> Cc: Paul Burton 
> Cc: James Hogan 
> Cc: Benjamin Herrenschmidt 
> Cc: Paul Mackerras 
> Cc: Michael Ellerman 
> Cc: Yoshinori Sato 
> Cc: Rich Felker 
> Cc: "David S. Miller" 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: Borislav Petkov 
> Cc: "H. Peter Anvin" 
> Acked-by: Will Deacon 

I've not heard any objections from the arch/ maintainers so...

Applied! Thanks.


> ---
> 
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v3: None
> Changes in v2:
> - Removing irq flags separated from fixing lockdep splat.
> 
>  arch/arc/kernel/kgdb.c | 2 +-
>  arch/arm/kernel/kgdb.c | 2 +-
>  arch/arm64/kernel/kgdb.c   | 2 +-
>  arch/hexagon/kernel/kgdb.c | 9 ++---
>  arch/mips/kernel/kgdb.c| 2 +-
>  arch/powerpc/kernel/kgdb.c | 2 +-
>  arch/sh/kernel/kgdb.c  | 2 +-
>  arch/sparc/kernel/smp_64.c | 2 +-
>  arch/x86/kernel/kgdb.c | 9 ++---
>  include/linux/kgdb.h   | 9 ++---
>  kernel/debug/debug_core.c  | 2 +-
>  11 files changed, 14 insertions(+), 29 deletions(-)
> 
> diff --git a/arch/arc/kernel/kgdb.c b/arch/arc/kernel/kgdb.c
> index 9a3c34af2ae8..0932851028e0 100644
> --- a/arch/arc/kernel/kgdb.c
> +++ b/arch/arc/kernel/kgdb.c
> @@ -197,7 +197,7 @@ static void kgdb_call_nmi_hook(void *ignored)
>   kgdb_nmicallback(raw_smp_processor_id(), NULL);
>  }
>  
> -void kgdb_roundup_cpus(unsigned long flags)
> +void kgdb_roundup_cpus(void)
>  {
>   local_irq_enable();
>   smp_call_function(kgdb_call_nmi_hook, NULL, 0);
> diff --git a/arch/arm/kernel/kgdb.c b/arch/arm/kernel/kgdb.c
> index caa0dbe3dc61..f21077b077be 100644
> --- a/arch/arm/kernel/kgdb.c
> +++ b/arch/arm/kernel/kgdb.c
> @@ -175,7 +175,7 @@ static void kgdb_call_nmi_hook(void *ignored)
> kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs());
>  }
>  
> -void kgdb_roundup_cpus(unsigned long flags)
> +void kgdb_roundup_cpus(void)
>  {
> local_irq_enable();
> smp_call_function(kgdb_call_nmi_hook, NULL, 0);
> diff --git a/arch/arm64/kernel/kgdb.c b/arch/arm64/kernel/kgdb.c
> index a20de58061a8..12c339ff6e75 100644
> --- a/arch/arm64/kernel/kgdb.c
> +++ b/arch/arm64/kernel/kgdb.c
> @@ -289,7 +289,7 @@ static void kgdb_call_nmi_hook(void *ignored)
>   kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs());
>  }
>  
> -void kgdb_roundup_cpus(unsigned long flags)
> +void kgdb_roundup_cpus(void)
>  {
>   local_irq_enable();
>   smp_call_function(kgdb_call_nmi_hook, NULL, 0);
> diff --git a/arch/hexagon/kernel/kgdb.c b/arch/hexagon/kernel/kgdb.c
> index 16c24b22d0b2..012e0e230ac2 100644
> --- a/arch/hexagon/kernel/kgdb.c
> +++ b/arch/hexagon/kernel/kgdb.c
> @@ -119,17 +119,12 @@ void kgdb_arch_set_pc(struct pt_regs *regs, unsigned 
> long pc)
>  
>  /**
>   * kgdb_roundup_cpus - Get other CPUs into a holding pattern
> - * @flags: Current IRQ state
>   *
>   * On SMP systems, we need to get the attention of the other CPUs
>   * and get them be in a known state.  This should do what is needed
>   * to get the other CPUs to call kgdb_wait(). Note that on some arches,
>   * the NMI approach is not used for rounding up all the CPUs. For example,
> - * in case of MIPS, smp_call_function() is used to roundup CPUs. In
> - * this case, we have to make sure that interrupts are enabled before
> - * calling smp_call_function(). The argument to this function is
> - * the flags that will be used when restoring the interrupts. There is
> - * local_irq_save() call before kgdb_roundup_cpus().
> + * in case of MIPS, smp_call_function() is used to roundup CPUs.
>   *
>   * On non-SMP systems, this is not called.
>   */
> @@ -139,7 +134,7 @@ static void hexagon_kgdb_nmi_hook(void *ignored)
>   kgdb_nmicallback(raw_smp_processor_id(), get_irq_regs());
>  }
>  
> -void kgdb_roundup_cpus(unsigned long flags)
> +void kgdb_roundup_cpus(void)
>  {
>   local_irq_enable();
>   smp_call_function(hexagon_kgdb_nmi_hook, NULL, 0);
> diff --git a/arch/mips/kernel/kgdb.c b/arch/mips/kernel/kgdb.c
> index eb6c0d582626..2b05effc17b4 100644
> --- a/arch/mips/kernel/kgdb.c
> +++ b/arch/mips/kernel/kgdb.c
> @@ -219,7 +219,7 @@ static void kgdb_call_nmi_hook(void *ignored)
>   set_fs(old_fs);
>  }
>  
> -void kgdb_roundup_cpus(unsigned long flags)
> +void kgdb_roundup_cpus(void)
>  {
>   local_irq_enable();
>   

Re: [PATCH] arc: remove redundant kernel-space generic-y

[Vineet Gupta]

On 12/19/18 7:16 AM, Masahiro Yamada wrote:
> Could you pick this up to your arc tree?

Done, will push it in a day or so !

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: remove redundant kernel-space generic-y

[Masahiro Yamada]

Hi Vineet,

On Mon, Dec 17, 2018 at 2:07 PM Vineet Gupta  wrote:
>
> On 12/16/18 6:17 AM, Masahiro Yamada wrote:
> > This commit removes redundant generic-y defines in
> > arch/arc/include/asm/Kbuild.
> >
> > It is redundant to define generic-y when arch-specific implementation
> > exists in arch/$(ARCH)/include/asm/*.h
> >
> > Remove the following generic-y:
> >
> > dma-mapping.h
> > fb.h
> > kmap_types.h
> > pci.h
> >
> > Signed-off-by: Masahiro Yamada 
>
> Acked-by: Vineet Gupta 


Could you pick this up to your arc tree?


-- 
Best Regards
Masahiro Yamada

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc

Re: [PATCH 12/21] ARC: Update syscall-names.list for ARC specific syscalls

[Florian Weimer]

* Vineet Gupta:

> On 12/18/18 1:26 PM, Florian Weimer wrote:
>> This should come in with a sync with a released upstream kernel.  Will
>> the port be in Linux 4.20?
>
> linux port was upstreamed in 2013 in v3.9

Ah, right, we add system calls to the lists only for our ports.

Thanks,
Florian

___
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc