Messages in this thread | | | Date | Wed, 17 May 2006 16:52:36 +0100 (BST) | From | Hugh Dickins <> | Subject | Re: swapper_space export |
| |
On Wed, 17 May 2006, Arjan van de Ven wrote: > On Tue, 2006-05-16 at 19:24 -0400, Josef Sipek wrote: > > I was trying to compile the Unionfs[1] to get it up to sync it up with > > the kernel developments from the past few months. Anyway, long story > > short...swapper_space (defined in mm/swap_state.c) is not exported > > anymore (git commit: 4936967374c1ad0eb3b734f24875e2484c3786cc). This > > apparently is not an issue for most modules. Troubles arise when the > > modules include mm.h or any of its relatives. > > > > One simply gets a linker error about swapper_space not being defined. > > The problem is that it is used in mm.h. > > don't you think it's really suspect that no other filesystem, in fact no > other driver in the kernel needs this? Could it just be that unionfs is > using a wrong API ? Because if that's the case you're patch is just the > wrong thing. Maybe the unionfs people should try to submit their code > for review etc......
Much as I'd love to side with Jeff against you and Adrian ;) I think you're right.
I see no reference to page_mapping() in the unionfs source (and at present there's no other justifiable modular use of swapper_space); but my guess would be that Jeff is being more conscientious than is called for in getting it to sync up with the kernel -
The unionfs source does contain its own inline "sync_page" which comments that it "is copied verbatim from mm/filemap.c". I'm guessing Jeff has noticed that it's no longer a verbatim copy, has made it so, and is thereby involving page_mapping().
No need for that here (nor for the smp_mb nor for the io_schedule): unionfs's sync_page is working on a locked pagecache page of the lower-level filesystem, that's not going to be a PageSwapCache page nor a PageAnon page (nor even a truncated page with NULL mapping: page lock is held). Just use page->mapping as before.
(But I notice that unionfs better not have a tmpfs in its union: the unionfs use of grab_cache_page is not strictly compatible with the way tmpfs pages are swapped out under memory pressure.)
Hugh - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |