Hey Christoph,
On Thu, Feb 09, 2012 at 05:56:26PM -0500, Christoph Hellwig wrote:
> 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.
Ah, I see.
> > > + 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.
That's a good point, but the shrinker interface has documented that
you're supposed to return -1 in this situation... and that you aren't
allowed to return -1 when nr_to_scan == 0.
> > > -
> > > - 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.
Sure... then it should be:
return atomic_read(&xfs_Gqm->qm_frlist_cnt);
What is the value of the additional calculation?
> > * 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.
No it isn't... if nr_to_scan == 0 we would have jumped to 'out' and
returned the count.
Thanks,
Ben
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
