LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: ebiederm@xmission.com (Eric W. Biederman)
To: Cedric Le Goater <clg@fr.ibm.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	containers@lists.osdl.org
Subject: Re: [patch -mm] update mq_notify to use a struct pid
Date: Mon, 11 Sep 2006 05:09:19 -0600	[thread overview]
Message-ID: <m1u03eacdc.fsf@ebiederm.dsl.xmission.com> (raw)
In-Reply-To: <450537B6.1020509@fr.ibm.com> (Cedric Le Goater's message of "Mon, 11 Sep 2006 12:17:26 +0200")

Cedric Le Goater <clg@fr.ibm.com> writes:

> Eric W. Biederman wrote:
>> Cedric Le Goater <clg@fr.ibm.com> writes:
>> 
>>> message queues can signal a process waiting for a message.
>>>
>>> this patch replaces the pid_t value with a struct pid to avoid pid wrap
>>> around problems.
>>>
>>> Signed-off-by: Cedric Le Goater <clg@fr.ibm.com>
>>> Cc: Eric Biederman <ebiederm@xmission.com>
>>> Cc: Andrew Morton <akpm@osdl.org>
>>> Cc: containers@lists.osdl.org
>> 
>> Signed-off-by: Eric Biederman <ebiederm@xmission.com>
>> 
>> I was just about to send out this patch in a couple more hours.
>
> Well, you did the same with the usb/devio.c and friends :)

Good.  The you should be familiar enough with it to review my patch
and make certain I didn't do anything stupid :)

>> So expect the fact we wrote the same code is a good sign :)
>
> How does oleg feel about it ? I've seen some long thread on possible race
> conditions with put_pid() and solutions with rcu. I didn't quite get all of
> it ... it will need another run for me.

Short.  Oleg felt it was a shame that locking was needed to use a
struct pid.

While parsing that I realized my second vt patch that deals with
vt_pid (the pid for console switching) has a subtle race, and
that patch needs to be reworked.

We confused each other. :)

> On the "pid_t to struct pid*" topic:
>
> * I started smbfs and realized it was useless.

Killing the user space part is useless?
I thought that is what I saw happening.

Of course I don't frequently mount smbfs.

> * in the following, the init process is being killed directly using 1. I'm
> not sure how useful it would be to use a struct pid. To begin with, may be
> they could use a :
>
> 	kill_init(int signum, int priv)

An interesting notion.  The other half of them use cad_pid.
Converting that is going to need some sysctl work, so I have been
ignoring it temporarily.

Filling in a struct pid through sysctl is extremely ugly at the
moment, plus cad_pid needs some locking.

> ./arch/mips/sgi-ip32/ip32-reset.c
> ./arch/powerpc/platforms/iseries/mf.c
> ./drivers/parisc/power.c
> ./drivers/char/snsc_event.c
> ./kernel/sys.c
> ./kernel/sysctl.c
> ./drivers/char/nwbutton.c
> ./drivers/s390/s390mach.c
>
> * some more drivers,
> * some more kthread to convert

Ok.  Time to exchange some status information, before I roll over and
go back to sleep.  



My patch todo list (almost a series file) currently looks like:
> n_r396r
> fs3270-Change-to-use-struct-pid.txt
> smbfs-Make-conn_pid-a-struct-pid.txt
> ncpfs-Use-struct-pid-to-track-the-userspace-watchdog-process.txt
> 
> Don-t-use-kill_pg-in-the-sunos-compatibility-code.txt
> 
> usbatm-use-kthread-api (I think I have this one)
I did usbatm mostly to figure out why kthread conversions seem
to be so hard, and got lucky this one wasn't too ugly.

> The-dvb_core-needs-to-use-the-kthread-api-not-kernel-threads.txt
> nfs-Note-we-need-to-start-using-the-kthreads-api.txt

dvb-core I have only started looking at.
nfs I noticed it is the svc stuff that matters.

usbatm, dvb-core, and nfs are the 3 kernel_thread users
that also use kill_proc, and thus are high on my immediate hit list.
 
> pid-Replace-session_of_pgrp-with-pgrp_in_current_session.txt
> pid-Use-struct-pid-for-talking-about-process-groups-in-exit.c.txt
> pid-Replace-is_orphaned_pgrp-with-is_current_pgrp_orphaned.txt
> 
> tty-Update-the-tty-layer-to-work-with-struct-pid.txt
I need to ensure I don't have a race with task->signal->tty_old_pgrp.
tty_old_pgrp is a weird notion that I haven't fully wrapped my head
around yet.

> pid-Remove-use-of-old-do_each_task_pid-while_each_task_pid.txt
> 
> Rewrite-kill_something_info-so-it-uses-newer-helpers.txt
> 
> pid-Remove-now-unused-do_each_task_pid-and-while_each_task_pid.txt
> Remove-the-now-unused-kill_pg-kill_pg_info-and-__kill_pg_info.txt
> 
> 
> pid-Better-tests-for-same-thread-group-membership.txt
> pid-Cleanup-the-pid-equality-tests.txt
> pid-Track-the-sending-pid-of-a-queued-signal.txt
> proc-Use-pid_nr-in-array.c-so-the-code-is-foobar-safe.txt
> 
> sysctl-Implement-get_data-put_data.txt
> 
> cad-pid (killing init)

Once the above list is processed none of the old none of the signal
functions that take a pid_t is needed anymore.
i.e. kill_proc, kill_pg, and do_each_task_pid will be removable.

I have at least a first draft of everything on my list except for the
kthread conversions which I just started messing with yesterday.   But
don't worry about beating me to something if you feel you have it
complete.  That just means I will have enough of a clue to review your
code :)

Eric


  reply	other threads:[~2006-09-11 11:10 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-09-08 16:39 Cedric Le Goater
2006-09-09  2:39 ` Eric W. Biederman
2006-09-11 10:17   ` Cedric Le Goater
2006-09-11 11:09     ` Eric W. Biederman [this message]
2006-09-11 14:05       ` Cedric Le Goater
2006-09-11 19:01         ` Eric W. Biederman
2006-09-11 21:53           ` Cedric Le Goater
2006-09-12  1:22             ` Eric W. Biederman
2006-09-12 15:37               ` Cedric Le Goater
2006-09-12 16:03                 ` Eric W. Biederman
2006-09-12 11:05           ` Herbert Poetzl
2006-09-12 15:14             ` Eric W. Biederman
2006-09-14 20:01             ` [PATCH/RFC] kthread API conversion for dvb_frontend and av7110 Herbert Poetzl
2006-09-14 21:07               ` Cedric Le Goater
2006-09-14 22:10                 ` Herbert Poetzl
2006-11-17  1:50                   ` Andrew de Quincey
2006-11-22 14:56                     ` [Devel] " Cedric Le Goater
2006-11-22 21:32                       ` [v4l-dvb-maintainer] " Andrew de Quincey
2007-01-24 15:47                       ` Cedric Le Goater
2006-12-12 22:58                     ` Eric W. Biederman
2006-12-12 23:13                       ` Herbert Poetzl
2006-12-13 15:55                         ` [Devel] " Cedric Le Goater
2006-09-11 15:48     ` [patch -mm] update mq_notify to use a struct pid Oleg Nesterov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m1u03eacdc.fsf@ebiederm.dsl.xmission.com \
    --to=ebiederm@xmission.com \
    --cc=clg@fr.ibm.com \
    --cc=containers@lists.osdl.org \
    --cc=linux-kernel@vger.kernel.org \
    --subject='Re: [patch -mm] update mq_notify to use a struct pid' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).