Re: CVS commit: src/sys/dev/i2c
You forgot to make the iic driver detachable, modunload iic will panic I
think.
-Original Message-
From: Marc Balmer
Sent: Sunday, October 02, 2011 7:38 AM Newsgroups:
gmane.os.netbsd.devel.cvs.full
To: source-changes-full-qavaossjccednm+yrof...@public.gmane.org
Subject: CVS commit: src/sys/dev/i2c
Module Name: src
Committed By: mbalmer
Date: Sun Oct 2 11:38:48 UTC 2011
Modified Files:
src/sys/dev/i2c: i2c.c
Log Message:
ii2c can be built as module.
To generate a diff of this commit:
cvs rdiff -u -r1.27 -r1.28 src/sys/dev/i2c/i2c.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/i2c/i2c.c
diff -u src/sys/dev/i2c/i2c.c:1.27 src/sys/dev/i2c/i2c.c:1.28
--- src/sys/dev/i2c/i2c.c:1.27 Tue Aug 2 18:46:35 2011
+++ src/sys/dev/i2c/i2c.c Sun Oct 2 11:38:48 2011
@@ -1,4 +1,4 @@
-/* $NetBSD: i2c.c,v 1.27 2011/08/02 18:46:35 pgoyette Exp $ */
+/* $NetBSD: i2c.c,v 1.28 2011/10/02 11:38:48 mbalmer Exp $ */
/*
* Copyright (c) 2003 Wasabi Systems, Inc.
@@ -36,7 +36,7 @@
*/
#include
-__KERNEL_RCSID(0, "$NetBSD: i2c.c,v 1.27 2011/08/02 18:46:35 pgoyette Exp
$");
+__KERNEL_RCSID(0, "$NetBSD: i2c.c,v 1.28 2011/10/02 11:38:48 mbalmer Exp
$");
#include
#include
@@ -47,6 +47,7 @@ __KERNEL_RCSID(0, "$NetBSD: i2c.c,v 1.27
#include
#include
#include
+#include
#include
@@ -116,7 +117,7 @@ iic_search(device_t parent, cfdata_t cf,
ia.ia_compat = NULL;
if (config_match(parent, cf, &ia) > 0) {
- if (ia.ia_addr == (i2c_addr_t)-1)
+ if (ia.ia_addr == (i2c_addr_t)-1)
config_attach(parent, cf, &ia, iic_print);
else if (ia.ia_addr <= I2C_MAX_ADDR &&
!sc->sc_devices[ia.ia_addr])
@@ -341,7 +342,7 @@ iic_smbus_intr_establish(i2c_tag_t ic, i
il = malloc(sizeof(struct ic_intr_list), M_DEVBUF, M_WAITOK);
if (il == NULL)
return NULL;
-
+
il->il_intr = intr;
il->il_intrarg = intrarg;
@@ -371,7 +372,7 @@ iic_smbus_intr_establish_proc(i2c_tag_t
il = malloc(sizeof(struct ic_intr_list), M_DEVBUF, M_WAITOK);
if (il == NULL)
return NULL;
-
+
il->il_intr = intr;
il->il_intrarg = intrarg;
@@ -460,3 +461,37 @@ iic_compat_match(struct i2c_attach_args
CFATTACH_DECL2_NEW(iic, sizeof(struct iic_softc),
iic_match, iic_attach, NULL, NULL, iic_rescan, iic_child_detach);
+
+MODULE(MODULE_CLASS_DRIVER, iic, NULL);
+
+#ifdef _MODULE
+#include "ioconf.c"
+#endif
+
+static int
+iic_modcmd(modcmd_t cmd, void *opaque)
+{
+ int error;
+
+ error = 0;
+ switch (cmd) {
+ case MODULE_CMD_INIT:
+#ifdef _MODULE
+ error = config_init_component(cfdriver_ioconf_iic,
+ cfattach_ioconf_iic, cfdata_ioconf_iic);
+ if (error)
+ aprint_error("%s: unable to init component\n",
+ iic_cd.cd_name);
+#endif
+ break;
+ case MODULE_CMD_FINI:
+#ifdef _MODULE
+ config_fini_component(cfdriver_ioconf_iic,
+ cfattach_ioconf_iic, cfdata_ioconf_iic);
+#endif
+ break;
+ default:
+ error = ENOTTY;
+ }
+ return error;
+}
Re: CVS commit: src/sys/kern
On Oct 2, 2011, at 3:26 PM, Adam Hamsik wrote: > > On Oct,Sunday 2 2011, at 3:00 PM, Juergen Hannken-Illjes wrote: > >> Module Name: src >> Committed By:hannken >> Date:Sun Oct 2 13:00:07 UTC 2011 >> >> Modified Files: >> src/sys/kern: vfs_vnode.c >> >> Log Message: >> The path getnewvnode()->getcleanvnode()->vclean()->VOP_LOCK() will panic >> if the vnode we want to clean is a layered vnode and the caller already >> locked its lower vnode. >> >> Change getnewvnode() to always allocate a fresh vnode and add a helper >> thread (vdrain) to keep the number of allocated vnodes within desiredvnodes. >> >> Rename getcleanvnode() to cleanvnode() and let it take a vnode from the >> lists, clean and free it. > > Thanks for doing this. This should help zfs too I saw couple of > panics with zfs which were caused by this. > > Have you done any benchmarks ? when I proposed solution like this > main objection was then before vnodes were not allocated from storage > pool every time(sometime we reused already allocated one). And this may > affect performance. I didn't run any benchmark. As getnewvnode() gets called when a file system sets up a new (not yet cached) vnode I suppose there are other operations taking much more time like reading an inode from disk, setting up the inode, creating the genfs_node etc. We also save the time spent in cleaning the vnode which is now done async to the allocation. >> >> Reviewed by: David Holland >> >> Should fix: >> PR #19110 (nullfs mounts over NFS cause lock manager problems) >> PR #34102 (ffs panic in NetBSD 3.0_STABLE) >> PR #45115 (lock error panic when build.sh*3 and daily script is running) >> PR #45355 (Reader/writer lock error: rw_vector_enter: locking against >> myself) > > Regards > > Adam. > -- Juergen Hannken-Illjes - hann...@eis.cs.tu-bs.de - TU Braunschweig (Germany)
Re: CVS commit: src/sys/kern
On Oct,Sunday 2 2011, at 3:00 PM, Juergen Hannken-Illjes wrote:
> Module Name: src
> Committed By: hannken
> Date: Sun Oct 2 13:00:07 UTC 2011
>
> Modified Files:
> src/sys/kern: vfs_vnode.c
>
> Log Message:
> The path getnewvnode()->getcleanvnode()->vclean()->VOP_LOCK() will panic
> if the vnode we want to clean is a layered vnode and the caller already
> locked its lower vnode.
>
> Change getnewvnode() to always allocate a fresh vnode and add a helper
> thread (vdrain) to keep the number of allocated vnodes within desiredvnodes.
>
> Rename getcleanvnode() to cleanvnode() and let it take a vnode from the
> lists, clean and free it.
Thanks for doing this. This should help zfs too I saw couple of
panics with zfs which were caused by this.
Have you done any benchmarks ? when I proposed solution like this
main objection was then before vnodes were not allocated from storage
pool every time(sometime we reused already allocated one). And this may
affect performance.
>
> Reviewed by: David Holland
>
> Should fix:
> PR #19110 (nullfs mounts over NFS cause lock manager problems)
> PR #34102 (ffs panic in NetBSD 3.0_STABLE)
> PR #45115 (lock error panic when build.sh*3 and daily script is running)
> PR #45355 (Reader/writer lock error: rw_vector_enter: locking against myself)
>
>
> To generate a diff of this commit:
> cvs rdiff -u -r1.11 -r1.12 src/sys/kern/vfs_vnode.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/kern/vfs_vnode.c
> diff -u src/sys/kern/vfs_vnode.c:1.11 src/sys/kern/vfs_vnode.c:1.12
> --- src/sys/kern/vfs_vnode.c:1.11 Thu Sep 29 20:51:38 2011
> +++ src/sys/kern/vfs_vnode.c Sun Oct 2 13:00:06 2011
> @@ -1,4 +1,4 @@
> -/* $NetBSD: vfs_vnode.c,v 1.11 2011/09/29 20:51:38 christos Exp $ */
> +/* $NetBSD: vfs_vnode.c,v 1.12 2011/10/02 13:00:06 hannken Exp $ */
>
> /*-
> * Copyright (c) 1997-2011 The NetBSD Foundation, Inc.
> @@ -76,7 +76,6 @@
> *starts in one of the following ways:
> *
> *- Allocation, via getnewvnode(9) and/or vnalloc(9).
> - * - Recycle from a free list, via getnewvnode(9) -> getcleanvnode(9).
> *- Reclamation of inactive vnode, via vget(9).
> *
> *The life-cycle ends when the last reference is dropped, usually
> @@ -121,7 +120,7 @@
> */
>
> #include
> -__KERNEL_RCSID(0, "$NetBSD: vfs_vnode.c,v 1.11 2011/09/29 20:51:38 christos
> Exp $");
> +__KERNEL_RCSID(0, "$NetBSD: vfs_vnode.c,v 1.12 2011/10/02 13:00:06 hannken
> Exp $");
>
> #include
> #include
> @@ -159,8 +158,10 @@ static kcondvar_tvrele_cv
> __cacheline_
> static lwp_t *vrele_lwp __cacheline_aligned;
> static intvrele_pending __cacheline_aligned;
> static intvrele_gen __cacheline_aligned;
> +static kcondvar_tvdrain_cv __cacheline_aligned;
>
> -static vnode_t * getcleanvnode(void);
> +static int cleanvnode(void);
> +static void vdrain_thread(void *);
> static void vrele_thread(void *);
> static void vnpanic(vnode_t *, const char *, ...)
> __attribute__((__format__(__printf__, 2, 3)));
> @@ -183,7 +184,11 @@ vfs_vnode_sysinit(void)
> TAILQ_INIT(&vrele_list);
>
> mutex_init(&vrele_lock, MUTEX_DEFAULT, IPL_NONE);
> + cv_init(&vdrain_cv, "vdrain");
> cv_init(&vrele_cv, "vrele");
> + error = kthread_create(PRI_VM, KTHREAD_MPSAFE, NULL, vdrain_thread,
> + NULL, NULL, "vdrain");
> + KASSERT(error == 0);
> error = kthread_create(PRI_VM, KTHREAD_MPSAFE, NULL, vrele_thread,
> NULL, &vrele_lwp, "vrele");
> KASSERT(error == 0);
> @@ -249,13 +254,12 @@ vnfree(vnode_t *vp)
> }
>
> /*
> - * getcleanvnode: grab a vnode from freelist and clean it.
> + * cleanvnode: grab a vnode from freelist, clean and free it.
> *
> * => Releases vnode_free_list_lock.
> - * => Returns referenced vnode on success.
> */
> -static vnode_t *
> -getcleanvnode(void)
> +static int
> +cleanvnode(void)
> {
> vnode_t *vp;
> vnodelst_t *listhd;
> @@ -288,7 +292,7 @@ try_nextlist:
> goto try_nextlist;
> }
> mutex_exit(&vnode_free_list_lock);
> - return NULL;
> + return EBUSY;
> }
>
> /* Remove it from the freelist. */
> @@ -300,7 +304,7 @@ try_nextlist:
>
> /*
>* The vnode is still associated with a file system, so we must
> - * clean it out before reusing it. We need to add a reference
> + * clean it out before freeing it. We need to add a reference
>* before doing this. If the vnode gains another reference while
>* being cleaned out then we lose - retry.
>*/
> @@ -308,15 +312,7 @@ try_nextlist:
> vclean(vp, DOCLOSE);
> KASSERT(vp->v_usecount >= 1 + VC_XLOCK);
> atomic_add_int(&vp->v_usec
