linux-modules.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Lucas De Marchi <lucas.demarchi@intel.com>
To: Luis Chamberlain <mcgrof@kernel.org>
Cc: linux-modules@vger.kernel.org, live-patching@vger.kernel.org,
	fstests@vger.kernel.org, linux-block@vger.kernel.org,
	hare@suse.de, dgilbert@interlog.com, jeyu@kernel.org,
	osandov@fb.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] libkmod-module: add support for a patient module removal option
Date: Wed, 4 Aug 2021 11:47:20 -0700	[thread overview]
Message-ID: <20210804184720.z27u5aymcl5hzqgh@ldmartin-desk2> (raw)
In-Reply-To: <YQrVY8Wxb026TDWN@bombadil.infradead.org>

On Wed, Aug 04, 2021 at 10:58:59AM -0700, Luis Chamberlain wrote:
>On Tue, Aug 03, 2021 at 01:24:17PM -0700, Luis Chamberlain wrote:
>> + diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
><-- snip -->
>> +		ERR(mod->ctx, "%s refcnt is %ld waiting for it to become 0\n", mod->name, refcnt);
>
>OK after running many tests with this I think we need to just expand
>this so that the error message only applies when -v is passed to
>modprobe, otherwise we get the print message every time, and using
>INFO() doesn't cut it, given the next priority level available to
>the library is LOG_INFO (6) and when modprobe -v is passed we set the
>log level to LOG_NOTICE (5), so we need a new NOTICE(). I'll send a v2
>with that included as a separate patch.

Or maybe move the sleep to modprobe instead of doing it in the
library?  The sleep(1) seems like an arbitrary number to be introduced
in the lib.

Since kernfs is pollable, maybe we could rather introduce an API to
return the pid in which the application has to wait for and then the
application can use whatever it wants to poll, including controlling a
timeout.

I'm saying this because sleep(1) may be all fine for modprobe, but for
other applications using libkmod it may not play well with their mainloops
(and they may want to control both granularity of the sleep and a max
timeout).

thanks
Lucas De Marchi

>
>  Luis

  reply	other threads:[~2021-08-04 18:47 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-03 20:24 [PATCH] libkmod-module: add support for a patient module removal option Luis Chamberlain
2021-08-04 17:58 ` Luis Chamberlain
2021-08-04 18:47   ` Lucas De Marchi [this message]
2021-08-06 21:45     ` Luis Chamberlain

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=20210804184720.z27u5aymcl5hzqgh@ldmartin-desk2 \
    --to=lucas.demarchi@intel.com \
    --cc=dgilbert@interlog.com \
    --cc=fstests@vger.kernel.org \
    --cc=hare@suse.de \
    --cc=jeyu@kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-modules@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=osandov@fb.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).