On Thu, 25 Oct 2007, Pekka Enberg wrote: > On 10/25/07, Hugh Dickins <hugh@xxxxxxxxxxx> wrote: > > --- 2.6.24-rc1/mm/shmem.c 2007-10-24 07:16:04.000000000 +0100 > > +++ linux/mm/shmem.c 2007-10-24 22:31:09.000000000 +0100 > > @@ -915,6 +915,21 @@ static int shmem_writepage(struct page * > > struct inode *inode; > > > > BUG_ON(!PageLocked(page)); > > + /* > > + * shmem_backing_dev_info's capabilities prevent regular writeback or > > + * sync from ever calling shmem_writepage; but a stacking filesystem > > + * may use the ->writepage of its underlying filesystem, in which case > > I find the above bit somewhat misleading as it implies that the > !wbc->for_reclaim case can be removed after ecryptfs has similar fix > as unionfs. Can we just say that while BDI_CAP_NO_WRITEBACK does > prevent some callers from entering ->writepage(), it's just an > optimization and ->writepage() must deal with !wbc->for_reclaim case > properly? Sorry for being obtuse, but I don't see how that's misleading at all. ecryptfs already has a (dissimilar) fix in 2.6.24-rc1, not using the writepage route at all. But it remains the case that some stacking filesystem may (would you prefer "might" to "may"? "may" has a nice double meaning of "might" and "we'll allow it", but this patch does indeed allow it) use the ->writepage of its underlying filesystem. With unionfs also fixed, we don't know of an absolute need for this patch (and so, on that basis, the !wbc->for_reclaim case could indeed be removed very soon); but as I see it, the unionfs case has shown that it's time to future-proof this code against whatever stacking filesystems come along. Hence I didn't mention the names of such filesystems in the source comment. The !page_mapped assumption has been built in there since earliest 2.4, but it took a while for us to get a way to express it in a BUG. Hugh - 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
