From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from cuda.sgi.com (cuda2.sgi.com [192.48.176.25]) by oss.sgi.com (8.14.3/8.14.3/SuSE Linux 0.8) with ESMTP id o4QGd1ej087469 for ; Wed, 26 May 2010 11:39:01 -0500 Received: from mx2.suse.de (localhost [127.0.0.1]) by cuda.sgi.com (Spam Firewall) with ESMTP id 43C9C3782A2 for ; Wed, 26 May 2010 09:41:23 -0700 (PDT) Received: from mx2.suse.de (cantor2.suse.de [195.135.220.15]) by cuda.sgi.com with ESMTP id U4H6xNCHLQHsG922 for ; Wed, 26 May 2010 09:41:23 -0700 (PDT) Date: Thu, 27 May 2010 02:41:16 +1000 From: Nick Piggin Subject: Re: [PATCH 3/5] superblock: introduce per-sb cache shrinker infrastructure Message-ID: <20100526164116.GD22536@laptop> References: <1274777588-21494-1-git-send-email-david@fromorbit.com> <1274777588-21494-4-git-send-email-david@fromorbit.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <1274777588-21494-4-git-send-email-david@fromorbit.com> List-Id: XFS Filesystem from SGI List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: xfs-bounces@oss.sgi.com Errors-To: xfs-bounces@oss.sgi.com To: Dave Chinner Cc: linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org, xfs@oss.sgi.com T24gVHVlLCBNYXkgMjUsIDIwMTAgYXQgMDY6NTM6MDZQTSArMTAwMCwgRGF2ZSBDaGlubmVyIHdy b3RlOgo+IEBAIC00NTYsMjEgKzQ1NiwxNiBAQCBzdGF0aWMgdm9pZCBwcnVuZV9vbmVfZGVudHJ5 KHN0cnVjdCBkZW50cnkgKiBkZW50cnkpCj4gICAqIHdoaWNoIGZsYWdzIGFyZSBzZXQuIFRoaXMg bWVhbnMgd2UgZG9uJ3QgbmVlZCB0byBtYWludGFpbiBtdWx0aXBsZQo+ICAgKiBzaW1pbGFyIGNv cGllcyBvZiB0aGlzIGxvb3AuCj4gICAqLwo+IC1zdGF0aWMgdm9pZCBfX3Nocmlua19kY2FjaGVf c2Ioc3RydWN0IHN1cGVyX2Jsb2NrICpzYiwgaW50ICpjb3VudCwgaW50IGZsYWdzKQo+ICtzdGF0 aWMgdm9pZCBfX3Nocmlua19kY2FjaGVfc2Ioc3RydWN0IHN1cGVyX2Jsb2NrICpzYiwgaW50IGNv dW50LCBpbnQgZmxhZ3MpCj4gIHsKPiAgCUxJU1RfSEVBRChyZWZlcmVuY2VkKTsKPiAgCUxJU1Rf SEVBRCh0bXApOwo+ICAJc3RydWN0IGRlbnRyeSAqZGVudHJ5Owo+IC0JaW50IGNudCA9IDA7Cj4g IAo+ICAJQlVHX09OKCFzYik7Cj4gLQlCVUdfT04oKGZsYWdzICYgRENBQ0hFX1JFRkVSRU5DRUQp ICYmIGNvdW50ID09IE5VTEwpOwo+ICsJQlVHX09OKChmbGFncyAmIERDQUNIRV9SRUZFUkVOQ0VE KSAmJiBjb3VudCA9PSAtMSk7Cj4gIAlzcGluX2xvY2soJmRjYWNoZV9sb2NrKTsKPiAtCWlmIChj b3VudCAhPSBOVUxMKQo+IC0JCS8qIGNhbGxlZCBmcm9tIHBydW5lX2RjYWNoZSgpIGFuZCBzaHJp bmtfZGNhY2hlX3BhcmVudCgpICovCj4gLQkJY250ID0gKmNvdW50Owo+IC1yZXN0YXJ0Ogo+IC0J aWYgKGNvdW50ID09IE5VTEwpCj4gKwlpZiAoY291bnQgPT0gLTEpCj4gIAkJbGlzdF9zcGxpY2Vf aW5pdCgmc2ItPnNfZGVudHJ5X2xydSwgJnRtcCk7Cj4gIAllbHNlIHsKPiAgCQl3aGlsZSAoIWxp c3RfZW1wdHkoJnNiLT5zX2RlbnRyeV9scnUpKSB7Cj4gQEAgLTQ5MiwxMyArNDg3LDEzIEBAIHJl c3RhcnQ6Cj4gIAkJCX0gZWxzZSB7Cj4gIAkJCQlsaXN0X21vdmVfdGFpbCgmZGVudHJ5LT5kX2xy dSwgJnRtcCk7Cj4gIAkJCQlzcGluX3VubG9jaygmZGVudHJ5LT5kX2xvY2spOwo+IC0JCQkJY250 LS07Cj4gLQkJCQlpZiAoIWNudCkKPiArCQkJCWlmICgtLWNvdW50ID09IDApCj4gIAkJCQkJYnJl YWs7Cj4gIAkJCX0KPiAgCQkJY29uZF9yZXNjaGVkX2xvY2soJmRjYWNoZV9sb2NrKTsKPiAgCQl9 Cj4gIAl9Cj4gK3BydW5lX21vcmU6Cj4gIAl3aGlsZSAoIWxpc3RfZW1wdHkoJnRtcCkpIHsKPiAg CQlkZW50cnkgPSBsaXN0X2VudHJ5KHRtcC5wcmV2LCBzdHJ1Y3QgZGVudHJ5LCBkX2xydSk7Cj4g IAkJZGVudHJ5X2xydV9kZWxfaW5pdChkZW50cnkpOwo+IEBAIC01MTYsODggKzUxMSwyOSBAQCBy ZXN0YXJ0Ogo+ICAJCS8qIGRlbnRyeS0+ZF9sb2NrIHdhcyBkcm9wcGVkIGluIHBydW5lX29uZV9k ZW50cnkoKSAqLwo+ICAJCWNvbmRfcmVzY2hlZF9sb2NrKCZkY2FjaGVfbG9jayk7Cj4gIAl9Cj4g LQlpZiAoY291bnQgPT0gTlVMTCAmJiAhbGlzdF9lbXB0eSgmc2ItPnNfZGVudHJ5X2xydSkpCj4g LQkJZ290byByZXN0YXJ0Owo+IC0JaWYgKGNvdW50ICE9IE5VTEwpCj4gLQkJKmNvdW50ID0gY250 Owo+ICsJaWYgKGNvdW50ID09IC0xICYmICFsaXN0X2VtcHR5KCZzYi0+c19kZW50cnlfbHJ1KSkg ewo+ICsJCWxpc3Rfc3BsaWNlX2luaXQoJnNiLT5zX2RlbnRyeV9scnUsICZ0bXApOwo+ICsJCWdv dG8gcHJ1bmVfbW9yZTsKPiArCX0KCk5pdHBpY2sgYnV0IEkgcHJlZmVyIGp1c3QgdGhlIHJlc3Rh cnQgbGFiZWwgd2hlciBpdCBpcyBwcmV2aW91c2x5LiBUaGlzCmlzIG1vdmluZyBzZXR1cCBmb3Ig dGhlIG5leHQgaXRlcmF0aW9uIGludG8gdGhlICJlcnJvciIgY2FzZS4KCgo+ICtzdGF0aWMgaW50 IHBydW5lX3N1cGVyKHN0cnVjdCBzaHJpbmtlciAqc2hyaW5rLCBpbnQgbnJfdG9fc2NhbiwgZ2Zw X3QgZ2ZwX21hc2spCj4gK3sKPiArCXN0cnVjdCBzdXBlcl9ibG9jayAqc2I7Cj4gKwlpbnQgY291 bnQ7Cj4gKwo+ICsJc2IgPSBjb250YWluZXJfb2Yoc2hyaW5rLCBzdHJ1Y3Qgc3VwZXJfYmxvY2ss IHNfc2hyaW5rKTsKPiArCj4gKwkvKgo+ICsJICogRGVhZGxvY2sgYXZvaWRhbmNlLiAgV2UgbWF5 IGhvbGQgdmFyaW91cyBGUyBsb2NrcywgYW5kIHdlIGRvbid0IHdhbnQKPiArCSAqIHRvIHJlY3Vy c2UgaW50byB0aGUgRlMgdGhhdCBjYWxsZWQgdXMgaW4gY2xlYXJfaW5vZGUoKSBhbmQgZnJpZW5k cy4uCj4gKwkgKi8KPiArCWlmICghKGdmcF9tYXNrICYgX19HRlBfRlMpKQo+ICsJCXJldHVybiAt MTsKPiArCj4gKwkvKgo+ICsJICogaWYgd2UgY2FuJ3QgZ2V0IHRoZSB1bW91bnQgbG9jaywgdGhl biB0aGVyZSdzIG5vIHBvaW50IGhhdmluZyB0aGUKPiArCSAqIHNocmlua2VyIHRyeSBhZ2FpbiBi ZWNhdXNlIHRoZSBzYiBpcyBiZWluZyB0b3JuIGRvd24uCj4gKwkgKi8KPiArCWlmICghZG93bl9y ZWFkX3RyeWxvY2soJnNiLT5zX3Vtb3VudCkpCj4gKwkJcmV0dXJuIC0xOwoKV291bGQgeW91IGp1 c3QgZWxhYm9yYXRlIG9uIHRoZSBsb2NrIG9yZGVyIHByb2JsZW0gc29tZXdoZXJlPyAodGhlCmNv bW1lbnQgbWFrZXMgaXQgbG9vayBsaWtlIHdlICpjb3VsZCogdGFrZSB0aGUgbXV0ZXggaWYgd2Ug d2FudGVkCnRvKS4KCgo+ICsKPiArCWlmICghc2ItPnNfcm9vdCkgewo+ICsJCXVwX3JlYWQoJnNi LT5zX3Vtb3VudCk7Cj4gKwkJcmV0dXJuIC0xOwo+ICsJfQo+ICsKPiArCWlmIChucl90b19zY2Fu KSB7Cj4gKwkJLyogcHJvcG9ydGlvbiB0aGUgc2NhbiBiZXR3ZWVuIHRoZSB0d28gY2FjaGXRlSAq Lwo+ICsJCWludCB0b3RhbDsKPiArCj4gKwkJdG90YWwgPSBzYi0+c19ucl9kZW50cnlfdW51c2Vk ICsgc2ItPnNfbnJfaW5vZGVzX3VudXNlZCArIDE7Cj4gKwkJY291bnQgPSAobnJfdG9fc2NhbiAq IHNiLT5zX25yX2RlbnRyeV91bnVzZWQpIC8gdG90YWw7Cj4gKwo+ICsJCS8qIHBydW5lIGRjYWNo ZSBmaXJzdCBhcyBpY2FjaGUgaXMgcGlubmVkIGJ5IGl0ICovCj4gKwkJcHJ1bmVfZGNhY2hlX3Ni KHNiLCBjb3VudCk7Cj4gKwkJcHJ1bmVfaWNhY2hlX3NiKHNiLCBucl90b19zY2FuIC0gY291bnQp Owo+ICsJfQo+ICsKPiArCWNvdW50ID0gKChzYi0+c19ucl9kZW50cnlfdW51c2VkICsgc2ItPnNf bnJfaW5vZGVzX3VudXNlZCkgLyAxMDApCj4gKwkJCQkJCSogc3lzY3RsX3Zmc19jYWNoZV9wcmVz c3VyZTsKCkRvIHlvdSB0aGluayB0cnVuY2F0aW5nIGluIHRoZSBkaXZpc2lvbnMgaXMgYXQgYWxs IGEgcHJvYmxlbT8gSXQKcHJvYmFibHkgZG9lc24ndCBtYXR0ZXIgbXVjaCBJIHN1cHBvc2UuCgo+ IEBAIC0xNjIsNiArMjEzLDcgQEAgdm9pZCBkZWFjdGl2YXRlX2xvY2tlZF9zdXBlcihzdHJ1Y3Qg c3VwZXJfYmxvY2sgKnMpCj4gIAlzdHJ1Y3QgZmlsZV9zeXN0ZW1fdHlwZSAqZnMgPSBzLT5zX3R5 cGU7Cj4gIAlpZiAoYXRvbWljX2RlY19hbmRfdGVzdCgmcy0+c19hY3RpdmUpKSB7Cj4gIAkJdmZz X2RxX29mZihzLCAwKTsKPiArCQl1bnJlZ2lzdGVyX3Nocmlua2VyKCZzLT5zX3Nocmluayk7Cj4g IAkJZnMtPmtpbGxfc2Iocyk7Cj4gIAkJcHV0X2ZpbGVzeXN0ZW0oZnMpOwo+ICAJCXB1dF9zdXBl cihzKTsKPiBAQCAtMzM1LDYgKzM4Nyw3IEBAIHJldHJ5Ogo+ICAJbGlzdF9hZGRfdGFpbCgmcy0+ c19saXN0LCAmc3VwZXJfYmxvY2tzKTsKPiAgCWxpc3RfYWRkKCZzLT5zX2luc3RhbmNlcywgJnR5 cGUtPmZzX3N1cGVycyk7Cj4gIAlzcGluX3VubG9jaygmc2JfbG9jayk7Cj4gKwlyZWdpc3Rlcl9z aHJpbmtlcigmcy0+c19zaHJpbmspOwo+ICAJZ2V0X2ZpbGVzeXN0ZW0odHlwZSk7Cj4gIAlyZXR1 cm4gczsKPiAgfQo+IGRpZmYgLS1naXQgYS9pbmNsdWRlL2xpbnV4L2ZzLmggYi9pbmNsdWRlL2xp bnV4L2ZzLmgKPiBpbmRleCA3YjkwYzQzLi41YmZmMmRjIDEwMDY0NAo+IC0tLSBhL2luY2x1ZGUv bGludXgvZnMuaAo+ICsrKyBiL2luY2x1ZGUvbGludXgvZnMuaAo+IEBAIC0zODIsNiArMzgyLDcg QEAgc3RydWN0IGlub2Rlc19zdGF0X3Qgewo+ICAjaW5jbHVkZSA8bGludXgvY2FwYWJpbGl0eS5o Pgo+ICAjaW5jbHVkZSA8bGludXgvc2VtYXBob3JlLmg+Cj4gICNpbmNsdWRlIDxsaW51eC9maWVt YXAuaD4KPiArI2luY2x1ZGUgPGxpbnV4L21tLmg+Cj4gIAo+ICAjaW5jbHVkZSA8YXNtL2F0b21p Yy5oPgo+ICAjaW5jbHVkZSA8YXNtL2J5dGVvcmRlci5oPgo+IEBAIC0xMzg1LDggKzEzODYsMTQg QEAgc3RydWN0IHN1cGVyX2Jsb2NrIHsKPiAgCSAqIGdlbmVyaWNfc2hvd19vcHRpb25zKCkKPiAg CSAqLwo+ICAJY2hhciAqc19vcHRpb25zOwo+ICsKPiArCXN0cnVjdCBzaHJpbmtlciBzX3Nocmlu azsJLyogcGVyLXNiIHNocmlua2VyIGhhbmRsZSAqLwo+ICB9OwoKX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX18KeGZzIG1haWxpbmcgbGlzdAp4ZnNAb3NzLnNn aS5jb20KaHR0cDovL29zcy5zZ2kuY29tL21haWxtYW4vbGlzdGluZm8veGZzCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757040Ab0EZQlZ (ORCPT ); Wed, 26 May 2010 12:41:25 -0400 Received: from cantor2.suse.de ([195.135.220.15]:46777 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1757007Ab0EZQlX (ORCPT ); Wed, 26 May 2010 12:41:23 -0400 Date: Thu, 27 May 2010 02:41:16 +1000 From: Nick Piggin To: Dave Chinner Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, xfs@oss.sgi.com Subject: Re: [PATCH 3/5] superblock: introduce per-sb cache shrinker infrastructure Message-ID: <20100526164116.GD22536@laptop> References: <1274777588-21494-1-git-send-email-david@fromorbit.com> <1274777588-21494-4-git-send-email-david@fromorbit.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1274777588-21494-4-git-send-email-david@fromorbit.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, May 25, 2010 at 06:53:06PM +1000, Dave Chinner wrote: > @@ -456,21 +456,16 @@ static void prune_one_dentry(struct dentry * dentry) > * which flags are set. This means we don't need to maintain multiple > * similar copies of this loop. > */ > -static void __shrink_dcache_sb(struct super_block *sb, int *count, int flags) > +static void __shrink_dcache_sb(struct super_block *sb, int count, int flags) > { > LIST_HEAD(referenced); > LIST_HEAD(tmp); > struct dentry *dentry; > - int cnt = 0; > > BUG_ON(!sb); > - BUG_ON((flags & DCACHE_REFERENCED) && count == NULL); > + BUG_ON((flags & DCACHE_REFERENCED) && count == -1); > spin_lock(&dcache_lock); > - if (count != NULL) > - /* called from prune_dcache() and shrink_dcache_parent() */ > - cnt = *count; > -restart: > - if (count == NULL) > + if (count == -1) > list_splice_init(&sb->s_dentry_lru, &tmp); > else { > while (!list_empty(&sb->s_dentry_lru)) { > @@ -492,13 +487,13 @@ restart: > } else { > list_move_tail(&dentry->d_lru, &tmp); > spin_unlock(&dentry->d_lock); > - cnt--; > - if (!cnt) > + if (--count == 0) > break; > } > cond_resched_lock(&dcache_lock); > } > } > +prune_more: > while (!list_empty(&tmp)) { > dentry = list_entry(tmp.prev, struct dentry, d_lru); > dentry_lru_del_init(dentry); > @@ -516,88 +511,29 @@ restart: > /* dentry->d_lock was dropped in prune_one_dentry() */ > cond_resched_lock(&dcache_lock); > } > - if (count == NULL && !list_empty(&sb->s_dentry_lru)) > - goto restart; > - if (count != NULL) > - *count = cnt; > + if (count == -1 && !list_empty(&sb->s_dentry_lru)) { > + list_splice_init(&sb->s_dentry_lru, &tmp); > + goto prune_more; > + } Nitpick but I prefer just the restart label wher it is previously. This is moving setup for the next iteration into the "error" case. > +static int prune_super(struct shrinker *shrink, int nr_to_scan, gfp_t gfp_mask) > +{ > + struct super_block *sb; > + int count; > + > + sb = container_of(shrink, struct super_block, s_shrink); > + > + /* > + * Deadlock avoidance. We may hold various FS locks, and we don't want > + * to recurse into the FS that called us in clear_inode() and friends.. > + */ > + if (!(gfp_mask & __GFP_FS)) > + return -1; > + > + /* > + * if we can't get the umount lock, then there's no point having the > + * shrinker try again because the sb is being torn down. > + */ > + if (!down_read_trylock(&sb->s_umount)) > + return -1; Would you just elaborate on the lock order problem somewhere? (the comment makes it look like we *could* take the mutex if we wanted to). > + > + if (!sb->s_root) { > + up_read(&sb->s_umount); > + return -1; > + } > + > + if (nr_to_scan) { > + /* proportion the scan between the two cacheѕ */ > + int total; > + > + total = sb->s_nr_dentry_unused + sb->s_nr_inodes_unused + 1; > + count = (nr_to_scan * sb->s_nr_dentry_unused) / total; > + > + /* prune dcache first as icache is pinned by it */ > + prune_dcache_sb(sb, count); > + prune_icache_sb(sb, nr_to_scan - count); > + } > + > + count = ((sb->s_nr_dentry_unused + sb->s_nr_inodes_unused) / 100) > + * sysctl_vfs_cache_pressure; Do you think truncating in the divisions is at all a problem? It probably doesn't matter much I suppose. > @@ -162,6 +213,7 @@ void deactivate_locked_super(struct super_block *s) > struct file_system_type *fs = s->s_type; > if (atomic_dec_and_test(&s->s_active)) { > vfs_dq_off(s, 0); > + unregister_shrinker(&s->s_shrink); > fs->kill_sb(s); > put_filesystem(fs); > put_super(s); > @@ -335,6 +387,7 @@ retry: > list_add_tail(&s->s_list, &super_blocks); > list_add(&s->s_instances, &type->fs_supers); > spin_unlock(&sb_lock); > + register_shrinker(&s->s_shrink); > get_filesystem(type); > return s; > } > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 7b90c43..5bff2dc 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -382,6 +382,7 @@ struct inodes_stat_t { > #include > #include > #include > +#include > > #include > #include > @@ -1385,8 +1386,14 @@ struct super_block { > * generic_show_options() > */ > char *s_options; > + > + struct shrinker s_shrink; /* per-sb shrinker handle */ > }; From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nick Piggin Subject: Re: [PATCH 3/5] superblock: introduce per-sb cache shrinker infrastructure Date: Thu, 27 May 2010 02:41:16 +1000 Message-ID: <20100526164116.GD22536@laptop> References: <1274777588-21494-1-git-send-email-david@fromorbit.com> <1274777588-21494-4-git-send-email-david@fromorbit.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, xfs@oss.sgi.com To: Dave Chinner Return-path: Content-Disposition: inline In-Reply-To: <1274777588-21494-4-git-send-email-david@fromorbit.com> Sender: owner-linux-mm@kvack.org List-Id: linux-fsdevel.vger.kernel.org On Tue, May 25, 2010 at 06:53:06PM +1000, Dave Chinner wrote: > @@ -456,21 +456,16 @@ static void prune_one_dentry(struct dentry * dent= ry) > * which flags are set. This means we don't need to maintain multiple > * similar copies of this loop. > */ > -static void __shrink_dcache_sb(struct super_block *sb, int *count, int= flags) > +static void __shrink_dcache_sb(struct super_block *sb, int count, int = flags) > { > LIST_HEAD(referenced); > LIST_HEAD(tmp); > struct dentry *dentry; > - int cnt =3D 0; > =20 > BUG_ON(!sb); > - BUG_ON((flags & DCACHE_REFERENCED) && count =3D=3D NULL); > + BUG_ON((flags & DCACHE_REFERENCED) && count =3D=3D -1); > spin_lock(&dcache_lock); > - if (count !=3D NULL) > - /* called from prune_dcache() and shrink_dcache_parent() */ > - cnt =3D *count; > -restart: > - if (count =3D=3D NULL) > + if (count =3D=3D -1) > list_splice_init(&sb->s_dentry_lru, &tmp); > else { > while (!list_empty(&sb->s_dentry_lru)) { > @@ -492,13 +487,13 @@ restart: > } else { > list_move_tail(&dentry->d_lru, &tmp); > spin_unlock(&dentry->d_lock); > - cnt--; > - if (!cnt) > + if (--count =3D=3D 0) > break; > } > cond_resched_lock(&dcache_lock); > } > } > +prune_more: > while (!list_empty(&tmp)) { > dentry =3D list_entry(tmp.prev, struct dentry, d_lru); > dentry_lru_del_init(dentry); > @@ -516,88 +511,29 @@ restart: > /* dentry->d_lock was dropped in prune_one_dentry() */ > cond_resched_lock(&dcache_lock); > } > - if (count =3D=3D NULL && !list_empty(&sb->s_dentry_lru)) > - goto restart; > - if (count !=3D NULL) > - *count =3D cnt; > + if (count =3D=3D -1 && !list_empty(&sb->s_dentry_lru)) { > + list_splice_init(&sb->s_dentry_lru, &tmp); > + goto prune_more; > + } Nitpick but I prefer just the restart label wher it is previously. This is moving setup for the next iteration into the "error" case. > +static int prune_super(struct shrinker *shrink, int nr_to_scan, gfp_t = gfp_mask) > +{ > + struct super_block *sb; > + int count; > + > + sb =3D container_of(shrink, struct super_block, s_shrink); > + > + /* > + * Deadlock avoidance. We may hold various FS locks, and we don't wa= nt > + * to recurse into the FS that called us in clear_inode() and friends= .. > + */ > + if (!(gfp_mask & __GFP_FS)) > + return -1; > + > + /* > + * if we can't get the umount lock, then there's no point having the > + * shrinker try again because the sb is being torn down. > + */ > + if (!down_read_trylock(&sb->s_umount)) > + return -1; Would you just elaborate on the lock order problem somewhere? (the comment makes it look like we *could* take the mutex if we wanted to). > + > + if (!sb->s_root) { > + up_read(&sb->s_umount); > + return -1; > + } > + > + if (nr_to_scan) { > + /* proportion the scan between the two cache=D1=95 */ > + int total; > + > + total =3D sb->s_nr_dentry_unused + sb->s_nr_inodes_unused + 1; > + count =3D (nr_to_scan * sb->s_nr_dentry_unused) / total; > + > + /* prune dcache first as icache is pinned by it */ > + prune_dcache_sb(sb, count); > + prune_icache_sb(sb, nr_to_scan - count); > + } > + > + count =3D ((sb->s_nr_dentry_unused + sb->s_nr_inodes_unused) / 100) > + * sysctl_vfs_cache_pressure; Do you think truncating in the divisions is at all a problem? It probably doesn't matter much I suppose. > @@ -162,6 +213,7 @@ void deactivate_locked_super(struct super_block *s) > struct file_system_type *fs =3D s->s_type; > if (atomic_dec_and_test(&s->s_active)) { > vfs_dq_off(s, 0); > + unregister_shrinker(&s->s_shrink); > fs->kill_sb(s); > put_filesystem(fs); > put_super(s); > @@ -335,6 +387,7 @@ retry: > list_add_tail(&s->s_list, &super_blocks); > list_add(&s->s_instances, &type->fs_supers); > spin_unlock(&sb_lock); > + register_shrinker(&s->s_shrink); > get_filesystem(type); > return s; > } > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 7b90c43..5bff2dc 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -382,6 +382,7 @@ struct inodes_stat_t { > #include > #include > #include > +#include > =20 > #include > #include > @@ -1385,8 +1386,14 @@ struct super_block { > * generic_show_options() > */ > char *s_options; > + > + struct shrinker s_shrink; /* per-sb shrinker handle */ > }; -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail138.messagelabs.com (mail138.messagelabs.com [216.82.249.35]) by kanga.kvack.org (Postfix) with SMTP id D079B6B01B8 for ; Wed, 26 May 2010 12:41:24 -0400 (EDT) Date: Thu, 27 May 2010 02:41:16 +1000 From: Nick Piggin Subject: Re: [PATCH 3/5] superblock: introduce per-sb cache shrinker infrastructure Message-ID: <20100526164116.GD22536@laptop> References: <1274777588-21494-1-git-send-email-david@fromorbit.com> <1274777588-21494-4-git-send-email-david@fromorbit.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1274777588-21494-4-git-send-email-david@fromorbit.com> Sender: owner-linux-mm@kvack.org To: Dave Chinner Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, xfs@oss.sgi.com List-ID: On Tue, May 25, 2010 at 06:53:06PM +1000, Dave Chinner wrote: > @@ -456,21 +456,16 @@ static void prune_one_dentry(struct dentry * dentry) > * which flags are set. This means we don't need to maintain multiple > * similar copies of this loop. > */ > -static void __shrink_dcache_sb(struct super_block *sb, int *count, int flags) > +static void __shrink_dcache_sb(struct super_block *sb, int count, int flags) > { > LIST_HEAD(referenced); > LIST_HEAD(tmp); > struct dentry *dentry; > - int cnt = 0; > > BUG_ON(!sb); > - BUG_ON((flags & DCACHE_REFERENCED) && count == NULL); > + BUG_ON((flags & DCACHE_REFERENCED) && count == -1); > spin_lock(&dcache_lock); > - if (count != NULL) > - /* called from prune_dcache() and shrink_dcache_parent() */ > - cnt = *count; > -restart: > - if (count == NULL) > + if (count == -1) > list_splice_init(&sb->s_dentry_lru, &tmp); > else { > while (!list_empty(&sb->s_dentry_lru)) { > @@ -492,13 +487,13 @@ restart: > } else { > list_move_tail(&dentry->d_lru, &tmp); > spin_unlock(&dentry->d_lock); > - cnt--; > - if (!cnt) > + if (--count == 0) > break; > } > cond_resched_lock(&dcache_lock); > } > } > +prune_more: > while (!list_empty(&tmp)) { > dentry = list_entry(tmp.prev, struct dentry, d_lru); > dentry_lru_del_init(dentry); > @@ -516,88 +511,29 @@ restart: > /* dentry->d_lock was dropped in prune_one_dentry() */ > cond_resched_lock(&dcache_lock); > } > - if (count == NULL && !list_empty(&sb->s_dentry_lru)) > - goto restart; > - if (count != NULL) > - *count = cnt; > + if (count == -1 && !list_empty(&sb->s_dentry_lru)) { > + list_splice_init(&sb->s_dentry_lru, &tmp); > + goto prune_more; > + } Nitpick but I prefer just the restart label wher it is previously. This is moving setup for the next iteration into the "error" case. > +static int prune_super(struct shrinker *shrink, int nr_to_scan, gfp_t gfp_mask) > +{ > + struct super_block *sb; > + int count; > + > + sb = container_of(shrink, struct super_block, s_shrink); > + > + /* > + * Deadlock avoidance. We may hold various FS locks, and we don't want > + * to recurse into the FS that called us in clear_inode() and friends.. > + */ > + if (!(gfp_mask & __GFP_FS)) > + return -1; > + > + /* > + * if we can't get the umount lock, then there's no point having the > + * shrinker try again because the sb is being torn down. > + */ > + if (!down_read_trylock(&sb->s_umount)) > + return -1; Would you just elaborate on the lock order problem somewhere? (the comment makes it look like we *could* take the mutex if we wanted to). > + > + if (!sb->s_root) { > + up_read(&sb->s_umount); > + return -1; > + } > + > + if (nr_to_scan) { > + /* proportion the scan between the two cacheN? */ > + int total; > + > + total = sb->s_nr_dentry_unused + sb->s_nr_inodes_unused + 1; > + count = (nr_to_scan * sb->s_nr_dentry_unused) / total; > + > + /* prune dcache first as icache is pinned by it */ > + prune_dcache_sb(sb, count); > + prune_icache_sb(sb, nr_to_scan - count); > + } > + > + count = ((sb->s_nr_dentry_unused + sb->s_nr_inodes_unused) / 100) > + * sysctl_vfs_cache_pressure; Do you think truncating in the divisions is at all a problem? It probably doesn't matter much I suppose. > @@ -162,6 +213,7 @@ void deactivate_locked_super(struct super_block *s) > struct file_system_type *fs = s->s_type; > if (atomic_dec_and_test(&s->s_active)) { > vfs_dq_off(s, 0); > + unregister_shrinker(&s->s_shrink); > fs->kill_sb(s); > put_filesystem(fs); > put_super(s); > @@ -335,6 +387,7 @@ retry: > list_add_tail(&s->s_list, &super_blocks); > list_add(&s->s_instances, &type->fs_supers); > spin_unlock(&sb_lock); > + register_shrinker(&s->s_shrink); > get_filesystem(type); > return s; > } > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 7b90c43..5bff2dc 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -382,6 +382,7 @@ struct inodes_stat_t { > #include > #include > #include > +#include > > #include > #include > @@ -1385,8 +1386,14 @@ struct super_block { > * generic_show_options() > */ > char *s_options; > + > + struct shrinker s_shrink; /* per-sb shrinker handle */ > }; -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@kvack.org. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: email@kvack.org