pagg
[Top] [All Lists]

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

To: Kaigai Kohei <kaigai@xxxxxxxxxxxxx>
Subject: Re: [patch] Minor PAGG attach/detach semantic change for 2.6.11
From: Erik Jacobson <erikj@xxxxxxx>
Date: Thu, 29 Sep 2005 09:47:38 -0500
Cc: Erik Jacobson <erikj@xxxxxxx>, Kingsley Cheung <kingsley@xxxxxxxxxx>, pagg@xxxxxxxxxxx, tonyt@xxxxxxxxxx, paulmck@xxxxxxxxxx
In-reply-to: <433B80B6.2010604@ak.jp.nec.com>
References: <20050617014512.GA10285@aurema.com> <20050927201020.GA30433@sgi.com> <433A7FE4.5040109@ak.jp.nec.com> <20050928141831.GA24110@sgi.com> <433B80B6.2010604@ak.jp.nec.com>
Sender: pagg-bounce@xxxxxxxxxxx
User-agent: Mutt/1.5.6i
> 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 ?

[ Correctedion in above paragraph from list_update_rcu to 
  list_replace_rcu noted ]

Hi.  I'm confused with the above paragraph.  My initial plan was only to
convert how job uses pnotify to be rcu-aware.  So this mainly relates to
the pnotify_subscriber_list.  

job_dispatch_attachpid adds the kernel module as a subscriber to the given
process.  It doesn't access the list directly but does (in my updated version)
have the write lock held and the rcu_read_lock/unlock calls.  But it calls
pnotify_subscribe to actually add the kernel module to the subscriber list
of the task.

pnotify_subscribe uses list_add_tail_rcu to do this.

Maybe there are other examples that need adjustments?  On the download site,
in pnotify-test, you'll see my first pass at an RCU versoin of pnotify.
In job-test, you'll see a first cut at a rcu-pnotify version of Job.

> >>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.

OK; I think I see some of what you're saying here now.  If it isn't 
urgent, let's defer this until we know what's happening with pnotify.

I guess I'm most interested in any logic problems I have in the way I used
pnotify RCU with job, not problems with how Job might have a flaw that has 
always been there.  Let's address those later.

Regarding your comments on if RCU is the right answer for pnotify - I don't
really know, it makes me uncomfortable but I need to be sure that feeling
is for a valid reason, not just because I'm new to RCU.

In some performance tests I ran yesterday, it seems (on a 2p ia64 altix
box) that system performance as mesaured by AIM and by a fork-bomb type
test are nearly identical between 2.6.14-rc2, 2.6.14-rc2 with RCU version
of pnotify, and 2.6.14-rc2 with old fully rwsem version of pnotify.

The version of job with RCU pnotify was changed to use RCU protections.
Also included was the keyring proof of concept where keyring makes use of
pnotify.  So, there were two pnotify users in the the two tests with
pnotify - keyring and job.  The AIM tests were run where the process
launching the test was part of a job.  In the stock 2.6.14-rc2 test,
a stock version of keyrings was enabled.

If there is no measurable difference, it seems that RCU might not be the
best answer because we're increasing complexity for no good resaon.

I'll post more formal numbers a bit later.

<Prev in Thread] Current Thread [Next in Thread>