On Mon, Nov 21, 2011 at 12:34 PM, Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > On Mon, Nov 21, 2011 at 12:11:32PM +0100, Miklos Szeredi wrote: >> Do not WARN_ON if set_nlink is called with zero count, just do a >> ratelimited printk. This happens on xfs and probably other >> filesystems after an unclean shutdown when the filesystem reads inodes >> which already have zero i_nlink. Reported by Christoph Hellwig. > > Given that this is part of the normal recovery process printing anything > seems like a bad idea. I also don't think the code for this actually > is correct. > > Remember when a filesystem recovery from unlinked but open inodes the > following happens: > > - we walk the list of unlinked but open inodes, and read them into > memory, remove the linkage and then iput it. > > With the current code that won't ever increment s_remove_count, It will increment s_remove_count +void set_nlink(struct inode *inode, unsigned int nlink) +{ + if (!nlink) { + printk_ratelimited(KERN_INFO + "set_nlink() clearing i_nlink on %s inode %li\n", + inode->i_sb->s_type->name, inode->i_ino); here: + clear_nlink(inode); > but > decrement it from __destroy_inode. I suspect the right fix is to > simply not warn for a set_nlink to zero, but rather simply increment > s_remove_count for that case. I don't really care about the printk. Without the printk clear_nlink() is just a shorthand for set_nlink(0), which is fine, but that's not what the original intention was AFAIK. Thanks, Miklos -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html