Bug 193400 - [mips] r269577 broke operation on MIPS32
Summary: [mips] r269577 broke operation on MIPS32
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: mips Any
: --- Affects Only Me
Assignee: freebsd-bugs (Nobody)
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-09-06 21:20 UTC by Adrian Chadd
Modified: 2014-09-09 14:50 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Adrian Chadd freebsd_committer freebsd_triage 2014-09-06 21:20:12 UTC
Hi Gleb!

This commit has broken mips32 on my 128MB RAM routerstation pro board.

I've tested the commit before this one (and it works).

I've also tested today's -HEAD as there was some subsequent fixes to
the sfbuf code. It hasn't completely fixed things - I still see
processes throwing VM errors:

umass0:0:0: Attached to scbus0
Trying to mount root from ufs:redboot/rootfs.uzip []...
warning: no time-of-day clock registered, system time will not be set accurately
da0 at umass-sim0 bus 0 scbus0 target 0 lun 0
da0: <Generic STORAGE DEVICE 9451> Removable Direct Access SCSI-0 device
da0: Serial Number 000000009451
da0: 40.000MB/s transfers
da0: 3902MB (7991296 512 byte sectors: 255H 63S/T 497C)
da0: quirks=0x3<NO_SYNC_CACHE,NO_6_BYTE>
Aug 28 08:07:57 init: login_getclass: unknown class 'daemon'
BAD_PAGE_FAULT: pid 27 tid 100045 (mount), uid 0: pc 0x40514dd0 got a
read fault (type 0x2) at 0x4040011c
Trapframe Register Dump:
        zero: 0 at: 0x7fffffff  v0: 0   v1: 0x404000fc
        a0: 0x54        a1: 0x40400000  a2: 0   a3: 0x1
        t0: 0   t1: 0x40c0300c  t2: 0x40800168  t3: 0x2f
        t4: 0x40c00030  t5: 0   t6: 0x748       t7: 0x402c70
        t8: 0x13        t9: 0x40514d58  s0: 0x3 s1: 0x40418798
        s2: 0   s3: 0x404ec4    s4: 0x40418798  s5: 0x40418798
        s6: 0   s7: 0   k0: 0   k1: 0
        gp: 0x405ec910  sp: 0x7ffee348  s8: 0   ra: 0x4051534c
        sr: 0xfc13      mullo: 0x6719   mulhi: 0xc      badvaddr: 0x4040011c
        cause: 0x8      pc: 0x40514dd0
Page table info for pc address 0x40514dd0: pde = 0x813da000, pte = 0xa00569da
Dumping 4 words starting at pc address 0x40514dd0:
8c700020 32030ff0 00032102 240300ff
Page table info for bad address 0x4040011c: pde = 0x813da000, pte = 0
pid 27 (mount), uid 0: exited on signal 11
*** Populating /var ..

.. so how can we debug what's going on?
Comment 1 Adrian Chadd freebsd_committer freebsd_triage 2014-09-06 21:22:44 UTC
Oh, and this is mipseb, not mipsel.
Comment 2 Adrian Chadd freebsd_committer freebsd_triage 2014-09-06 22:20:12 UTC
Ok, here's what fixed it (ignore the error checks):

adrian@adrian-hackbox:~/work/freebsd/embedded/head/src/sys % svn diff kern
Index: kern/subr_sfbuf.c
===================================================================
--- kern/subr_sfbuf.c   (revision 271210)
+++ kern/subr_sfbuf.c   (working copy)
@@ -77,6 +77,14 @@
  */
 static struct mtx sf_buf_lock;
 
+#ifndef SFBUF
+#error wtf?
+#endif
+
+#ifdef SFBUF_OPTIONAL_DIRECT_MAP
+#error wtf?
+#endif
+
 /*
  * Allocate a pool of sf_bufs (sendfile(2) or "super-fast" if you prefer. :-))
  */
@@ -127,6 +135,7 @@
            ("sf_buf_alloc(SFB_CPUPRIVATE): curthread not pinned"));
        hash_list = &sf_buf_active[SF_BUF_HASH(m)];
        mtx_lock(&sf_buf_lock);
+#if 0
        LIST_FOREACH(sf, hash_list, list_entry) {
                if (sf->m == m) {
                        sf->ref_count++;
@@ -141,6 +150,7 @@
                        goto done;
                }
        }
+#endif
        while ((sf = TAILQ_FIRST(&sf_buf_freelist)) == NULL) {
                if (flags & SFB_NOWAIT)
                        goto done;

.. the original MIPS sfbuf code didn't do the refcount stuff; it just allocated a new page for each sfbuf requested. I don't know how correct that was, but it worked.

So maybe you've exposed a bug in the MIPS pmap code?


-a
Comment 3 Adrian Chadd freebsd_committer freebsd_triage 2014-09-06 22:34:30 UTC
Actually, here's a potential fix:

Index: sys/mips/include/sf_buf.h
===================================================================
--- sys/mips/include/sf_buf.h   (revision 271210)
+++ sys/mips/include/sf_buf.h   (working copy)
@@ -48,4 +48,23 @@
 }
 
 #endif /* __mips_n64 */
+
+#ifndef        __mips_n64      /* in 32 bit mode we manage our own mappings */
+
+static inline void
+sf_buf_map(struct sf_buf *sf, int flags)
+{
+
+       pmap_qenter(sf->kva, &sf->m, 1);
+}
+
+static inline int
+sf_buf_unmap(struct sf_buf *sf)
+{
+       pmap_qremove(sf->kva, 1);
+       return (1);
+}
+
+#endif /* ! __mips_n64 */
+
 #endif /* !_MACHINE_SF_BUF_H_ */
Index: sys/mips/include/vmparam.h
===================================================================
--- sys/mips/include/vmparam.h  (revision 271210)
+++ sys/mips/include/vmparam.h  (working copy)
@@ -189,6 +189,7 @@
 
 #ifndef __mips_n64
 #define        SFBUF
+#define        SFBUF_MAP
 #endif
 
 #endif /* !_MACHINE_VMPARAM_H_ */


.. the MIPS sfbuf code didn't keep the temporarily mapped kva bits around. It recycled them each time.
Comment 4 Adrian Chadd freebsd_committer freebsd_triage 2014-09-06 22:38:54 UTC
Committed the fix in r271213.
Comment 5 commit-hook freebsd_committer freebsd_triage 2014-09-06 22:39:18 UTC
A commit references this bug:

Author: adrian
Date: Sat Sep  6 22:38:33 UTC 2014
New revision: 271213
URL: http://svnweb.freebsd.org/changeset/base/271213

Log:
  Implement local sfbuf_map and sfbuf_unmap for MIPS32.

  The pre-rework behaviour was not to keep the cached mappings around after
  the sfbuf was used but instead to recycle said mappings.

  PR:		kern/193400

Changes:
  head/sys/mips/include/sf_buf.h
  head/sys/mips/include/vmparam.h
Comment 6 Adrian Chadd freebsd_committer freebsd_triage 2014-09-07 01:34:37 UTC
alc's asked me to test:

> Is this something that should be fixed in the vm/pmap code somewhere,
> or is this just the correct behaviour?

No.  However, if you feel like tinkering, you could replace the body of
sf_buf_unmap() with

mips_dcache_wbinv_range_index(va, PAGE_SIZE);
return (0);

and see what happens.


-a