Updating the status on these changes:
One comment questioned whether or not a version bump was required, and
I've more-or-less convinced myself it is at least desired. While
properly-working modules from the pre-update will continue to work on a
post-update kernel, the reverse is not
Paul Goyette p...@whooppee.com wrote:
Following up on all the various comments from jmcneil@, pooka@, and
rmind@, I've attached a revised set of diffs. The most significant
change between this and the previous revision is the separation of the
kernconfig_lock_*() stuff into a separate
On Thu, 5 Aug 2010, Paul Goyette wrote:
OK, got it.
I'll have another set of patches in a few days.
Well, it was a slow day at the office today, so I found some time to
work on this! :)
Attached is the latest version of this change. For simplicity, I have
broken the patches up into
Hello,
Paul Goyette p...@whooppee.com wrote:
Well, it was a slow day at the office today, so I found some time to
work on this! :)
Attached is the latest version of this change. For simplicity, I have
broken the patches up into five separate groups:
...
Few comments on the patch:
On Fri, 6 Aug 2010, Mindaugas Rasiukevicius wrote:
Hello,
Paul Goyette p...@whooppee.com wrote:
Well, it was a slow day at the office today, so I found some time to
work on this! :)
Attached is the latest version of this change. For simplicity, I have
broken the patches up into five
On Thu, 5 Aug 2010, Paul Goyette wrote:
On Thu, 5 Aug 2010, Paul Goyette wrote:
OK, got it.
I'll have another set of patches in a few days.
Well, it was a slow day at the office today, so I found some time to work on
this! :)
Attached is the latest version of this change. For
OK, got it.
I'll have another set of patches in a few days.
On Thu, 5 Aug 2010, Andrew Doran wrote:
On Wed, Aug 04, 2010 at 10:28:56AM -0700, Paul Goyette wrote:
On Wed, 4 Aug 2010, Andrew Doran wrote:
Sorry for not replying sooner.
Please don't add a generic recursive lock facility, it
On Wed, 4 Aug 2010, Andrew Doran wrote:
Sorry for not replying sooner.
Please don't add a generic recursive lock facility, it would be abused.
Yeah! My current patches have no new generic facilities at all.
I'd like something roughly like the following. I think this should
also cover
On Tue, Aug 3, 2010 at 3:03 PM, Paul Goyette p...@whooppee.com wrote:
On Mon, 2 Aug 2010, Paul Goyette wrote:
On Tue, 3 Aug 2010, matthew green wrote:
i think this looks good enough. wait for some others eyes, though.
No hurry. I'm going to try to add a new atf test case for the recursive
On Tue, 3 Aug 2010, haad wrote:
Well, still no hurry, but it would be nice if some additional eyes were
pointed this way. I've got the recursive-module-load test case added
to the existing tests/modules/ stuff (and it works)!
BTW, since this is changing the kernel ABI, I guess it will need a
According to the mutex(9) man page:
mutex_owned(mtx)
For adaptive mutexes, return non-zero if the current LWP holds
the mutex. ...
this is an incomplete reading of the manual page, and you can not
use mutex_owned() the way you are trying to (regardless of what
pooka
On Aug,Sunday 1 2010, at 3:53 PM, Antti Kantee wrote:
On Sun Aug 01 2010 at 06:10:07 -0700, Paul Goyette wrote:
One question: Since an adaptive kmutex_t already includes an owner
field, would we really need to have another copy of it in the rmutex_t
structure?
Good point. I think it's
On Sun, 1 Aug 2010, Eduardo Horvath wrote:
Why is there a need to hold a mutex across module loading instead of, say,
having a reference count tht's protected by a mutex? I thought holding a
mutex across a potentially blocking operation such as a module load is
considered bad practice.
On Mon, 2 Aug 2010, Adam Hamsik wrote:
On Aug,Monday 2 2010, at 4:27 AM, Paul Goyette wrote:
On Sun, 1 Aug 2010, John Nemeth wrote:
I'm thinking the acquisition of module_lock should be pushed into
module_autoload() instead of having the caller acquire it for this very
reason. It makes
On Mon Aug 02 2010 at 16:30:03 +1000, matthew green wrote:
this is an incomplete reading of the manual page, and you can not
use mutex_owned() the way you are trying to (regardless of what
pooka posted.) you can't even using it in various forms of assertions
safely. from the man page:
On Mon, 2 Aug 2010, Antti Kantee wrote:
On Mon Aug 02 2010 at 16:30:03 +1000, matthew green wrote:
this is an incomplete reading of the manual page, and you can not
use mutex_owned() the way you are trying to (regardless of what
pooka posted.) you can't even using it in various forms of
On Aug,Monday 2 2010, at 4:27 AM, Paul Goyette wrote:
On Sun, 1 Aug 2010, John Nemeth wrote:
I'm thinking the acquisition of module_lock should be pushed into
module_autoload() instead of having the caller acquire it for this very
reason. It makes it hard to change the way locking
On Mon, 2 Aug 2010, Paul Goyette wrote:
Yes, the cv mechanism, coupled with changing the semantics of
module_autoload() (to not require the caller to obtain the lock) makes good
sense here. I'm working on it and I should be able to post new diffs
tomorrow.
OK, it's time for Round #3!
On Mon Aug 02 2010 at 16:30:03 +1000, matthew green wrote:
this is an incomplete reading of the manual page, and you can not
use mutex_owned() the way you are trying to (regardless of what
pooka posted.) you can't even using it in various forms of assertions
safely. from the man page:
The most recent verion of this patch set was recently posted, and it
uses the cv_wait() mechanism suggested by Adam Hamsik.
This discussion is still very intresting, but perhaps we could start a
new thread (or at least change the subject line)? :)
On Tue, 3 Aug 2010, matthew green wrote:
On Tue Aug 03 2010 at 02:17:43 +1000, matthew green wrote:
Now, on to sensible stuff. I'm quite certain that warning was written
to make people avoid writing bad code without understanding locking --
if you need to used mutex_owned() to decide if you should lock a normal
mutex, your code
On Sat Jul 31 2010 at 15:48:26 -0700, Paul Goyette wrote:
If modload-from-modcmd is found necessary, sounds more like a case for
the infamous recursive lock.
Recursive lock is the way to go. I think the same lock should also cover
all device configuration activites (i.e. autoconf) and any
On Sun, 1 Aug 2010, Antti Kantee wrote:
Well, folks, here is a first pass recursive locks! The attached diffs
are against -current as of a few minutes ago.
Oh, heh, I thought we have recursive lock support. But with that gone
from the vfs locks, I guess not (apart from the kernel lock ;).
On Sun Aug 01 2010 at 06:10:07 -0700, Paul Goyette wrote:
One question: Since an adaptive kmutex_t already includes an owner
field, would we really need to have another copy of it in the rmutex_t
structure?
Good point. I think it's ok to do:
if (mutex_owned(mtx))
cnt++
else
On Sun, 1 Aug 2010, Paul Goyette wrote:
Good point, and it will be a lot less work, too! :) And it solves the
problem of not permitting a rmutex being used with condvars.
One question: Since an adaptive kmutex_t already includes an owner field,
would we really need to have another copy
On Sun, 1 Aug 2010, Antti Kantee wrote:
I'm not sure if it's a good idea to change the size of kmutex_t. I
guess plenty of data structures have carefully been adjusted by hand
to its size and I don't know of any automatic way to recalculate that
stuff.
Even if not, since this is the only user
On Sun, Aug 01, 2010 at 05:17:01PM -0700, Paul Goyette wrote:
[...]
+void
+rmutex_enter(rmutex_t *rmtx)
+{
+ if (mutex_owned(rmtx-rmtx_mtx)) {
+ rmtx-rmtx_recurse++;
+ KASSERT(rmtx-rmtx_recurse != 0);
+ } else {
+ mutex_enter(rmtx-rmtx_mtx);
+
On Mon, 2 Aug 2010, Quentin Garnier wrote:
+int
+rmutex_tryenter(rmutex_t *rmtx)
+{
+ int rv = 1;
+
+ if (mutex_owned(rmtx-rmtx_mtx)) {
+ rmtx-rmtx_recurse++;
+ KASSERT(rmtx-rmtx_recurse != 0);
+ } else if ((rv = mutex_tryenter(rmtx-rmtx_mtx)) != 0)
On Sun, Aug 01, 2010 at 06:05:20PM -0700, Paul Goyette wrote:
On Mon, 2 Aug 2010, Quentin Garnier wrote:
+int
+rmutex_tryenter(rmutex_t *rmtx)
+{
+ int rv = 1;
+
+ if (mutex_owned(rmtx-rmtx_mtx)) {
+ rmtx-rmtx_recurse++;
+ KASSERT(rmtx-rmtx_recurse != 0);
+ }
On Sun, 1 Aug 2010, John Nemeth wrote:
I'm thinking the acquisition of module_lock should be pushed into
module_autoload() instead of having the caller acquire it for this very
reason. It makes it hard to change the way locking works in the
MODULAR code if you expect the caller to acquire
On Mon, 2 Aug 2010, Masao Uebayashi wrote:
mutex_owned(mtx)
...
mutex_owned() is provided for making diagnostic checks to verify
that a lock is held. For example:
KASSERT(mutex_owned(driver_lock));
It should not be used to make
On Wed, 28 Jul 2010, Andrew Doran wrote:
it seems to me the root problem is that module_mutex is held while
calling into the module startup routines.
thus, the right solution is to remove this requirement.
Yes, that's what is needed.
I'm far from convinced that's a good idea. First, it
On Wed, 28 Jul 2010, Andrew Doran wrote:
If modload-from-modcmd is found necessary, sounds more like a case for
the infamous recursive lock.
Recursive lock is the way to go. I think the same lock should also cover
all device configuration activites (i.e. autoconf) and any other
heavy lifting
On Mon, Jul 26, 2010 at 02:49:56PM +0300, Antti Kantee wrote:
On Sun Jul 25 2010 at 15:17:29 -0700, Paul Goyette wrote:
On Mon, 26 Jul 2010, matthew green wrote:
it seems to me the root problem is that module_mutex is held while
calling into the module startup routines.
thus, the
On Sun Jul 25 2010 at 15:17:29 -0700, Paul Goyette wrote:
On Mon, 26 Jul 2010, matthew green wrote:
it seems to me the root problem is that module_mutex is held while
calling into the module startup routines.
thus, the right solution is to remove this requirement.
Yes, that's what is
On Mon, 26 Jul 2010, der Mouse wrote:
We have a modular device driver, let's call it xxxmod. [...] It []
might attempt to use an optional module (e.g., zzzverbose) to print
some device attachment messages.
First, a required module cannot be optional. If the desired module
is not present,
Consider the following scenario:
We have a modular device driver, let's call it xxxmod. The driver's
xxx_modcmd(MODULE_CMD_INIT, ...) routine handles all of its initialization,
including interaction with autoconf(9). It therefore might attempt to use
an optional module (e.g., zzzverbose) to
On Mon, 26 Jul 2010, matthew green wrote:
it seems to me the root problem is that module_mutex is held while
calling into the module startup routines.
thus, the right solution is to remove this requirement.
Yes, that's what is needed.
But it sounds a lot simpler than it is.
On Mon, Jul 26, 2010 at 06:41:11AM +1000, matthew green wrote:
it seems to me the root problem is that module_mutex is held while
calling into the module startup routines.
Here is one related question: is it ensured that the module lock is dropped
immediately after a modular device driver
39 matches
Mail list logo
