All of lore.kernel.org
 help / color / mirror / Atom feed
From: Petr Pavlu <petr.pavlu@suse.com>
To: Lucas De Marchi <lucas.demarchi@intel.com>,
	Luis Chamberlain <mcgrof@kernel.org>
Cc: Greg KH <gregkh@linuxfoundation.org>,
	david@redhat.com, patches@lists.linux.dev,
	linux-modules@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, pmladek@suse.com,
	prarit@redhat.com, torvalds@linux-foundation.org,
	rafael@kernel.org, christophe.leroy@csgroup.eu,
	tglx@linutronix.de, peterz@infradead.org, song@kernel.org,
	rppt@kernel.org, dave@stgolabs.net, willy@infradead.org,
	vbabka@suse.cz, mhocko@suse.com, dave.hansen@linux.intel.com,
	colin.i.king@gmail.com, jim.cromie@gmail.com,
	catalin.marinas@arm.com, jbaron@akamai.com,
	rick.p.edgecombe@intel.com, j.granados@samsung.com
Subject: Re: [PATCH] module: add debugging auto-load duplicate module support
Date: Wed, 26 Apr 2023 12:13:07 +0200	[thread overview]
Message-ID: <23bd0ce6-ef78-1cd8-1f21-0e706a00424a@suse.com> (raw)
In-Reply-To: <wjgsfhr642ec2ly24tsdqb5a3hlhvsyxknyajqql4zziqemrwh@w5rdsmxuownn>

On 4/21/23 20:31, Lucas De Marchi wrote:
> On Fri, Apr 21, 2023 at 10:38:49AM -0700, Luis Chamberlain wrote:
[...]
>>> libkmod only skips the call if the module is already in
>>> the live state.
>>
>> It can do better, it can converge requests to avoid a kernel_read*()
>>from using vmalloc space. Note that this was not well known before,
>> but now it is clear.
> 
> in userspace, if using the same context and using init_module() rather
> than finit_module(), I **guess** we would have a similar thing due to
> the memory pool for modules: we don't read the module again. That is not
> true for finit_module() though as we just open and pass the fd.
> 
>>
>> I realize though that this could mean sharing a context between all
>> loads thoughs in udev, and such a change could take significant time
>> and review to complete.
> 
> But there is only one context. There aren't multiple paralell requests
> from multiple sources. Probably need to Cc someone still changing
> udev's builtin...  but from a quick look, from what I remember about
> that the last time I touched it and without data to prove me wrong,
> it seems we are not looking at the right problem space to come up with a
> solution.

My understanding is that udev workers are forked. An initial kmod context is
created by the main udevd process but no sharing happens after the fork. It
means that the mentioned memory pool logic doesn't really kick in.

Multiple parallel load requests come from multiple udev workers, for instance,
each handling an udev event for one CPU device and making the exactly same
requests as all others are doing at the same time.

The optimization idea would be to recognize these duplicate requests at the
udevd/kmod level and converge them.

Cheers,
Petr

      parent reply	other threads:[~2023-04-26 10:13 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-18 20:46 [PATCH] module: add debugging auto-load duplicate module support Luis Chamberlain
2023-04-19  7:15 ` Greg KH
2023-04-19 23:32   ` Luis Chamberlain
2023-04-20  5:32     ` Greg KH
2023-04-20 21:03       ` Luis Chamberlain
2023-04-21 15:12         ` Greg KH
2023-04-21 16:42           ` Lucas De Marchi
2023-04-21 17:38             ` Luis Chamberlain
2023-04-21 18:31               ` Lucas De Marchi
2023-04-21 18:45                 ` Luis Chamberlain
2023-04-26 10:13                 ` Petr Pavlu [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=23bd0ce6-ef78-1cd8-1f21-0e706a00424a@suse.com \
    --to=petr.pavlu@suse.com \
    --cc=catalin.marinas@arm.com \
    --cc=christophe.leroy@csgroup.eu \
    --cc=colin.i.king@gmail.com \
    --cc=dave.hansen@linux.intel.com \
    --cc=dave@stgolabs.net \
    --cc=david@redhat.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=j.granados@samsung.com \
    --cc=jbaron@akamai.com \
    --cc=jim.cromie@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=lucas.demarchi@intel.com \
    --cc=mcgrof@kernel.org \
    --cc=mhocko@suse.com \
    --cc=patches@lists.linux.dev \
    --cc=peterz@infradead.org \
    --cc=pmladek@suse.com \
    --cc=prarit@redhat.com \
    --cc=rafael@kernel.org \
    --cc=rick.p.edgecombe@intel.com \
    --cc=rppt@kernel.org \
    --cc=song@kernel.org \
    --cc=tglx@linutronix.de \
    --cc=torvalds@linux-foundation.org \
    --cc=vbabka@suse.cz \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.