On Tue, Jul 04, 2006 at 10:41:43AM +0200, Ingo Molnar wrote:
>
> * Ingo Molnar <mingo@xxxxxxx> wrote:
>
> the patch below should solve the case mentioned above, but i'm sure
> there will be other cases as well.
Thanks Ingo! I'll run with this for awhile and see if I can find
any other issues.
> XFS locking seems quite complex all around. All this dynamic
Mhmm.
> flag-passing into an opaque function (such as xfs_ilock), just to have
> them untangled in xfs_ilock():
>
> if (lock_flags & XFS_IOLOCK_EXCL) {
> mrupdate(&ip->i_iolock);
> } else if (lock_flags & XFS_IOLOCK_SHARED) {
> mraccess(&ip->i_iolock);
> }
> if (lock_flags & XFS_ILOCK_EXCL) {
> mrupdate(&ip->i_lock);
> } else if (lock_flags & XFS_ILOCK_SHARED) {
> mraccess(&ip->i_lock);
> }
>
> is pretty inefficient too - there are 85 calls to xfs_ilock(), and 74 of
> them have static flags.
Right... but that leaves plenty that don't, and they're not simple
to change. There are generic routines that need to be called from
different contexts with different locking requirements (xfs_iget).
> while xfs_iunlock() knows whether the release is for read or for write!
> So ->mr_writer seems like a totally unnecessary layer. In fact basically
Hmm, I did look into removing that sometime back, but didn't get
there for some reason that escapes me now... (I have a vague memory
of the use of downgrade_write messing me up there). I'll take
another look when I get some time, it would be beneficial to remove
that if we possibly can.
> i'd really suggest to clean this up and to convert:
> xfs_ilock(ip, XFS_ILOCK_EXCL);
> to:
> down_write(&ip->i_lock);
> and:
> xfs_iunlock(ip, XFS_ILOCK_EXCL);
> to:
> up_write(&ip->i_lock);
> and eliminate all those layers of indirection.
That would be good, but it doesn't work for all situations
unfortunately, and it would loose that debug-kernel sanity
checking that we have in there which validates ilock/iolock
ordering rules.
cheers.
--
Nathan
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
