Re: [PATCH 16/18] xfs: serialise inode reclaim within an AG

Christoph Hellwig
2010-09-25T23:49:02Z

I really don't like the way the "trylock" variable is overloaded here.
Just add a new skipped variable for restarting the scan and otherwise
use (flags & SYNC_TRYLOCK) directly.

> + int trylock = !!(flags & SYNC_TRYLOCK);
>
> +restart:
> ag = 0;
> while ((pag = xfs_perag_get_tag(mp, ag, XFS_ICI_RECLAIM_TAG))) {
> unsigned long first_index = 0;
> @@ -837,6 +839,17 @@ xfs_reclaim_inodes_ag(
>
> ag = pag->pag_agno + 1;
>
> + if (!mutex_trylock(&pag->pag_ici_reclaim_lock)) {
> + if (trylock) {
> + trylock++;
> + continue;
> + }
> + mutex_lock(&pag->pag_ici_reclaim_lock);
> + }
> +
> + if (trylock)
> + first_index = pag->pag_ici_reclaim_cursor;

Also this could be made more clear by:

if (flags & SYNC_TRYLOCK) {
if (!mutex_trylock(&pag->pag_ici_reclaim_lock)) {
skipped++;
continue;
}

first_index = pag->pag_ici_reclaim_cursor;
} else {
mutex_lock(&pag->pag_ici_reclaim_lock);
}

> +
> do {
> struct xfs_inode *batch[XFS_LOOKUP_BATCH];
> int i;
> @@ -889,8 +902,19 @@ xfs_reclaim_inodes_ag(
>
> } while (nr_found && !done && *nr_to_scan > 0);
>
> + pag->pag_ici_reclaim_cursor = (done || !trylock) ? 0 : first_index;

if ((flags & SYNC_TRYLOCK) && !done)
pag->pag_ici_reclaim_cursor = first_index;
else
pag->pag_ici_reclaim_cursor = 0;

> + /*
> + * if we skipped any AG, and we still have scan count remaining, do
> + * another pass this time waiting on the reclaim locks.
> + */
> + if (trylock > 1 && *nr_to_scan) {
> + trylock = 0;
> + goto restart;
> + }

In addition to waiting on the lock this also ignores the reclaim cursor.