On Sun, 2010-02-07 at 16:56 +0530, Suresh Jayaraman wrote: > On 02/06/2010 04:13 AM, Trond Myklebust wrote: > > Not having an fscache cookie is perfectly valid if the user didn't mount > > with the fscache option. > > > > This patch fixes http://bugzilla.kernel.org/show_bug.cgi?id=15234 > > > > Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> > > Cc: stable@xxxxxxxxxx > > --- > > fs/nfs/fscache.c | 6 +++--- > > 1 files changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/fs/nfs/fscache.c b/fs/nfs/fscache.c > > index fa58800..534adb8 100644 > > --- a/fs/nfs/fscache.c > > +++ b/fs/nfs/fscache.c > > @@ -355,11 +355,11 @@ void nfs_fscache_reset_inode_cookie(struct inode *inode) > > int nfs_fscache_release_page(struct page *page, gfp_t gfp) > > { > > struct nfs_inode *nfsi = NFS_I(page->mapping->host); > > - struct fscache_cookie *cookie = nfsi->fscache; > > - > > - BUG_ON(!cookie); > > > > if (PageFsCache(page)) { > > + struct fscache_cookie *cookie = nfsi->fscache; > > + > > + BUG_ON(!cookie); > > dfprintk(FSCACHE, "NFS: fscache releasepage (0x%p/0x%p/0x%p)\n", > > cookie, page, nfsi); > > > > There are only two callers for nfs_fscache_release_page() - > nfs_release_page() and nfs_migrate_page(). nfs_migrate_page already does > this: > > if (PageFsCache(page)) > nfs_fscache_release_page(page, GFP_KERNEL); > > and the assumption in nfs_release_page() is that the page should have > either PG_private set or PG_fscache set and nfs_fscache_release_page > gets called only if PG_private is not set. ...or if it gets cleared. > I think the idea is that nfs_fscache_release_page should not get called > if fsc option is not used. So it appears to me this patch is fixing the > symptom not the actual issue. Perhaps, this the assumption in > nfs_release_page is wrong or the PageFsCache() check should be moved to > nfs_release_page? No. We should rather get rid of the redundant check for PageFsCache() in nfs_migrate_page. PageFsCache() is particular to fscache, so the test belongs in the fscache code. Trond -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
