Re: Checking page_count(page) in invalidate_complete_page

Andrew Morton
2006-10-04T19:43:56Z

On Wed, 04 Oct 2006 15:29:24 -0400
Chuck Lever <chuck.lever@oracle.com> wrote:

> 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.

true. I think. I'll take a look at adding a WARN_ON_ONCE().