xfs
[Top] [All Lists]

Re: [PATCH SERIES] untangle spinlock macros

To: Donald Douwsma <donaldd@xxxxxxx>
Subject: Re: [PATCH SERIES] untangle spinlock macros
From: Eric Sandeen <sandeen@xxxxxxxxxxx>
Date: Tue, 11 Sep 2007 20:55:29 -0500
Cc: xfs-oss <xfs@xxxxxxxxxxx>
In-reply-to: <46E7460D.3000502@xxxxxxx>
References: <46E6221E.803@xxxxxxxxxxx> <46E7460D.3000502@xxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Thunderbird 2.0.0.6 (Macintosh/20070728)
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


<Prev in Thread] Current Thread [Next in Thread>