On 15.03.2018 18:53, Darrick J. Wong wrote:
> On Thu, Mar 15, 2018 at 06:01:34PM +0300, Kirill Tkhai wrote:
>> xfs_reclaim_inodes_count(XFS_M(sb)) does not care about memcg.
>> So, it's called for memcg reclaim too, e.g. this list is shrinked
>> disproportionality to another lists.
>>
>> This looks confusing, so I'm reporting about this.
>> Consider this patch as RFC.
>
> So... I think the reasoning here is that xfs doesn't allocate inodes on
> behalf of any particular memcg (or put another way the cost of the
> inodes is shared by everything in the system) so if the shrinkers get
> called because a particular memcg is bumping up against its limits it
> makes no sense to try to purge xfs inodes?
Yes, since shrinking xfs cached objects doesn't free memcg kmem.
> Followup questions: does the same reasoning apply to the xfs buffer and
> quota shrinkers?
It's not need, as these shrinker don't have SHRINKER_MEMCG_AWARE flag.
So, they are called only in case of global reclaim (memcg == NULL).
But they may require another type of change. They use list_lru,
so they may need to have SHRINKER_MEMCG_AWARE flag. This is depends
on how objects linked to xfs_buftarg::bt_lru and xfs_quotainfo::qi_lru
are allocated. If they are accounted to memcg (i.e., use GFP_ACCOUNT
flag in their kmalloc/slab alloc/etc), they have to have this flag.
Ideally, all user initiated allocations should be made with this flag.
If they already use GFP_ACCOUNT while shrinker is not marked as SHRINKER_MEMCG_AWARE,
it never shrinks objects related to memcgs.
> Do any filesystems associate their metadata memory
> allocations with a specific memcg? If not, why not put this in
> super_cache_{scan,count}?
They do, since generic super_cache_{scan,count} have a deal with sb's s_dentry_lru
and s_inode_lru, and use list lru, which supports memcg accounting.
> --D
>
>> Signed-off-by: Kirill Tkhai <ktkhai@xxxxxxxxxxxxx>
>> ---
>> fs/xfs/xfs_super.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
>> index 951271f57d00..124568aefa94 100644
>> --- a/fs/xfs/xfs_super.c
>> +++ b/fs/xfs/xfs_super.c
>> @@ -1788,6 +1788,8 @@ xfs_fs_nr_cached_objects(
>> struct super_block *sb,
>> struct shrink_control *sc)
>> {
>> + if (sc->memcg)
>> + return 0;
>> return xfs_reclaim_inodes_count(XFS_M(sb));
>> }
>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
