On Sat, 17 Mar 2012, Linus Torvalds wrote: > On Sat, Mar 17, 2012 at 1:23 PM, Dave Airlie <airlied@xxxxxxxxx> wrote: > > > > I did however get a flashback in google to this: > > > > http://lists.fedoraproject.org/pipermail/scm-commits/2010-July/456636.html > > > > Linus don't think we ever did work out why that worked, I wonder if we > > lost something after that. > > Hmm. Maybe we should stop making the default gfp_mask be > GFP_HIGHMEM_MOVABLE (see inode_init_always() in fs/inode.c), and > instead add the _MOVABLE flag only for mappings that use > "generic_file_mmap()". > > It does sound a bit scary to just make default mappings use MOVABLE > pages, when we know that can be incorrect for some cases. > > But that would require that filesystems like ext4 etc that don't just > use the generic_file_mmap() function would have add do that MOVABLE > thing on their own. And the *normal* case is certainly that pages are > movable and putting them in themovable zone should be ok. It's just > *not* ok if you also map them into some hardware GTT thing or just > otherwise keep track of the pages directly, like DRI does. > > The other possibility is to just make this a shm thing, since it's > generally that layer that has the case of "pages allocated with the > mapping gfp masks". Hugh Dickins clearly tried to make sure that the > DRM initialization of the gfp mask was honored, but maybe there is > some case where it is missed. Hugh added a mapping_set_gfp_mask() to > i915_gem_alloc_object(), but maybe we have i915 shmem mappings that > get set up through some other path. > > What about the ttm swap storage thing, for example? That also does > shmem_file_setup(), without limiting the pages. But I don't think i915 > uses ttm, right? > > Al? Hugh? Opinions? Right now anybody who uses the > shmem_read_mapping_page() function is in danger of getting a MOVABLE > page by default. Which may or may not be right. > > That said, I don't see how i915 would get a movable page. It *seems* > to do the right setup for the gfp flags of the mapping. Yes, i915_gem_alloc_object() does its own mapping_set_gfp_mask(mapping, GFP_HIGHUSER | __GFP_RECLAIMABLE), which should be as good as ever. I've got to go out for an hour: I'll digest more and think more about this when I get back. If someone could explain the original problem with _MOVABLE, that would help me: I didn't see an explanation in the patch or in the bugzilla. I would expect using _MOVABLE for i915_gem would render a block of otherwise movable pages immovable because of the GEM pages pinned, but I wouldn't expect them to move in mysterious ways - or are references held to the pages without them being pinned??. A factor which might turn out to be relevant: swapin_readahead() (or swapoff) brings in pages from swap before it knows who they belong to, so the swapped-in swapcache pages don't necessarily have the limitations mapping_set_gfp_mask() has asked for. It's been discussed with Alan, we know it will need fixing for gma500 when eventually that comes to be used on machines with more than 4GB, but for now I wasn't aware of a problem. Hugh _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
