On 13.02.2008 [11:55:59 +1100], David Gibson wrote: > On Tue, Feb 12, 2008 at 04:30:51PM -0800, Nishanth Aravamudan wrote: > > > > tests: CONFIG out if required number of free hugepages are unavailable > > > > Some tests (not all) require a certain number of hugepages (known in > > advance) to work. Right now, we will fail to mmap() with ENOMEM, when > > it's really a configuration issue (I'd argue). Use what was previously a > > local helper, hugetlbfs_num_free_pages(), but renamed to > > num_free_hpages() [as, as far as I can tell, it is not technically tied > > to hugetlbfs], to see how many hugepages are free before we run the > > affected tests. > > > > Signed-off-by: Nishanth Aravamudan <[EMAIL PROTECTED]> > > --- > > I can split this into separate bits (the rename / globalization could be > > two separate patches, then the users could be added), but I wanted to > > get this out as an RFC. There are two follow-on test fixes that I'm > > working on still: overflowing long when calculating number of hugepages > > requested * hpage_size (which includes overflowing long in the > > calculation of hpage_size!) and [perhaps] dealing better with mmap() > > failing due to lack of space in the address space. > > Certainly we should do CONFIG() for these cases, not FAIL(). However, > I'm uneasy about exporting num_free_hugepages().
That's fair. For what it's worth, before this patch, there are no users of either function in the library and the symbols are not visible to users of libhugetlbfs.so. > I've deliberately avoided exporting that interface so far, because > most obvious uses of it are racy (because someone else could grab a > bunch of pages between checking the total and performing our map for > real). Now in the case of the testsuite, where it's purely a helpful > diagnostic, it doesn't really matter, but I don't want people to look > at this usage and think that's an acceptable way of checking if there > are enough hugepages for their app. True. To be clear, without reserving the hugepages up-front, there is no non-racy way of checking available huge pages. > What we should perhaps do instead, is explicitly check for ENOMEM on > the first map, and CONFIG() instead of FAIL() on that case. That seems like a good idea. I'll respin the patch to do this. I also noticed a few cases (well, at least one), where get_unlinked_fd()'s failure was treated as a CONFIG(), when I think it should be a FAIL() [and is one in the majority of tests]. Just to be clear, are we sure that ENOMEM from the first mmap() always will indicate an insufficiently large pool? One issue I had, even with my patch, was what to do in the presence of the dynamic pool, as even with 0 free huge pages, the mmap() might succeed. Checking mmap()s return values works for that case, though. Thanks, Nish -- Nishanth Aravamudan <[EMAIL PROTECTED]> IBM Linux Technology Center ------------------------------------------------------------------------- This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/ _______________________________________________ Libhugetlbfs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/libhugetlbfs-devel
