diff for duplicates of <20181022015258.GA3649@guoren-Inspiron-7460> diff --git a/a/1.txt b/N1/1.txt index 32ebe86..a94efe2 100644 --- a/a/1.txt +++ b/N1/1.txt @@ -18,115 +18,3 @@ Yes, approve! I should use WRITE_ONCE/READ_ONCE as necessary. > implementations; to me that is just extra maintenance overhead. Test and set (spinlock & rwlock) is easier for debug :P, and I don't know the details of queue-rwlock (maybe I should learn it). - -From education's view, we could teach students both of them in -arch/csky :) - -Anyway, I just want to keep both of them. - -Thx - -> > + asm volatile ( -> > + " movi %0, 0 \n" -> > + " stw %0, (%1) \n" -> > + : "=&r" (tmp) -> > + : "r"(p) -> > + : "cc"); -> -> WRITE_ONCE(lock->lock, 0); -> ? -Cool ... I like WRITE_ONCE style. - -> > + -> > +#define arch_spin_is_locked(x) (READ_ONCE((x)->lock) != 0) -> > + -> > +/* -> > + * read lock/unlock/trylock -> > + */ -> -> Idem, why do you want a second rwlock_t implementation? -The same as above of spinlock. - -> > + asm volatile ( -> > + "1: ldex.w %0, (%1) \n" -> > + " movi %0, 0 \n" -> > + " stex.w %0, (%1) \n" -> > + " bez %0, 1b \n" -> > + : "=&r" (tmp) -> > + : "r"(p) -> > + : "cc"); -> -> Isn't that: -> -> WRITE_ONCE(lock->lock, 0); -Yes, no need ldex/stex and you've mentioned in spinlock before :P - -> > diff --git a/arch/csky/kernel/atomic.S b/arch/csky/kernel/atomic.S -> > new file mode 100644 -> > index 0000000..d2357c8 -> > --- /dev/null -> > +++ b/arch/csky/kernel/atomic.S -> > @@ -0,0 +1,87 @@ -> > +/* SPDX-License-Identifier: GPL-2.0 */ -> > +// Copyright (C) 2018 Hangzhou C-SKY Microsystems co.,ltd. -> > + -> > +#include <linux/linkage.h> -> > +#include <abi/entry.h> -> > + -> > +.text -> > + -> > +/* -> > + * int csky_cmpxchg(int oldval, int newval, int *ptr) -> > + * -> > + * If *ptr != oldval && return 1, -> > + * else *ptr = newval return 0. -> > + */ -> > +#ifdef CONFIG_CPU_HAS_LDSTEX -> > +ENTRY(csky_cmpxchg) -> > + USPTOKSP -> > + mfcr a3, epc -> > + INCTRAP a3 -> > + -> > + subi sp, 8 -> > + stw a3, (sp, 0) -> > + mfcr a3, epsr -> > + stw a3, (sp, 4) -> > + -> > + psrset ee -> > +1: -> > + ldex a3, (a2) -> > + cmpne a0, a3 -> > + bt16 2f -> > + mov a3, a1 -> > + stex a3, (a2) -> > + bez a3, 1b -> > +2: -> > + sync.is -> > + mvc a0 -> > + ldw a3, (sp, 0) -> > + mtcr a3, epc -> > + ldw a3, (sp, 4) -> > + mtcr a3, epsr -> > + addi sp, 8 -> > + KSPTOUSP -> > + rte -> > +END(csky_cmpxchg) -> -> I don't understand why you have this; if the CPU has ll/sc, why do you -> need syscall support? -I've really considered your advice before, but from abi view 610/807/810 -all have csky_cmpxchg trap and we want to make them the same. Some apps -use the trap directly and not use libc api. Maybe we could delete the -trap in future version of kernel. - -> -> In any case, nothing terminally broken; so I suppose that's good enough -> for starters. I just really don't understand some decisions (like having -> two lock implementations and having that cmpxchg syscall when you have -> hardware ll/sc). -> -> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> -Thx peter and the two questions which I've clarified in above. - -Best Regards - Guo Ren diff --git a/a/content_digest b/N1/content_digest index 0ec296a..dfa3a84 100644 --- a/a/content_digest +++ b/N1/content_digest @@ -42,118 +42,6 @@ "> I'm still not entirely sure why you want to have two spinlock\n" "> implementations; to me that is just extra maintenance overhead.\n" "Test and set (spinlock & rwlock) is easier for debug :P, and I don't\n" - "know the details of queue-rwlock (maybe I should learn it).\n" - "\n" - "From education's view, we could teach students both of them in\n" - "arch/csky :)\n" - "\n" - "Anyway, I just want to keep both of them.\n" - "\n" - "Thx\n" - "\n" - "> > +\tasm volatile (\n" - "> > +\t\t\"\tmovi\t\t%0, 0 \\n\"\n" - "> > +\t\t\"\tstw\t\t%0, (%1) \\n\"\n" - "> > +\t\t: \"=&r\" (tmp)\n" - "> > +\t\t: \"r\"(p)\n" - "> > +\t\t: \"cc\");\n" - "> \n" - "> \tWRITE_ONCE(lock->lock, 0);\n" - "> ?\n" - "Cool ... I like WRITE_ONCE style. \n" - "\n" - "> > +\n" - "> > +#define arch_spin_is_locked(x)\t(READ_ONCE((x)->lock) != 0)\n" - "> > +\n" - "> > +/*\n" - "> > + * read lock/unlock/trylock\n" - "> > + */\n" - "> \n" - "> Idem, why do you want a second rwlock_t implementation?\n" - "The same as above of spinlock.\n" - "\n" - "> > +\tasm volatile (\n" - "> > +\t\t\"1:\tldex.w\t\t%0, (%1) \\n\"\n" - "> > +\t\t\"\tmovi\t\t%0, 0 \\n\"\n" - "> > +\t\t\"\tstex.w\t\t%0, (%1) \\n\"\n" - "> > +\t\t\"\tbez\t\t%0, 1b \\n\"\n" - "> > +\t\t: \"=&r\" (tmp)\n" - "> > +\t\t: \"r\"(p)\n" - "> > +\t\t: \"cc\");\n" - "> \n" - "> Isn't that:\n" - "> \n" - "> \tWRITE_ONCE(lock->lock, 0);\n" - "Yes, no need ldex/stex and you've mentioned in spinlock before :P\n" - "\n" - "> > diff --git a/arch/csky/kernel/atomic.S b/arch/csky/kernel/atomic.S\n" - "> > new file mode 100644\n" - "> > index 0000000..d2357c8\n" - "> > --- /dev/null\n" - "> > +++ b/arch/csky/kernel/atomic.S\n" - "> > @@ -0,0 +1,87 @@\n" - "> > +/* SPDX-License-Identifier: GPL-2.0 */\n" - "> > +// Copyright (C) 2018 Hangzhou C-SKY Microsystems co.,ltd.\n" - "> > +\n" - "> > +#include <linux/linkage.h>\n" - "> > +#include <abi/entry.h>\n" - "> > +\n" - "> > +.text\n" - "> > +\n" - "> > +/*\n" - "> > + * int csky_cmpxchg(int oldval, int newval, int *ptr)\n" - "> > + *\n" - "> > + * If *ptr != oldval && return 1,\n" - "> > + * else *ptr = newval return 0.\n" - "> > + */\n" - "> > +#ifdef CONFIG_CPU_HAS_LDSTEX\n" - "> > +ENTRY(csky_cmpxchg)\n" - "> > +\tUSPTOKSP\n" - "> > +\tmfcr\ta3, epc\n" - "> > +\tINCTRAP\ta3\n" - "> > +\n" - "> > +\tsubi sp, 8\n" - "> > +\tstw a3, (sp, 0)\n" - "> > +\tmfcr a3, epsr\n" - "> > +\tstw a3, (sp, 4)\n" - "> > + \n" - "> > +\tpsrset\tee\n" - "> > +1:\n" - "> > +\tldex\ta3, (a2)\n" - "> > +\tcmpne\ta0, a3\n" - "> > +\tbt16\t2f\n" - "> > +\tmov\ta3, a1\n" - "> > +\tstex\ta3, (a2)\n" - "> > +\tbez\ta3, 1b\n" - "> > +2:\n" - "> > +\tsync.is\n" - "> > +\tmvc\ta0\n" - "> > +\tldw\ta3, (sp, 0)\n" - "> > +\tmtcr\ta3, epc\n" - "> > +\tldw a3, (sp, 4)\n" - "> > +\tmtcr\ta3, epsr\n" - "> > +\taddi\tsp, 8\n" - "> > +\tKSPTOUSP\n" - "> > +\trte\n" - "> > +END(csky_cmpxchg)\n" - "> \n" - "> I don't understand why you have this; if the CPU has ll/sc, why do you\n" - "> need syscall support?\n" - "I've really considered your advice before, but from abi view 610/807/810\n" - "all have csky_cmpxchg trap and we want to make them the same. Some apps\n" - "use the trap directly and not use libc api. Maybe we could delete the\n" - "trap in future version of kernel.\n" - "\n" - "> \n" - "> In any case, nothing terminally broken; so I suppose that's good enough\n" - "> for starters. I just really don't understand some decisions (like having\n" - "> two lock implementations and having that cmpxchg syscall when you have\n" - "> hardware ll/sc).\n" - "> \n" - "> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>\n" - "Thx peter and the two questions which I've clarified in above.\n" - "\n" - "Best Regards\n" - Guo Ren + know the details of queue-rwlock (maybe I should learn it). -28b83612c9d53cfc6bbdcc79dd48edeefb901d86af1ec0e95b37c556e5d9ef50 +3db84a99bc17bdda25a547cdd83a662dfd7ac280bc5049ee875a815a67f1cb59
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox