Hi,
Erik Jacobson wrote:
I fixed this in the RCU version of pnotify I'm working per the lse-tech
community discussion - thanks for the reminder the other day (in a non-list
email).
If the RCU version crashes and burns for some reason and we go back to
the non-CUu one, I'll need to make the fix there too. The function now
looks like this. I hope this is what you had in mind (untested as of
this moment).
In my understanding, any _write_ operations can not be implemented
without locking, even if we can use RCU.
(In addition, RCU conscious writing/update style is required.)
For example, pnotify permits to attach a new pnotify_subscriber object
to another task. If someone calls pnotify_subscribe() for other task
which is doing fork(), there is a possibility to break the
pnotify_subscriber_list of victim task.
Therefore, procedures with updates such __pnotify_fork() should be
serialized by somethig locking. RCU is so effective for seldom-write/
frequentrly-read pass, such as SELinux's Access Vector Cache(AVC).
But it's not omnipotence, and it restricts write methodology.
In the past, I made a proposition of applying RCU for PAGG. But it might
be inappropriate for pnotify/PAGG as a general framework.
I have attention to another respect. The current pnotify implementation
requires to hold pnotify_event_list_sem before calling pnotify_get_events().
Threfore, we must repeat read_lock/unlock(&tasklist_lock) on
do_each_thread()/while_each_thread() loop as follows:
----------------------------
read_lock(&tasklist_lock);
do_each_thread(g, p) {
get_task_struct(p);
read_unlock(&tasklist_lock);
down_read(&p->pnotify_subscriber_list_sem);
subscriber = pnotify_get_subscriber(p, events->name);
:
up_read(&p->pnotify_subscriber_list_sem);
read_lock(&tasklist_lock);
<< checking, p is dead or not ? >>
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
----------------------------
I'm happy, if pnotify_subscriber_list would be protected by rwlock.
If rwlock is used, we can not implement pnotify_subscribe() with current
spec. But is it impossible to prepare pnotify_subscribe_atomic() or
pnotify_subscribe_bind() which associates task_struct with pre-allocated
pnotify_events object ?
---- in rwlock world :-) ---
read_lock(&tasklist_lock);
do_each_thread(g, p) {
read_lock(&p->pnotify_subscriber_list_rwlock);
subscriber = pnotify_get_subscriber(p, events->name);
:
read_unlock(&p->pnotify_subscriber_list_rwlock);
} while_each_thread(g, p);
read_unlock(&tasklist_lock);
----------------------------
Thanks,
/**
* __pnotify_fork - Add kernel module subscriber to same subscribers as parent
* @to_task: The child task that will inherit the parent's subscribers
* @from_task: The parent task
*
* Used to attach a new task to the same subscribers the parent has in its
* subscriber list.
*
* The "from" argument is the parent task. The "to" argument is the child
* task.
*
* See Documentation/pnotify.txt for details on
* how to handle return codes from the attach function pointer.
*
* Locking: The to_task is currently in-construction, so we don't
* need to worry about write-locks. We do need to be sure the parent's
* subscriber list, which we copy here, doesn't go away on us. This is
* done via RCU.
*
*/
int
__pnotify_fork(struct task_struct *to_task, struct task_struct *from_task)
{
struct pnotify_subscriber *from_subscriber;
int ret;
/* We need to be sure the parent's list we copy from doesn't disappear
*/
rcu_read_lock();
list_for_each_entry_rcu(from_subscriber,
&from_task->pnotify_subscriber_list, entry) {
struct pnotify_subscriber *to_subscriber = NULL;
to_subscriber = pnotify_subscribe(to_task,
from_subscriber->events);
if (!to_subscriber) {
ret=-ENOMEM;
__pnotify_exit(to_task);
rcu_read_unlock();
return ret;
}
ret = to_subscriber->events->fork(to_task, to_subscriber,
from_subscriber->data);
rcu_read_unlock(); /* no more to do with the parent's data */
rcu_read_unlovk(); should be deployed outside of the
list_for_each_entry_rcu(){...}.
if (ret < 0) {
/* Propagates to copy_process as a fork failure */
/* No __pnotify_exit because there is one in the
failure path
* for copy_process in fork.c */
return ret; /* Fork failure */
}
else if (ret > 0) {
/* Success, but fork function pointer in the
pnotify_events structure
* doesn't want the kenrel module subscribed */
/* Again, this is the in-construction-child so no write
lock */
pnotify_unsubscribe(to_subscriber);
}
}
return 0; /* success */
}
--
Linux Promotion Center, NEC
KaiGai Kohei <kaigai@xxxxxxxxxxxxx>
|