bind9 failures on mips

[Robert Edmonds <edmonds@debian.org>]

Hi,

I've been reviewing bugs filed against bind9, and I came across #778720
("Bin[d]9 hangs on mips jessie based machine"), which points to a forum
thread [0] where multiple people have reported problems running our
bind9 package on mips-based networking devices.

The workaround cited in #778720 is to build with "--disable-atomic" on
mips.  That's not a patch, just a workaround; the multi-threaded
red-black tree implementation in BIND uses different (presumably less
efficient) code paths if the atomic operations are not available.

Looking into the actual atomic implementation in bind9 on MIPS, I found
something interesting: our lib/isc/mips/include/isc/atomic.h [1] is
completely different from upstream's version [2].  According to our
bind9 packaging repository, the changes to the mips implementation [3,4]
were due to #406409 and #516616.  From reading the bug log for #516616,
it looks like the changes were reviewed by several people from the
debian-mips@ mailing list in 2010.  So, given that bind9 is crashing on
mips, I'm requesting re-review :-)

There are three isc_atomic_* functions implemented for mips:
isc_atomic_xadd(), isc_atomic_store(), and isc_atomic_cmpxchg().

I think isc_atomic_xadd() is probably correct: the "t_atomic" test in
the BIND9 test suite exercises that function, and passes when I run it
manually on minkus.

isc_atomic_store() is probably also correct, or at least probably not
the culprit.  It is only used in one place in the entire BIND9 source
tree, to update a 32-bit timestamp in the acache code.

That leaves the most complicated operation, isc_atomic_cmpxchg(), which
unfortunately isn't exercised by the "t_atomic" test program.  Does this
code [5] look correct?

    /*
     * This routine atomically replaces the value in 'p' with 'val', if the
     * original value is equal to 'cmpval'.  The original value is returned in any
     * case.
     */
    static inline isc_int32_t
    isc_atomic_cmpxchg(isc_int32_t *p, int cmpval, int val) {
        isc_int32_t orig;
        isc_int32_t tmp;

        __asm__ __volatile__ (
        "   .set    push        \n"
        "   .set    mips2       \n"
        "   .set    noreorder   \n"
        "   .set    noat        \n"
        "1: ll  $1, %1      \n"
        "   bne $1, %3, 2f  \n"
        "   move    %2, %4      \n"
        "   sc  %2, %1      \n"
        "   beqz    %2, 1b      \n"
        "2: move    %0, $1      \n"
        "   .set    pop     \n"
        : "=&r"(orig), "+R" (*p), "=r" (tmp)
        : "r"(cmpval), "r"(val)
        : "memory");

        return (orig);
    }

Thanks!

[0] http://community.ubnt.com/t5/EdgeMAX/How-To-Install-BIND-DNS-Server-on-EdgeRouter/td-p/558349/page/3

[1] http://sources.debian.net/src/bind9/1:9.9.5.dfsg-12/lib/isc/mips/include/isc/atomic.h/

[2] https://source.isc.org/cgi-bin/gitweb.cgi?p=bind9.git;a=blob;f=lib/isc/mips/include/isc/atomic.h;h=bb739f7404a9d5f683a40b8b81e299014511d3d6;hb=refs/heads/master

[3] http://anonscm.debian.org/cgit/users/lamont/bind9.git/commit/?id=14cb2d0750593e2d51c7e028a678f3fd00775fea

[4] http://anonscm.debian.org/cgit/users/lamont/bind9.git/commit/?id=98b8d193db59e79d7f98df904852988ed1dd71da

[5] http://sources.debian.net/src/bind9/1:9.9.5.dfsg-12/lib/isc/mips/include/isc/atomic.h/#L60-L87

-- 
Robert Edmonds
edmonds@debian.org