Re: [PATCH SERIES] untangle spinlock macros

Eric Sandeen
2007-09-12T01:55:32Z

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