On Wed, 26 Sep 2007, Tejun Heo wrote:
> >> Hmmm... I might be missing something here. Who else can wake up a
> >> thread in uninterruptible sleep?
> >
> > In principle, anything can. There has never been any guarantee in the
> > kernel that a task sleeping on a waitqueue will remain asleep
On Wed, 26 Sep 2007, Tejun Heo wrote:
Hmmm... I might be missing something here. Who else can wake up a
thread in uninterruptible sleep?
In principle, anything can. There has never been any guarantee in the
kernel that a task sleeping on a waitqueue will remain asleep until
the
Rusty Russell wrote:
> On Wed, 2007-09-26 at 08:15 +0900, Tejun Heo wrote:
>> I have no problem with changing the condition check to loop but it would
>> be great if someone can point me to a code where this unexpected wake up
>> is used.
>
> This is one of those areas where we're conservative.
On Wed, 2007-09-26 at 08:15 +0900, Tejun Heo wrote:
> I have no problem with changing the condition check to loop but it would
> be great if someone can point me to a code where this unexpected wake up
> is used.
This is one of those areas where we're conservative. Historically there
have been
Alan Stern wrote:
> On Tue, 25 Sep 2007, Tejun Heo wrote:
>
>> Alan Stern wrote:
The unloading can proceed once module_unload_inhibit_cnt reaches zero.
An unloading thread only has to care about inhibition put in effect
before unloading has started, so there's no need to check
On Tue, 25 Sep 2007, Tejun Heo wrote:
> Alan Stern wrote:
> >> The unloading can proceed once module_unload_inhibit_cnt reaches zero.
> >> An unloading thread only has to care about inhibition put in effect
> >> before unloading has started, so there's no need to check again.
> >
> > You haven't
Alan Stern wrote:
>> The unloading can proceed once module_unload_inhibit_cnt reaches zero.
>> An unloading thread only has to care about inhibition put in effect
>> before unloading has started, so there's no need to check again.
>
> You haven't fully answered Jon's question. Suppose
>
On Tue, 25 Sep 2007, Tejun Heo wrote:
> Jonathan Corbet wrote:
> > Hi, Tejun,
> >
> > I was just looking over these changes...
> >
> >> + /* Don't proceed till inhibition is lifted. */
> >> + add_wait_queue(_unload_wait, );
> >> + set_current_state(TASK_UNINTERRUPTIBLE);
> >> + if
Rusty Russell wrote:
> On Tue, 2007-09-25 at 17:36 +0900, Tejun Heo wrote:
>> Hmmm There doesn't seem to any reason why the blocking should be
>> after calling ->exit(). And, yeah, it would be more useful and
>> intuitive if blocking happens before ->exit(). What do you think?
>
> *That* I
On Tue, 2007-09-25 at 17:36 +0900, Tejun Heo wrote:
> Hmmm There doesn't seem to any reason why the blocking should be
> after calling ->exit(). And, yeah, it would be more useful and
> intuitive if blocking happens before ->exit(). What do you think?
*That* I have no problem with.
I was
Tejun Heo wrote:
> Rusty Russell wrote:
>> Now, are you sure that calling cleanup_ccwgroup just after
>> device_unregister() works?
>>
>> static void __exit
>> cleanup_ccwgroup (void)
>> {
>> bus_unregister (_bus_type);
>> }
>
> It should. After ->exit() is called, there can't be any object
Rusty Russell wrote:
> On Tue, 2007-09-25 at 12:36 +0900, Tejun Heo wrote:
>> Rusty Russell wrote:
>>> As stated you cannot protect arbitrary code this way, as you are trying
>>> to do. I do not think you've broken any of the current code, but I
>>> cannot tell. You're certainly going to
On Tue, 25 Sep 2007 14:38:38 +1000,
Rusty Russell <[EMAIL PROTECTED]> wrote:
> Have you tested that *this* path works? Let's take your first change as
> an example:
>
> + mutex_lock(>reg_mutex);
> + __ccwgroup_remove_symlinks(gdev);
> + device_unregister(dev);
> +
On Tue, 25 Sep 2007 14:38:38 +1000,
Rusty Russell [EMAIL PROTECTED] wrote:
Have you tested that *this* path works? Let's take your first change as
an example:
+ mutex_lock(gdev-reg_mutex);
+ __ccwgroup_remove_symlinks(gdev);
+ device_unregister(dev);
+
Rusty Russell wrote:
On Tue, 2007-09-25 at 12:36 +0900, Tejun Heo wrote:
Rusty Russell wrote:
As stated you cannot protect arbitrary code this way, as you are trying
to do. I do not think you've broken any of the current code, but I
cannot tell. You're certainly going to surprise
Tejun Heo wrote:
Rusty Russell wrote:
Now, are you sure that calling cleanup_ccwgroup just after
device_unregister() works?
static void __exit
cleanup_ccwgroup (void)
{
bus_unregister (ccwgroup_bus_type);
}
It should. After -exit() is called, there can't be any object left
On Tue, 2007-09-25 at 17:36 +0900, Tejun Heo wrote:
Hmmm There doesn't seem to any reason why the blocking should be
after calling -exit(). And, yeah, it would be more useful and
intuitive if blocking happens before -exit(). What do you think?
*That* I have no problem with.
I was going
Rusty Russell wrote:
On Tue, 2007-09-25 at 17:36 +0900, Tejun Heo wrote:
Hmmm There doesn't seem to any reason why the blocking should be
after calling -exit(). And, yeah, it would be more useful and
intuitive if blocking happens before -exit(). What do you think?
*That* I have no
On Tue, 25 Sep 2007, Tejun Heo wrote:
Jonathan Corbet wrote:
Hi, Tejun,
I was just looking over these changes...
+ /* Don't proceed till inhibition is lifted. */
+ add_wait_queue(module_unload_wait, wait);
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ if
Alan Stern wrote:
The unloading can proceed once module_unload_inhibit_cnt reaches zero.
An unloading thread only has to care about inhibition put in effect
before unloading has started, so there's no need to check again.
You haven't fully answered Jon's question. Suppose
On Tue, 25 Sep 2007, Tejun Heo wrote:
Alan Stern wrote:
The unloading can proceed once module_unload_inhibit_cnt reaches zero.
An unloading thread only has to care about inhibition put in effect
before unloading has started, so there's no need to check again.
You haven't fully
Alan Stern wrote:
On Tue, 25 Sep 2007, Tejun Heo wrote:
Alan Stern wrote:
The unloading can proceed once module_unload_inhibit_cnt reaches zero.
An unloading thread only has to care about inhibition put in effect
before unloading has started, so there's no need to check again.
You haven't
On Wed, 2007-09-26 at 08:15 +0900, Tejun Heo wrote:
I have no problem with changing the condition check to loop but it would
be great if someone can point me to a code where this unexpected wake up
is used.
This is one of those areas where we're conservative. Historically there
have been
Rusty Russell wrote:
On Wed, 2007-09-26 at 08:15 +0900, Tejun Heo wrote:
I have no problem with changing the condition check to loop but it would
be great if someone can point me to a code where this unexpected wake up
is used.
This is one of those areas where we're conservative.
On Tue, 2007-09-25 at 12:36 +0900, Tejun Heo wrote:
> Rusty Russell wrote:
> > As stated you cannot protect arbitrary code this way, as you are trying
> > to do. I do not think you've broken any of the current code, but I
> > cannot tell. You're certainly going to surprise unsuspecting future
>
Rusty Russell wrote:
> As stated you cannot protect arbitrary code this way, as you are trying
> to do. I do not think you've broken any of the current code, but I
> cannot tell. You're certainly going to surprise unsuspecting future
> authors.
Can you elaborate a bit? Why can't it protect the
On Tue, 2007-09-25 at 11:39 +0900, Tejun Heo wrote:
> Rusty Russell wrote:
> >>> I really wonder if an explicit "kill_this_attribute()" is a better way
> >>> to go than this...
> >> I think this sort of temporary unload blocking would be useful for other
> >> cases like this.
> >
> > I hope not:
Rusty Russell wrote:
>>> I really wonder if an explicit "kill_this_attribute()" is a better way
>>> to go than this...
>> I think this sort of temporary unload blocking would be useful for other
>> cases like this.
>
> I hope not: this doesn't work in general. Calling into a module after
>
On Tue, 2007-09-25 at 10:40 +0900, Tejun Heo wrote:
> Rusty Russell wrote:
> > My concern is that you're dropping the module mutex around ->exit now.
> > I don't *think* this should matter, but it's worth considering.
>
> We always did that. Before the patch the code segment looked like the
>
Rusty Russell wrote:
> On Tue, 2007-09-25 at 08:18 +0900, Tejun Heo wrote:
>>> Given your description of this tool as a "sledgehammer," might it not be
>>> easier to just take and hold module_mutex for the duration of the unload
>>> block?
>> That would be easier but...
>>
>> * It would serialize
On Tue, 2007-09-25 at 08:18 +0900, Tejun Heo wrote:
> > Given your description of this tool as a "sledgehammer," might it not be
> > easier to just take and hold module_mutex for the duration of the unload
> > block?
>
> That would be easier but...
>
> * It would serialize users of the
Jonathan Corbet wrote:
> Hi, Tejun,
>
> I was just looking over these changes...
>
>> +/* Don't proceed till inhibition is lifted. */
>> +add_wait_queue(_unload_wait, );
>> +set_current_state(TASK_UNINTERRUPTIBLE);
>> +if (atomic_read(_unload_inhibit_cnt))
>> +
Hi, Tejun,
I was just looking over these changes...
> + /* Don't proceed till inhibition is lifted. */
> + add_wait_queue(_unload_wait, );
> + set_current_state(TASK_UNINTERRUPTIBLE);
> + if (atomic_read(_unload_inhibit_cnt))
> + schedule();
> +
Hi, Tejun,
I was just looking over these changes...
+ /* Don't proceed till inhibition is lifted. */
+ add_wait_queue(module_unload_wait, wait);
+ set_current_state(TASK_UNINTERRUPTIBLE);
+ if (atomic_read(module_unload_inhibit_cnt))
+ schedule();
+
Jonathan Corbet wrote:
Hi, Tejun,
I was just looking over these changes...
+/* Don't proceed till inhibition is lifted. */
+add_wait_queue(module_unload_wait, wait);
+set_current_state(TASK_UNINTERRUPTIBLE);
+if (atomic_read(module_unload_inhibit_cnt))
+
On Tue, 2007-09-25 at 08:18 +0900, Tejun Heo wrote:
Given your description of this tool as a sledgehammer, might it not be
easier to just take and hold module_mutex for the duration of the unload
block?
That would be easier but...
* It would serialize users of the sledgehammer.
* It
Rusty Russell wrote:
On Tue, 2007-09-25 at 08:18 +0900, Tejun Heo wrote:
Given your description of this tool as a sledgehammer, might it not be
easier to just take and hold module_mutex for the duration of the unload
block?
That would be easier but...
* It would serialize users of the
On Tue, 2007-09-25 at 10:40 +0900, Tejun Heo wrote:
Rusty Russell wrote:
My concern is that you're dropping the module mutex around -exit now.
I don't *think* this should matter, but it's worth considering.
We always did that. Before the patch the code segment looked like the
following.
Rusty Russell wrote:
I really wonder if an explicit kill_this_attribute() is a better way
to go than this...
I think this sort of temporary unload blocking would be useful for other
cases like this.
I hope not: this doesn't work in general. Calling into a module after
-exit has called
On Tue, 2007-09-25 at 11:39 +0900, Tejun Heo wrote:
Rusty Russell wrote:
I really wonder if an explicit kill_this_attribute() is a better way
to go than this...
I think this sort of temporary unload blocking would be useful for other
cases like this.
I hope not: this doesn't work in
Rusty Russell wrote:
As stated you cannot protect arbitrary code this way, as you are trying
to do. I do not think you've broken any of the current code, but I
cannot tell. You're certainly going to surprise unsuspecting future
authors.
Can you elaborate a bit? Why can't it protect the
On Tue, 2007-09-25 at 12:36 +0900, Tejun Heo wrote:
Rusty Russell wrote:
As stated you cannot protect arbitrary code this way, as you are trying
to do. I do not think you've broken any of the current code, but I
cannot tell. You're certainly going to surprise unsuspecting future
42 matches
Mail list logo
