On Fri, 19 Dec 2008, Linus Torvalds wrote: > > That said, I did notice a problem. Namely that while the VM code is good > about looking at ->mapping (because it doesn't know whether the page is > anonymous or a true mapping), much of the filesystem code is _not_ careful > about page->mapping, since the filesystem code knows a-priori that the > mapping pointer must be an inode mapping (or we'd not have called it). > > So filesystems do tend to do things like > > struct inode *inode = page->mapping->host; > > and while the low bit of mapping is magic, those code-paths don't care > because they depend on it being zero. > > So hiding the lock bit there would involve a lot more work than I naïvely > expected before looking closer. We'd have to change the name (to > "_mapping", presumably), and make all users use an accessor function to > make code like the above do > > struct inode *inode = page_mapping(page)->host; > > or something (we migth want to have a "page_host_inode()" helper to do it, > it seems to be the most common reason for accessing "->mapping" that > there is. > > So it could be done pretty mechanically, but it's still a _big_ change. > Maybe not worth it, unless we can really translate it into some other > advantage (ie real simplification of page flag access) Yes, it's messy, particularly given out-of-tree filesystems. Perhaps there's somewhere else in the struct page you could keep the lock bit and get the advantage you're looking for: playing with the low bits of page->mapping is best left to anon pages. I did have a patch to keep PG_swapcache there: that got stalled on dreaming up enough memorable names for variants of page_mapping() that I turned out to need, then it got buried under other work. I'll resurrect it in the next month or so; it would be nice to keep PG_swapbacked there too, but it's not quite as easy since Rik's split LRU categorizes shmem/tmpfs pages as swapbacked i.e. that flag applies to anon and also to one particular filesystem. Hugh
