* Nathan Scott <nathans@xxxxxxx> wrote:
> > 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).
the main variation in xfs_iget() is whether we lock the inode
read-write, read-only or not at all, correct? (XFS_ILOCK_EXCL,
XFS_ILOCK_SHARED and 0)
That could be cleaned up the following way:
- rename the current xfs_iget() to __xfs_iget() and remove ilock locking
from it.
- add 3 new APIs: xfs_iget_read(), xfs_iget_write() and
xfs_iget_nolock():
- xfs_iget_read() just calls __xfs_iget() and does a down_read() if
the inode was looked up successfully.
- xfs_iget_write() does the same but with down_write()
- xfs_iget_nolock() is just an alias to __xfs_iget().
- update all 13 uses of xfs_iget() to one of the 3 API variants
- [ there might be other details i missed, but this seems to be the
rough list of things to do. ]
NOTE: since the majority (9 out of 13) of xfs_iget() uses are for the
'no lock' variant, this construction of functions, besides making the
code more readable, _further_ reduces overhead, because there is no
ilock-flags checking overhead in __xfs_iget() anymore.
Ingo
-
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
