Thread (33 messages) 33 messages, 4 authors, 2014-03-07

Re: [PATCH 3/4] sparc64: convert spinlock_t to raw_spinlock_t in mmu_context_t

From: Allen Pais <hidden>
Date: 2014-02-19 09:31:29
Also in: sparclinux

On Wednesday 19 February 2014 02:55 PM, Kirill Tkhai wrote:
19.02.2014, 13:13, "Allen Pais" [off-list ref]:
quoted
 On Wednesday 19 February 2014 02:27 PM, Kirill Tkhai wrote:
quoted
  19.02.2014, 12:12, "Allen Pais" [off-list ref]:
quoted
quoted
quoted
   diff --git a/arch/sparc/mm/tsb.c b/arch/sparc/mm/tsb.c
   index 9eb10b4..24dcd29 100644
   --- a/arch/sparc/mm/tsb.c
   +++ b/arch/sparc/mm/tsb.c
   @@ -6,6 +6,7 @@
    #include <linux/kernel.h>
    #include <linux/preempt.h>
    #include <linux/slab.h>
   +#include <linux/locallock.h>
    #include <asm/page.h>
    #include <asm/pgtable.h>
    #include <asm/mmu_context.h>
   @@ -14,6 +15,7 @@
    #include <asm/oplib.h>

    extern struct tsb swapper_tsb[KERNEL_TSB_NENTRIES];
   +static DEFINE_LOCAL_IRQ_LOCK(tsb_lock);

    static inline unsigned long tsb_hash(unsigned long vaddr, unsigned long hash_sh
    {
   @@ -71,9 +73,9 @@ static void __flush_tsb_one(struct tlb_batch *tb, unsigned lon
    void flush_tsb_user(struct tlb_batch *tb)
    {
           struct mm_struct *mm = tb->mm;
   -       unsigned long nentries, base, flags;
   +       unsigned long nentries, base;

   -       raw_spin_lock_irqsave(&mm->context.lock, flags);
   +       local_lock(tsb_lock);

           base = (unsigned long) mm->context.tsb_block[MM_TSB_BASE].tsb;
           nentries = mm->context.tsb_block[MM_TSB_BASE].tsb_nentries;
   @@ -90,7 +92,7 @@ void flush_tsb_user(struct tlb_batch *tb)
                   __flush_tsb_one(tb, HPAGE_SHIFT, base, nentries);
           }
    #endif
   -       raw_spin_unlock_irqrestore(&mm->context.lock, flags);
   +       local_unlock(tsb_lock);
   It seems to be not good for me. Tsb setup is in tsb_grow() and it must
   be synchronized with flushing. Flushing is also being made in flush_tsb_user_page()..

   Which last stack stack has you received with tb->active, permanently set to zero?
  I agree with you point about flushing in flush_tbs_user_page too. Like i said, this is
  a bit tricky to actually debug.

  Yes, tb->active was set to zero.
  If tb->active is zero, flush_tsb_user() is never called, because of tlb_nr is permanently zero.
 Sorry, my bad. tb->active was set to one when I ran the test with the above patch.
It seems for me it's better to decide the problem not changing protector of tsb like in patch above.
You may get good stack without sun4v_data_access_exception error, which was in the first or second
message.
I agree, I hope to get more a few opinions on the patches/issues. Meanwhile, Lemme see if I could
manage a way to stick to the old protectors and fix the stalls.

- Allen
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help