Bug 472040

Summary:

GFS2: [RFE] Change gfs2_quota_scan() into a shrinker

Product:

[Fedora] Fedora

Reporter:

Steve Whitehouse <swhiteho>

Component:

GFS-kernel

Assignee:

Abhijith Das <adas>

Status:

CLOSED UPSTREAM

QA Contact:

Severity:

medium

Docs Contact:

Priority:

medium

Version:

rawhide

CC:

bmarzins, rpeterso

Target Milestone:

---

Keywords:

FutureFeature

Target Release:

---

Hardware:

All

OS:

Linux

Whiteboard:

Fixed In Version:

Doc Type:

Enhancement

Doc Text:

Story Points:

---

Clone Of:

Environment:

Last Closed:

2009-03-27 10:40:14 UTC

Type:

---

Regression:

---

Mount Type:

---

Documentation:

---

CRM:

Verified Versions:

Category:

---

oVirt Team:

---

RHEL 7.3 requirements from Atomic Host:

Cloudforms Team:

---

Target Upstream Version:

Embargoed:

Bug Depends On:

Bug Blocks:

519832

Attachments:

Description	Flags
Patch to bring back some of lock_nolock's lvb-related functionality to make broken quotas work	none
gfs2_quota_data shrinker first attempt patch	none
removes printk from earlier lock_nolock lvb-related patch	none
3 changes from previous shrinker patch	none
one change from previous patch in gfs2_quota_cleanup to avoid deadlock	none
Tried to reduce double locking	none
Missed out a lock in the previous patch. Fixed here	none

Description Steve Whitehouse 2008-11-18 13:21:13 UTC

"Exactly what it says in the summary"... it looks like we should have an LRU list of freeable struct gfs2_quota_data elements, together with a shrinker which can manage the cached data.

That should allow us to remove the call to gfs2_quota_scan from gfs2_quotad and also to be more responsive to memory pressure.

This bug is just a reminder that we need to do that at some future stage.

Comment 1 Abhijith Das 2008-12-18 16:50:48 UTC

Created attachment 327338 [details]
Patch to bring back some of lock_nolock's lvb-related functionality to make broken quotas work

Comment 2 Abhijith Das 2008-12-18 17:12:53 UTC

argh, I missed out a stray printk in the previous patch.

Comment 3 Abhijith Das 2008-12-18 17:23:16 UTC

Created attachment 327342 [details]
gfs2_quota_data shrinker first attempt patch

adds reclaimable quota data to another list managed by a shrinker. This patch has not been tested yet.

Comment 4 Abhijith Das 2008-12-18 23:13:53 UTC

Created attachment 327388 [details]
removes printk from earlier lock_nolock lvb-related patch

Comment 5 Abhijith Das 2008-12-18 23:23:28 UTC

Created attachment 327389 [details]
3 changes from previous shrinker patch

1. added a line to atomic_dec(&sdp->sd_quota_count); in gfs2_shrink_qd_memory() that was missing before when we freed a qd from both the reclaim list and the fs-specific sd_quota_list.

2. changed "(atomic_read(&qd_lru_count) / 100) * sysctl_vfs_cache_pressure" to "(atomic_read(&qd_lru_count) * sysctl_vfs_cache_pressure) / 100" to get qds if qd_lru_count < 100;

3. Added SetPageUptodate(page); to stuffed_readpage() when returning a zero-filled page when index > 0; (This fixes the -EIO, I was seeing earlier; not sure if this is the right thing to do though)

Comment 6 Steve Whitehouse 2008-12-19 10:02:17 UTC

Looks qood. One question though, are we using sdp->sd_quota_spin on its own in enough places to justify keeping it? If we just used qd_lru_lock on its own, then it would be possible to reduce qd_put to something like:

if (atomic_dec_and_lock()) {
        list_add();
        atomic_inc();
        spin_unlock();
}

and it would also simplify the other places in the code where we've now got double locking too. I don't think there should be any other issues wrt stuffed_readpage as I think all the other uses would sequentially read a file, stopping at EOF so it shouldn't be an issue.

Comment 7 Abhijith Das 2008-12-19 10:29:37 UTC

Created attachment 327425 [details]
one change from previous patch in gfs2_quota_cleanup to avoid deadlock

Comment 8 Abhijith Das 2009-01-07 17:47:58 UTC

Created attachment 328402 [details]
Tried to reduce double locking

This patch uses the lru_lock for both quota lists and sd_quota_spin for internal quota fields, bitmap and such. It reduces a bit of double locking.

Comment 9 Abhijith Das 2009-01-07 18:47:12 UTC

Created attachment 328404 [details]
Missed out a lock in the previous patch. Fixed here

Comment 10 Steve Whitehouse 2009-01-14 14:00:41 UTC

This is now upstream in the -nmw git tree.

Comment 11 Steve Whitehouse 2009-03-27 10:40:14 UTC

In 2.6.30