| To: | Dave Chinner <david@xxxxxxxxxxxxx> |
|---|---|
| Subject: | Re: [PATCH 08/12] superblock: introduce per-sb cache shrinker infrastructure |
| From: | Al Viro <viro@xxxxxxxxxxxxxxxxxx> |
| Date: | Sat, 4 Jun 2011 01:42:31 +0100 |
| Cc: | linux-fsdevel@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx, linux-mm@xxxxxxxxx, xfs@xxxxxxxxxxx |
| In-reply-to: | <1306998067-27659-9-git-send-email-david@xxxxxxxxxxxxx> |
| References: | <1306998067-27659-1-git-send-email-david@xxxxxxxxxxxxx> <1306998067-27659-9-git-send-email-david@xxxxxxxxxxxxx> |
| Sender: | Al Viro <viro@xxxxxxxxxxxxxxxx> |
| User-agent: | Mutt/1.5.21 (2010-09-15) |
> @@ -278,7 +325,12 @@ void generic_shutdown_super(struct super_block *sb)
> {
> const struct super_operations *sop = sb->s_op;
>
> -
> + /*
> + * shut down the shrinker first so we know that there are no possible
> + * races when shrinking the dcache or icache. Removes the need for
> + * external locking to prevent such races.
> + */
> + unregister_shrinker(&sb->s_shrink);
> if (sb->s_root) {
> shrink_dcache_for_umount(sb);
> sync_filesystem(sb);
What it means is that shrinker_rwsem now nests inside ->s_umount... IOW,
if any ->shrink() gets stuck, so does every generic_shutdown_super().
I'm still not convinced it's a good idea - especially since _this_
superblock will be skipped anyway. Is there any good reason to evict
shrinker that early? Note that doing that after ->s_umount is dropped
should be reasonably safe - your shrinker will see that superblock is
doomed if it's called anywhere in that window...
|
| Previous by Date: | Re: [PATCH 06/12] inode: Make unused inode LRU per superblock, Al Viro |
|---|---|
| Next by Date: | Re: [PATCH 06/12] inode: Make unused inode LRU per superblock, Dave Chinner |
| Previous by Thread: | [PATCH 08/12] superblock: introduce per-sb cache shrinker infrastructure, Dave Chinner |
| Next by Thread: | Re: [PATCH 08/12] superblock: introduce per-sb cache shrinker infrastructure, Dave Chinner |
| Indexes: | [Date] [Thread] [Top] [All Lists] |