Re: Checking page_count(page) in invalidate_complete_page

Chuck Lever
2006-10-04T19:29:24Z

Andrew Morton wrote:
> On Tue, 03 Oct 2006 15:10:01 -0400
> Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
>
>>>>> + result = invalidate_inode_pages2(mapping);
>>>>> + if (unlikely(result) < 0) {
>>>>> + nfs_inc_stats(inode, NFSIOS_INVALIDATEFAILED);
>>>>> + printk(KERN_ERR
>>>>> + "NFS: error %d invalidating pages for inode (%s/%Ld)\n",
>>>>> + result, inode->i_sb->s_id,
>>>>> + (long long)NFS_FILEID(inode));
>>>> So what _are_ users expected to do about this? Sue us? Complain bitterly
>>>> to lkml, and then get told that the VM is broken?
>>> Such a message will be reported to distributors or lkml, and we will be
>>> able to collect data about the scenario where there is a problem.
>> i.e. we're throwing our hands up into the air, and saying "we don't
>> understand what is going on here"?
>>
>> Sorry, but that is not an option. If we can't design a clearly defined
>> API for cache invalidation, with well defined errors, then we should all
>> go home and start learning Solaris programming now.
>
> I don't think we need the stats increment in there. We could make it a
> plain WARN_ON() or BUG_ON(). Or just whack a printk in there. It's hardly
> a big deal.

A "WARN_ON(ret != 0);" placed at the end of
invalidate_inode_pages2_range() should be sufficient and harmless.