From owner-freebsd-ia64@FreeBSD.ORG Tue Feb 3 16:08:08 2009 Return-Path: Delivered-To: ia64@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3861A10656BA for ; Tue, 3 Feb 2009 16:08:08 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id F2B828FC0C for ; Tue, 3 Feb 2009 16:08:07 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from server.baldwin.cx (pool-98-109-39-197.nwrknj.fios.verizon.net [98.109.39.197]) by cyrus.watson.org (Postfix) with ESMTPSA id 81FC346B0D; Tue, 3 Feb 2009 11:08:07 -0500 (EST) Received: from localhost (john@localhost [127.0.0.1]) (authenticated bits=0) by server.baldwin.cx (8.14.3/8.14.3) with ESMTP id n13G7thS050843; Tue, 3 Feb 2009 11:08:01 -0500 (EST) (envelope-from jhb@freebsd.org) From: John Baldwin To: Marcel Moolenaar Date: Tue, 3 Feb 2009 11:07:52 -0500 User-Agent: KMail/1.9.7 References: <200902021344.19249.jhb@freebsd.org> <5396BD91-541F-4EDF-9034-106477C99B7F@mac.com> In-Reply-To: <5396BD91-541F-4EDF-9034-106477C99B7F@mac.com> MIME-Version: 1.0 Content-Disposition: inline Message-Id: <200902031107.52507.jhb@freebsd.org> Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit X-Greylist: Sender succeeded SMTP AUTH authentication, not delayed by milter-greylist-2.0.2 (server.baldwin.cx [127.0.0.1]); Tue, 03 Feb 2009 11:08:01 -0500 (EST) X-Virus-Scanned: ClamAV 0.94.2/8946/Tue Feb 3 07:32:04 2009 on server.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=4.2 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.1.3 X-Spam-Checker-Version: SpamAssassin 3.1.3 (2006-06-01) on server.baldwin.cx Cc: ia64@freebsd.org Subject: Re: Change ia64 mca sysctls to use standard dynamic sysctls X-BeenThere: freebsd-ia64@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Porting FreeBSD to the IA-64 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 03 Feb 2009 16:08:08 -0000 On Monday 02 February 2009 7:59:11 pm Marcel Moolenaar wrote: > > On Feb 2, 2009, at 10:44 AM, John Baldwin wrote: > > > While working on the locking for the sysctl tree, I ran into the > > pseudo-dynamic machine check sysctls in ia64. It is not really a > > good idea > > to call sysctl_register_oid() with a spin lock held and once I add > > locking to > > sysctl it becomes a bad LOR. This changes the code to use the > > public API for > > adding dynamic sysctls outside of the spin lock. Please test this > > as it is a > > prerequisite for finishing the locking of the sysctl tree. > > This won't be a solution, because eventually this code gets called > for machine checks, which means that locks can be held (which is > then the least of our worries :-) > > In any case: malloc(M_WAITOK) cannot be used. We should probably > delay updating the sysctl tree and do it in a more controlled > environment. If you can implement a simple linked list to store > the saved state and remove the call to SYSCTL_ADD_OID, then I'll > work on that part after you're done. Does that sound like a plan? Ok. Checking for failures from M_NOWAIT is important, too. :) Here's an updated patch that uses the queue. I haven't implemented the stuff to schedule calls to the populate routine to drain the queue though. Would you be able to do that? --- //depot/projects/smpng/sys/ia64/ia64/mca.c 2007/05/19 15:31:43 +++ //depot/user/jhb/lock/ia64/ia64/mca.c 2009/02/03 16:04:36 @@ -42,6 +42,16 @@ MALLOC_DEFINE(M_MCA, "MCA", "Machine Check Architecture"); +struct mca_info { + STAILQ_ENTRY(mca_info) mi_link; + char mi_name[32]; + size_t mi_recsz; + char mi_record[0]; +}; + +static STAILQ_HEAD(, mca_info) mca_records = + STAILQ_HEAD_INITIALIZER(mca_records); + int64_t mca_info_size[SAL_INFO_TYPES]; vm_offset_t mca_info_block; struct mtx mca_info_block_lock; @@ -76,14 +86,31 @@ } void +ia64_mca_populate(void) +{ + struct mca_info *rec; + + mtx_lock_spin(&mca_info_block_lock); + while (!STAILQ_EMPTY(&mca_records)) { + rec = STAILQ_FIRST(&mca_records); + STAILQ_REMOVE_HEAD(&mca_records, mi_link); + mtx_unlock_spin(&mca_info_block_lock); + (void)SYSCTL_ADD_PROC(NULL, SYSCTL_STATIC_CHILDREN(_hw_mca), + OID_AUTO, name, CTLTYPE_OPAQUE | CTLFLAG_RD, rec->mi_record, + rec->mi_recsz, mca_sysctl_handler, "S,MCA", "Error record"); + mtx_lock_spin(&mca_info_block_lock); + } + mtx_unlock_spin(&mca_info_block_lock); +} + +void ia64_mca_save_state(int type) { struct ia64_sal_result result; struct mca_record_header *hdr; - struct sysctl_oid *oidp; - char *name, *state; + struct mca_info *rec; uint64_t seqnr; - size_t recsz, totsz; + size_t recsz; /* * Don't try to get the state if we couldn't get the size of @@ -95,9 +122,8 @@ if (mca_info_block == 0) return; + mtx_lock_spin(&mca_info_block_lock); while (1) { - mtx_lock_spin(&mca_info_block_lock); - result = ia64_sal_entry(SAL_GET_STATE_INFO, type, 0, mca_info_block, 0, 0, 0, 0); if (result.sal_status < 0) { @@ -111,11 +137,12 @@ mtx_unlock_spin(&mca_info_block_lock); - totsz = sizeof(struct sysctl_oid) + recsz + 32; - oidp = malloc(totsz, M_MCA, M_NOWAIT|M_ZERO); - state = (char*)(oidp + 1); - name = state + recsz; - sprintf(name, "%lld", (long long)seqnr); + rec = malloc(sizeof(mca_info) + recsz, M_MCA, M_NOWAIT | M_ZERO); + if (rec == NULL) + /* XXX: Not sure what to do. */ + return; + + sprintf(rec->mi_name, "%lld", (long long)seqnr); mtx_lock_spin(&mca_info_block_lock); @@ -133,25 +160,15 @@ mca_info_block, 0, 0, 0, 0); if (seqnr != hdr->rh_seqnr) { mtx_unlock_spin(&mca_info_block_lock); - free(oidp, M_MCA); + free(rec, M_MCA); + mtx_lock_spin(&mca_info_block_lock); continue; } } - bcopy((char*)mca_info_block, state, recsz); + rec->mi_recsz = recsz; + bcopy((char*)mca_info_block, rec->mi_record, recsz); - oidp->oid_parent = &sysctl__hw_mca_children; - oidp->oid_number = OID_AUTO; - oidp->oid_kind = CTLTYPE_OPAQUE|CTLFLAG_RD|CTLFLAG_DYN; - oidp->oid_arg1 = state; - oidp->oid_arg2 = recsz; - oidp->oid_name = name; - oidp->oid_handler = mca_sysctl_handler; - oidp->oid_fmt = "S,MCA"; - oidp->oid_descr = "Error record"; - - sysctl_register_oid(oidp); - if (mca_count > 0) { if (seqnr < mca_first) mca_first = seqnr; @@ -161,6 +178,7 @@ mca_first = mca_last = seqnr; mca_count++; + STAILQ_INSERT_TAIL(&mca_records, rec, mi_link); /* * Clear the state so that we get any other records when @@ -168,8 +186,6 @@ */ result = ia64_sal_entry(SAL_CLEAR_STATE_INFO, type, 0, 0, 0, 0, 0, 0); - - mtx_unlock_spin(&mca_info_block_lock); } } -- John Baldwin