On Tue, Dec 13, 2016 at 10:10:41PM +0100, Luis R. Rodriguez wrote: > On Thu, Dec 08, 2016 at 12:24:35PM -0800, Kees Cook wrote: > > On Thu, Dec 8, 2016 at 10:47 AM, Luis R. Rodriguez <mcgrof@xxxxxxxxxx> wrote: > > > 3) finit_module() consumes quite a bit of memory. > > > > Is this due to reading the module into kernel memory or something else? > > Very likely yes, but to be honest I have not had chance to instrument too > carefully, its TODO work :) I've checked and the issue is since get_fs_type() does not check for aliases we end up hammering tons of module requests, this in turn is an analysis on load_module(). Within there layout_and_allocate() uses first a local copy of the passed user data and mapping it into a struct module, after a bit of sanity checks it finally allocates a copy for us, so its struct module size * however many requests were allowed to get in for load_module(). We could simply avoid an allocation if the module is already present. I have this as another optimization now but am running many other tests to compare performance. > > > +# Once tese are enabled please leave them as-is. Write your own test, > > > +# we have tons of space. > > > +kmod_test_0001 > > > +kmod_test_0002 > > > +kmod_test_0003 > > > +kmod_test_0004 > > > +kmod_test_0005 > > > +kmod_test_0006 > > > +kmod_test_0007 > > > + > > > +#kmod_test_0008 > > > +#kmod_test_0009 > > > > While it's documented in the commit log, I think a short note for each > > disabled test should be added here too. > > Will do, thanks so much for the review! As I added test 0008's reason for why I think it fails I realized that the reason the test can sometimes fail is very different than test 0009 which is for get_fs_type(). You see get_fs_type() hammers kmod concurrent since we don't have an alias check and moprobe calling fs-xfs for instance does not catch that the module is already loaded so it delays the get_fs_type() call and so the __request_module() call, hogging up its kmod concurrent increment. For direct request_module() calls we don't have the alias issue, but since we don't check if a module is loaded prior to calling userspace (I now have a fix for this, reducing this latency does help) it means there are often times the chances we will pour in tons of requests without them getting processed and go over the concurrent limit. I've added a clutch into __request_module() then so instead of just failing we first check if we're at a threshold (say about 1/4 away from limit) and if so we let a few threads breath, until they are done. This fixes *both* test cases without much code changes, however as I've noted in other threads, this is not the only issue to address. Luis -- To unsubscribe from this list: send the line "unsubscribe linux-kselftest" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
