Subject: Re: kern/36673 (dubious code is sysmon_envsys)
To: None <xtraeme@NetBSD.org, gnats-admin@netbsd.org,>
From: YAMAMOTO Takashi <yamt@mwd.biglobe.ne.jp>
List: netbsd-bugs
Date: 07/29/2007 22:55:03
The following reply was made to PR kern/36673; it has been noted by GNATS.
From: yamt@mwd.biglobe.ne.jp (YAMAMOTO Takashi)
To: juan@xtrarom.org
Cc: gnats-bugs@NetBSD.org
Subject: Re: kern/36673 (dubious code is sysmon_envsys)
Date: Mon, 30 Jul 2007 07:53:19 +0900 (JST)
> On Wed, 25 Jul 2007 11:45:04 +0000 (UTC)
> yamt@mwd.biglobe.ne.jp (YAMAMOTO Takashi) wrote:
>
> > can you provide a diff with -p?
>
> Sure, here it is: (I didn't know about -p, it's very useful :-)
can you explain what you are trying to achieve with this change?
it seems like almost no-op me. (except that it seems to introduce
a double-unlock bug of sme_list_mtx on some errors.)
YAMAMOTO Takashi
> Index: sysmon_envsys.c
> ===================================================================
> RCS file: /cvsroot/src/sys/dev/sysmon/sysmon_envsys.c,v
> retrieving revision 1.43
> diff -b -u -p -r1.43 sysmon_envsys.c
> --- sysmon_envsys.c 23 Jul 2007 17:51:16 -0000 1.43
> +++ sysmon_envsys.c 25 Jul 2007 12:34:15 -0000
> @@ -483,7 +483,6 @@ sysmon_envsys_register(struct sysmon_env
> mutex_enter(&sme_list_mtx);
> LIST_FOREACH(lsme, &sysmon_envsys_list, sme_list) {
> if (strcmp(lsme->sme_name, sme->sme_name) == 0) {
> - mutex_exit(&sme_list_mtx);
> error = EEXIST;
> goto out;
> }
> @@ -496,16 +495,14 @@ sysmon_envsys_register(struct sysmon_env
> sme->sme_fsensor = sysmon_envsys_next_sensor_index;
> sysmon_envsys_next_sensor_index += sme->sme_nsensors;
> #endif
> - mutex_exit(&sme_list_mtx);
> error = sysmon_envsys_createplist(sme);
> if (!error) {
> - mutex_enter(&sme_list_mtx);
> LIST_INSERT_HEAD(&sysmon_envsys_list, sme, sme_list);
> sme_uniqsensors = 0;
> - mutex_exit(&sme_list_mtx);
> }
>
> out:
> + mutex_exit(&sme_list_mtx);
> return error;
> }
>
> @@ -626,6 +623,9 @@ sysmon_envsys_createplist(struct sysmon_
>
> nsens = error = 0;
>
> + KASSERT(mutex_owned(&sme_list_mtx));
> + mutex_exit(&sme_list_mtx);
> +
> /* create the sysmon envsys device array. */
> array = prop_array_create();
> if (array == NULL)
> @@ -688,6 +688,7 @@ sysmon_envsys_createplist(struct sysmon_
>
>
> prop_object_release(array);
> + mutex_enter(&sme_list_mtx);
> return error;
> }
>
>
> --
> Juan Romero Pardines - The NetBSD Project
> http://plog.xtrarom.org - NetBSD/pkgsrc news in Spanish