On Thu, Feb 09, 2012 at 04:03:20PM -0600, Ben Myers wrote:
> I've been messing with this and haven't gotten it to call us with
> nr_to_scan other than 0 or -1 yet. Maybe I need more dquots.
> (time passes) Ok, I have it going now. Comments below.
To actually hit this I hade to use a VM with very little memory assigned
to it, and then creat lots of dquots and causes memory pressure.
I have about 20.000 users on it, and I did a quota report for all of
them while catting one block device into another using buffered I/O.
>
> > This also fixes an bug in the previous lock ordering, where we would take
> > the hash and dqlist locks inside of the freelist lock against the normal
> > lock ordering. This is only solvable by introducing the dispose list,
> > and thus not when using direct reclaim of unused dquots for new allocations.
>
> FWICS this fixes a possible deadlock, xfs_qm_dqget vs xfs_qm_dqreclaim
> one.
Yes.
> > + LIST_HEAD (dispose_list);
> > + struct xfs_dquot *dqp;
> >
> > - if (nfree <= ndqused && nfree < ndquot)
> > + if ((sc->gfp_mask & (__GFP_FS|__GFP_WAIT)) != (__GFP_FS|__GFP_WAIT))
> > return 0;
> > + if (!nr_to_scan)
> > + goto out;
>
> I suggest something more like:
>
> if (!nr_to_scan)
> goto out;
> if ((sc->gfp_mask...
> return -1;
Why? Counting the number of objects when we can't actually do anything
is just a waste of time, and -1 vs 0 for the sizing pass seem to be
treateds the same in the calling code.
> > -
> > - return B_TRUE;
> > + while (!list_empty(&dispose_list)) {
> > + dqp = list_first_entry(&dispose_list, struct xfs_dquot,
> > + q_freelist);
> > + list_del_init(&dqp->q_freelist);
> > + xfs_qm_dqfree_one(dqp);
> > + }
> > +out:
> > + return (xfs_Gqm->qm_dqfrlist_cnt / 100) * sysctl_vfs_cache_pressure;
>
> return atomic_read(&xfs_Gqm->qm_totaldquots);
>
> This works well for me and seems to be closer to the shrinker interface
> as documented:
It's pointless - we can only apply pressure to dquots that are on the
freelist. No amount of shaking will allow us to reclaim a referenced
dquot.
> * The callback must not return -1 if nr_to_scan is zero.
this is against your suggestion of using -1 for the estimation pass
above, btw.
|