xfs
[Top] [All Lists]

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

To: Dave Chinner <david@xxxxxxxxxxxxx>
Subject: Re: [PATCH 16/18] xfs: serialise inode reclaim within an AG
From: Christoph Hellwig <hch@xxxxxxxxxxxxx>
Date: Sat, 25 Sep 2010 19:49:29 -0400
Cc: xfs@xxxxxxxxxxx
In-reply-to: <1285331476-23015-17-git-send-email-david@xxxxxxxxxxxxx>
References: <1285331476-23015-1-git-send-email-david@xxxxxxxxxxxxx> <1285331476-23015-17-git-send-email-david@xxxxxxxxxxxxx>
User-agent: Mutt/1.5.20 (2009-08-17)
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.

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