Re: CVS commit: src/sys/dev/pci

To: Marc Balmer <mbalmer%NetBSD.org@localhost>
Subject: Re: CVS commit: src/sys/dev/pci
From: Paul Goyette <paul%whooppee.com@localhost>
Date: Tue, 14 Feb 2012 08:56:06 -0800 (PST)

Log message has been updated in cvs:

Replace the xxx_acquire()/xxx_release() rwlocks with mutexes.  There are
only RW_WRITERs for these, and no RW_READERs, so no need to incur the
extra overhead of allowing for both.  As discussed on tech-kern.

For piixpm and ichsmb, the acquire/release protocol needs to be used,even if the request is I2C_F_POLL'd (or if the device supports only

polled mode).  Otherwise multiple requests can be running at the same
time, and they stomp on each other and create anomolous results.

Part 2 addresses my PR kern/45889

3 ACKs from releng



On Tue, 14 Feb 2012, Marc Balmer wrote:

Am 14.02.12 16:08, schrieb Paul Goyette:

Module Name:    src
Committed By:   pgoyette
Date:           Tue Feb 14 15:08:07 UTC 2012

Modified Files:
        src/sys/dev/pci: amdpm_smbus.c amdpmvar.h ichsmb.c nfsmb.c piixpm.c

Log Message:
/home/paul/COMMIT


could you please be a bit more verbose?



To generate a diff of this commit:
cvs rdiff -u -r1.17 -r1.18 src/sys/dev/pci/amdpm_smbus.c
cvs rdiff -u -r1.7 -r1.8 src/sys/dev/pci/amdpmvar.h
cvs rdiff -u -r1.26 -r1.27 src/sys/dev/pci/ichsmb.c
cvs rdiff -u -r1.22 -r1.23 src/sys/dev/pci/nfsmb.c
cvs rdiff -u -r1.39 -r1.40 src/sys/dev/pci/piixpm.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.




Modified files:

Index: src/sys/dev/pci/amdpm_smbus.c
diff -u src/sys/dev/pci/amdpm_smbus.c:1.17 src/sys/dev/pci/amdpm_smbus.c:1.18
--- src/sys/dev/pci/amdpm_smbus.c:1.17  Sat Nov 19 02:39:14 2011
+++ src/sys/dev/pci/amdpm_smbus.c       Tue Feb 14 15:08:07 2012
@@ -1,4 +1,4 @@
-/*     $NetBSD: amdpm_smbus.c,v 1.17 2011/11/19 02:39:14 christos Exp $ */
+/*     $NetBSD: amdpm_smbus.c,v 1.18 2012/02/14 15:08:07 pgoyette Exp $ */

 /*
  * Copyright (c) 2005 Anil Gopinath (anil_public%yahoo.com@localhost)
@@ -32,14 +32,14 @@
  * AMD-8111 HyperTransport I/O Hub
  */
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: amdpm_smbus.c,v 1.17 2011/11/19 02:39:14 christos Exp 
$");
+__KERNEL_RCSID(0, "$NetBSD: amdpm_smbus.c,v 1.18 2012/02/14 15:08:07 pgoyette Exp 
$");

 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/kernel.h>
 #include <sys/device.h>
 #include <sys/rnd.h>
-#include <sys/rwlock.h>
+#include <sys/mutex.h>

 #include <dev/pci/pcireg.h>
 #include <dev/pci/pcivar.h>
@@ -83,7 +83,7 @@ amdpm_smbus_attach(struct amdpm_softc *s
        sc->sc_i2c.ic_write_byte = NULL;
        sc->sc_i2c.ic_exec = amdpm_smbus_exec;

-       rw_init(&sc->sc_rwlock);
+       mutex_init(&sc->sc_mutex, MUTEX_DEFAULT, IPL_NONE);

        iba.iba_tag = &sc->sc_i2c;
        (void)config_found_ia(&sc->sc_dev, "i2cbus", &iba, iicbus_print);
@@ -94,7 +94,7 @@ amdpm_smbus_acquire_bus(void *cookie, in
 {
        struct amdpm_softc *sc = cookie;

-       rw_enter(&sc->sc_rwlock, RW_WRITER);
+       mutex_enter(&sc->sc_mutex);
        return 0;
 }

@@ -103,7 +103,7 @@ amdpm_smbus_release_bus(void *cookie, in
 {
        struct amdpm_softc *sc = cookie;

-       rw_exit(&sc->sc_rwlock);
+       mutex_exit(&sc->sc_mutex);
 }

 static int

Index: src/sys/dev/pci/amdpmvar.h
diff -u src/sys/dev/pci/amdpmvar.h:1.7 src/sys/dev/pci/amdpmvar.h:1.8
--- src/sys/dev/pci/amdpmvar.h:1.7      Sat Nov 19 22:51:23 2011
+++ src/sys/dev/pci/amdpmvar.h  Tue Feb 14 15:08:07 2012
@@ -1,4 +1,4 @@
-/*     $NetBSD: amdpmvar.h,v 1.7 2011/11/19 22:51:23 tls Exp $ */
+/*     $NetBSD: amdpmvar.h,v 1.8 2012/02/14 15:08:07 pgoyette Exp $    */

 /*-
  * Copyright (c) 2002 The NetBSD Foundation, Inc.
@@ -32,7 +32,7 @@
 #ifndef _DEV_PCI_AMDPMVAR_H_
 #define _DEV_PCI_AMDPMVAR_H_

-#include <sys/rwlock.h>
+#include <sys/mutex.h>

 struct amdpm_softc {
        struct device sc_dev;
@@ -47,7 +47,7 @@ struct amdpm_softc {

        i2c_addr_t sc_smbus_slaveaddr;          /* address of smbus slave */
        struct i2c_controller sc_i2c;           /* i2c controller info */
-       krwlock_t sc_rwlock;
+       kmutex_t sc_mutex;

        void *sc_ih;


Index: src/sys/dev/pci/ichsmb.c
diff -u src/sys/dev/pci/ichsmb.c:1.26 src/sys/dev/pci/ichsmb.c:1.27
--- src/sys/dev/pci/ichsmb.c:1.26       Mon Jan 30 19:41:19 2012
+++ src/sys/dev/pci/ichsmb.c    Tue Feb 14 15:08:07 2012
@@ -1,4 +1,4 @@
-/*     $NetBSD: ichsmb.c,v 1.26 2012/01/30 19:41:19 drochner Exp $     */
+/*     $NetBSD: ichsmb.c,v 1.27 2012/02/14 15:08:07 pgoyette Exp $     */
 /*     $OpenBSD: ichiic.c,v 1.18 2007/05/03 09:36:26 dlg Exp $ */

 /*
@@ -22,13 +22,13 @@
  */

 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ichsmb.c,v 1.26 2012/01/30 19:41:19 drochner Exp 
$");
+__KERNEL_RCSID(0, "$NetBSD: ichsmb.c,v 1.27 2012/02/14 15:08:07 pgoyette Exp 
$");

 #include <sys/param.h>
 #include <sys/device.h>
 #include <sys/errno.h>
 #include <sys/kernel.h>
-#include <sys/rwlock.h>
+#include <sys/mutex.h>
 #include <sys/proc.h>

 #include <sys/bus.h>
@@ -59,7 +59,7 @@ struct ichsmb_softc {
        int                     sc_poll;

        struct i2c_controller   sc_i2c_tag;
-       krwlock_t               sc_i2c_rwlock;
+       kmutex_t                sc_i2c_mutex;
        struct {
                i2c_op_t     op;
                void *       buf;
@@ -166,7 +166,7 @@ ichsmb_attach(device_t parent, device_t
        }

        /* Attach I2C bus */
-       rw_init(&sc->sc_i2c_rwlock);
+       mutex_init(&sc->sc_i2c_mutex, MUTEX_DEFAULT, IPL_NONE);
        sc->sc_i2c_tag.ic_cookie = sc;
        sc->sc_i2c_tag.ic_acquire_bus = ichsmb_i2c_acquire_bus;
        sc->sc_i2c_tag.ic_release_bus = ichsmb_i2c_release_bus;
@@ -186,10 +186,10 @@ ichsmb_i2c_acquire_bus(void *cookie, int
 {
        struct ichsmb_softc *sc = cookie;

-       if (cold || sc->sc_poll || (flags & I2C_F_POLL))
+       if (cold)
                return 0;

-       rw_enter(&sc->sc_i2c_rwlock, RW_WRITER);
+       mutex_enter(&sc->sc_i2c_mutex);
        return 0;
 }

@@ -198,10 +198,10 @@ ichsmb_i2c_release_bus(void *cookie, int
 {
        struct ichsmb_softc *sc = cookie;

-       if (cold || sc->sc_poll || (flags & I2C_F_POLL))
+       if (cold)
                return;

-       rw_exit(&sc->sc_i2c_rwlock);
+       mutex_exit(&sc->sc_i2c_mutex);
 }

 static int

Index: src/sys/dev/pci/nfsmb.c
diff -u src/sys/dev/pci/nfsmb.c:1.22 src/sys/dev/pci/nfsmb.c:1.23
--- src/sys/dev/pci/nfsmb.c:1.22        Mon Jan 30 19:41:22 2012
+++ src/sys/dev/pci/nfsmb.c     Tue Feb 14 15:08:07 2012
@@ -1,4 +1,4 @@
-/*     $NetBSD: nfsmb.c,v 1.22 2012/01/30 19:41:22 drochner Exp $      */
+/*     $NetBSD: nfsmb.c,v 1.23 2012/02/14 15:08:07 pgoyette Exp $      */
 /*
  * Copyright (c) 2007 KIYOHARA Takashi
  * All rights reserved.
@@ -26,13 +26,13 @@
  *
  */
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nfsmb.c,v 1.22 2012/01/30 19:41:22 drochner Exp 
$");
+__KERNEL_RCSID(0, "$NetBSD: nfsmb.c,v 1.23 2012/02/14 15:08:07 pgoyette Exp 
$");

 #include <sys/param.h>
 #include <sys/device.h>
 #include <sys/errno.h>
 #include <sys/kernel.h>
-#include <sys/rwlock.h>
+#include <sys/mutex.h>
 #include <sys/proc.h>

 #include <sys/bus.h>
@@ -73,7 +73,7 @@ struct nfsmb_softc {
        bus_space_handle_t sc_ioh;

        struct i2c_controller sc_i2c;   /* i2c controller info */
-       krwlock_t sc_rwlock;
+       kmutex_t sc_mutex;
 };


@@ -237,7 +237,7 @@ nfsmb_attach(device_t parent, device_t s
        sc->sc_i2c.ic_write_byte = NULL;
        sc->sc_i2c.ic_exec = nfsmb_exec;

-       rw_init(&sc->sc_rwlock);
+       mutex_init(&sc->sc_mutex, MUTEX_DEFAULT, IPL_NONE);

        if (bus_space_map(sc->sc_iot, nfsmbcap->nfsmb_addr, NFORCE_SMBSIZE, 0,
            &sc->sc_ioh) != 0) {
@@ -261,7 +261,7 @@ nfsmb_acquire_bus(void *cookie, int flag
 {
        struct nfsmb_softc *sc = cookie;

-       rw_enter(&sc->sc_rwlock, RW_WRITER);
+       mutex_enter(&sc->sc_mutex);
        return 0;
 }

@@ -270,7 +270,7 @@ nfsmb_release_bus(void *cookie, int flag
 {
        struct nfsmb_softc *sc = cookie;

-       rw_exit(&sc->sc_rwlock);
+       mutex_exit(&sc->sc_mutex);
 }

 static int

Index: src/sys/dev/pci/piixpm.c
diff -u src/sys/dev/pci/piixpm.c:1.39 src/sys/dev/pci/piixpm.c:1.40
--- src/sys/dev/pci/piixpm.c:1.39       Mon Jan 30 19:41:22 2012
+++ src/sys/dev/pci/piixpm.c    Tue Feb 14 15:08:07 2012
@@ -1,4 +1,4 @@
-/* $NetBSD: piixpm.c,v 1.39 2012/01/30 19:41:22 drochner Exp $ */
+/* $NetBSD: piixpm.c,v 1.40 2012/02/14 15:08:07 pgoyette Exp $ */
 /*     $OpenBSD: piixpm.c,v 1.20 2006/02/27 08:25:02 grange Exp $      */

 /*
@@ -22,13 +22,13 @@
  */

 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: piixpm.c,v 1.39 2012/01/30 19:41:22 drochner Exp 
$");
+__KERNEL_RCSID(0, "$NetBSD: piixpm.c,v 1.40 2012/02/14 15:08:07 pgoyette Exp 
$");

 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/device.h>
 #include <sys/kernel.h>
-#include <sys/rwlock.h>
+#include <sys/mutex.h>
 #include <sys/proc.h>

 #include <sys/bus.h>
@@ -82,7 +82,7 @@ struct piixpm_softc {
        pcireg_t                sc_id;

        struct i2c_controller   sc_i2c_tag;
-       krwlock_t               sc_i2c_rwlock;
+       kmutex_t                sc_i2c_mutex;
        struct {
                i2c_op_t     op;
                void *      buf;
@@ -251,7 +251,7 @@ nopowermanagement:

 attach_i2c:
        /* Attach I2C bus */
-       rw_init(&sc->sc_i2c_rwlock);
+       mutex_init(&sc->sc_i2c_mutex, MUTEX_DEFAULT, IPL_NONE);
        sc->sc_i2c_tag.ic_cookie = sc;
        sc->sc_i2c_tag.ic_acquire_bus = piixpm_i2c_acquire_bus;
        sc->sc_i2c_tag.ic_release_bus = piixpm_i2c_release_bus;
@@ -331,6 +331,7 @@ piixpm_sb800_init(struct piixpm_softc *s
                aprint_error_dev(sc->sc_dev, "can't map smbus I/O space\n");
                return EBUSY;
        }
+       aprint_normal_dev(sc->sc_dev, "polling (SB800)\n");
        sc->sc_poll = 1;

        return 0;
@@ -362,10 +363,9 @@ piixpm_i2c_acquire_bus(void *cookie, int
 {
        struct piixpm_softc *sc = cookie;

-       if (cold || sc->sc_poll || (flags & I2C_F_POLL))
-               return (0);
+       if (!cold)
+               mutex_enter(&sc->sc_i2c_mutex);

-       rw_enter(&sc->sc_i2c_rwlock, RW_WRITER);
        return 0;
 }

@@ -374,10 +374,8 @@ piixpm_i2c_release_bus(void *cookie, int
 {
        struct piixpm_softc *sc = cookie;

-       if (cold || sc->sc_poll || (flags & I2C_F_POLL))
-               return;
-
-       rw_exit(&sc->sc_i2c_rwlock);
+       if (!cold)
+               mutex_exit(&sc->sc_i2c_mutex);
 }

 static int



--
 \~~~~~.                The NetBSD Foundation
  \~~~~~'               Marc Balmer, Developer / Marketing
 NetBSD
    \                   mbalmer%NetBSD.org@localhost   http://www.NetBSD.org/

!DSPAM:4f3a919c2063692315988!


-------------------------------------------------------------------------
| Paul Goyette     | PGP Key fingerprint:     | E-mail addresses:       |
| Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com    |
| Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net |
| Kernel Developer |                          | pgoyette at netbsd.org  |
-------------------------------------------------------------------------

References:
- Re: CVS commit: src/sys/dev/pci
  - From: Marc Balmer

Prev by Date: Re: CVS commit: src/sys/dev/pci
Next by Date: Re: CVS commit: src/sys/arch/arm/iomd
Previous by Thread: Re: CVS commit: src/sys/dev/pci
Indexes:

Home | Main Index | Thread Index | Old Index