On Mon, 2010-02-08 at 08:23 -0500, Trond Myklebust wrote: > On Sun, 2010-02-07 at 16:56 +0530, Suresh Jayaraman wrote: > > 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. To be more precise, even before we put call to nfs_wb_page() in nfs_release_page(), it was possible for the PG_private bit to be set when doing the test in shrink_page_list(), but for an outstanding commit operation to complete before the second test in nfs_release_page. In this case, nfs_fscache_release_page would get called with neither PG_private nor PG_fscache being set, and the Oops could occur. > > 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. I've added a cleanup patch (which will not go to stable@xxxxxxxxxx) to do this. 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
