From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: with ECARTIS (v1.0.0; list xfs); Tue, 11 Sep 2007 18:55:32 -0700 (PDT) Received: from sandeen.net (sandeen.net [209.173.210.139]) by oss.sgi.com (8.12.10/8.12.10/SuSE Linux 0.7) with ESMTP id l8C1tS4p001307 for ; Tue, 11 Sep 2007 18:55:29 -0700 Message-ID: <46E74711.5050108@sandeen.net> Date: Tue, 11 Sep 2007 20:55:29 -0500 From: Eric Sandeen MIME-Version: 1.0 Subject: Re: [PATCH SERIES] untangle spinlock macros References: <46E6221E.803@sandeen.net> <46E7460D.3000502@sgi.com> In-Reply-To: <46E7460D.3000502@sgi.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: xfs-bounce@oss.sgi.com Errors-to: xfs-bounce@oss.sgi.com List-Id: xfs To: Donald Douwsma Cc: xfs-oss Donald Douwsma wrote: > Hi Eric, > >> unwrap_AIL_LOCK > Here you change the comment to use the descriptive name > > - * We must not be holding the AIL_LOCK at this point. Calling incore() to > - * search the buffer cache can be a time consuming thing, and AIL_LOCK is a > + * We must not be holding the AIL lock at this point. Calling incore() to > + * search the buffer cache can be a time consuming thing, and AIL lock is a > * spinlock. > */ > >> unwrap_LOG_LOCK >> unwrap_GRANT_LOCK >> unwrap_XFS_DQ_PINUNLOCK >> unwrap_pagb_lock >> unwrap_xfs_dabuf_global_lock >> unwrap_mru_lock >> unwrap_XFS_SB_LOCK > But here you use the name of the lock variable. > > /* > - * We actually don't have to acquire the SB_LOCK at all. > + * We actually don't have to acquire the m_sb_lock at all. > * This can only be called from mount, and that's single threaded. XXX > */ Hm yup. 2 different evenings, oops. ;-) Have a preference? >> no_kt_lock >> cleanup_lock_goop >> >> Patches have comments/descriptions/signed-off lines in them. >> >> By the end of the series, spin.h is almost empty, only spin_lock_init / >> spinlock_destroy are left, and could maybe even be pulled out.... wasn't >> sure how far to go. Since the kernel has a mutex_destroy, I wonder if >> spinlocks will ever get similar treatment... anyway.... > So the only things left in spin.h are the spinlock headers and > > #define spinlock_init(lock, name) spin_lock_init(lock) > #define spinlock_destroy(lock) > > I cant se why we need these abstractions. Should we nuke the whole file and > add the spinlock headers elsewhere? We don't, really. It'd be easy to nuke the file and add them to xfs_linux.h. I'll send a patch to do so if you like. -Eric