Re: [patch] Minor PAGG attach/detach semantic change for 2.6.11

Hi,

Erik Jacobson wrote:
>>frequentrly-read pass, such as SELinux's Access Vector Cache(AVC).
>>But it's not omnipotence, and it restricts write methodology.
> 
> The feeling I had is that most users of pnotify won't be writing super-often.
> This is a generalization that may be incorrect.  Taking Job as an example,
> once the process is made part of a job, not much usually happens in terms of
> adjusting the data pointer associated with the task struct until Job is done.
> 
> I could imagine there may be things this isn't the case for, then the writes
> will be a penalty possibly.

In fork-hook, scaning the parent's list is indeed read-only.
But exit-hook contains writing-operations like list_del_rcu(),
thus something locking is required.
Probably, the ratio of readonly-pass and writable-pass might be
about 49.9%:50.1%. (Although I didn't actually measure it.)

# BTW, more than 99.9% is read-only pass in SELinux's case.
# see, /selinux/avc/cache_stats

In addition, you must revise whole job's implementation. For example,
job_dispatch_attachpid() must be written by using list_update_rcu().
RCU rules affects an implementation of pnotify's clients widely.
Is it appropriate as a general framework widely used ?

>>I have attention to another respect. The current pnotify implementation
>>requires to hold pnotify_event_list_sem before calling pnotify_get_events().
> 
> 
> As I recall, that code only would happen at most twice in the life of a kernel
> module, right?  The only time the init function pointer would fire, if it's
> present, is at pnotify_register time.  A similar piece of code happens at
> unregister time I think.  I guess I'm wondering if this happens enough to
> worry about?  Please let me know if I missed your entire point.

When anyone tries to associate a job with a running multithread-process,
it's required to scan for each thread in this process under
read_lock(&tasklist_lock), because job is an aggregation of processes,
not an aggregation of threads.

In SGI's JOB (job-2.6.13-patch), job_dispatch_attachpid() associate
a task_struct specified by PID with a existing job, but it does not
throw siblings of this task into the job. Therefore, this implementation
allow a part of the thread in this process to belong to defferent job.

e.g)
[BEFORE]
task-X1(PID=100,TGID=100) -- job-Alpha
task-X2(PID=101,TGID=100) -- job-Alpha
task-X3(PID=102,TGID=100) -- job-Alpha

  -> job_dispatch_attachpid(PID=100, job-Beta)

[AFTER]
task-X1(PID=100,TGID=100) -- job-Beta  <-- Same Process belongs
task-X2(PID=101,TGID=100) -- job-Alpha <-- to different job ???
task-X3(PID=102,TGID=100) -- job-Alpha

Do I have any misunderstandings ?

Because of this, while_each_thread(){...} loop under 'tasklist_lock'
is also necessary other than initialization or unregistering.

A similar problem will happen on detaching job procedure, I think.

Thanks,
-- 
Linux Promotion Center, NEC
KaiGai Kohei <[email protected]>