[PATCH] coda: alloc_upcall: s/task_pgrp_nr/task_pgrp_vnr/

[PATCH] coda: alloc_upcall: s/task_pgrp_nr/task_pgrp_vnr/

[Oleg Nesterov]

Needs an ack from maintaner, I do not know where coda_in_hdr->pgid is used.

alloc_upcall() is the last user of deprecated task_pgrp_nr(), change it
to use task_pgrp_vnr() instead. Or it really needs the pid_t from the global
namespace?

Signed-off-by: Oleg Nesterov <oleg@redhat.com>

--- CUR/fs/coda/upcall.c~3_CODA	2009-01-12 23:07:46.000000000 +0100
+++ CUR/fs/coda/upcall.c	2009-01-18 08:21:26.000000000 +0100
@@ -51,7 +51,7 @@ static void *alloc_upcall(int opcode, in
 
         inp->ih.opcode = opcode;
 	inp->ih.pid = current->pid;
-	inp->ih.pgid = task_pgrp_nr(current);
+	inp->ih.pgid = task_pgrp_vnr(current);
 	inp->ih.uid = current_fsuid();
 
 	return (void*)inp;

Re: [PATCH] coda: alloc_upcall: s/task_pgrp_nr/task_pgrp_vnr/

[Jan Harkes]

On Sun, Jan 18, 2009 at 08:34:53AM +0100, Oleg Nesterov wrote:
> Needs an ack from maintaner, I do not know where coda_in_hdr->pgid is used.

It is used to uniquely identify a process and any of it children during
conflict resolution.

When a conflict is detected, all accesses to the inconsistent object are
blocked. A special resolver process is forked off by the cache manager
and this is run in a new process group and only accesses from processes
in this group are allowed. The resolver process (or any of it's children)
compare the conflicting replicas, and ideally resolve the inconsistency
after which normal accesses are unblocked.

So yes this should not a per namespace thing, but also not a process
specific pid, the resolver forks off different helper processes
depending on the type of files that are involved in the conflict, i.e.
mbox files require different merge strategy compared to opendocument
files.

I'm not sure what you are trying to do.

Jan

Re: [PATCH] coda: alloc_upcall: s/task_pgrp_nr/task_pgrp_vnr/

[Eric W. Biederman]

Jan Harkes <jaharkes@cs.cmu.edu> writes:

> On Sun, Jan 18, 2009 at 08:34:53AM +0100, Oleg Nesterov wrote:
>> Needs an ack from maintaner, I do not know where coda_in_hdr->pgid is used.
>
> It is used to uniquely identify a process and any of it children during
> conflict resolution.
>
> When a conflict is detected, all accesses to the inconsistent object are
> blocked. A special resolver process is forked off by the cache manager
> and this is run in a new process group and only accesses from processes
> in this group are allowed. The resolver process (or any of it's children)
> compare the conflicting replicas, and ideally resolve the inconsistency
> after which normal accesses are unblocked.
>
> So yes this should not a per namespace thing, but also not a process
> specific pid, the resolver forks off different helper processes
> depending on the type of files that are involved in the conflict, i.e.
> mbox files require different merge strategy compared to opendocument
> files.
>
> I'm not sure what you are trying to do.

We currently have two pid data types in the kernel.
pid_t and struct pid *.

pid_t's are the tokens we pass to user space to talk about a 
process, a process group or a session.

struct pid pointers are used internally to the kernel, are
reference counted, are not susceptible to pid wrap around,
and are generally faster to use for sending signals or other
tasks that require looking up a process.

With the introduction of the pid namespaces the difference between
pid_t's and struct pid has become even more important.  Because
based on the pid namespace you are in a given struct pid will have
a different pid_t value.  So internally we are moving as much
as possible to using struct pid pointers.


Oleg is in the process of cleaning up some of the transition code
and we just need to convert the last couple of pieces of code
so we can do that.


In the case of coda I'm assuming it is the user space daemon that
decides if the access is from the resolver process group or not?

That the user space filesystem code does the blocking based on which
process group you are in.

In that case it looks like what needs to happen is that alloc_upcall
needs to know which pid namespace your user space daemon is in.
Probably grab the pid namespace at either mount or connect time (is
there a difference).

Then since I believe the values in the upcall go straight to the user
space daemon we should do roughly:

inp->in.pid  = task_pid_nr_ns(&fs_daemon_pidns, current);
inp->in.pgid = task_pgrp_nr_ns(&fs_daemon_pidns, current);

Does that make sense?

Eric

Re: [PATCH] coda: alloc_upcall: s/task_pgrp_nr/task_pgrp_vnr/

[Oleg Nesterov]

On 01/21, Jan Harkes wrote:
>
> On Sun, Jan 18, 2009 at 08:34:53AM +0100, Oleg Nesterov wrote:
> > Needs an ack from maintaner, I do not know where coda_in_hdr->pgid is used.
>
> It is used to uniquely identify a process and any of it children during
> conflict resolution.
>
> When a conflict is detected, all accesses to the inconsistent object are
> blocked. A special resolver process is forked off by the cache manager
> and this is run in a new process group and only accesses from processes
> in this group are allowed. The resolver process (or any of it's children)
> compare the conflicting replicas, and ideally resolve the inconsistency
> after which normal accesses are unblocked.
>
> So yes this should not a per namespace thing, but also not a process
> specific pid, the resolver forks off different helper processes
> depending on the type of files that are involved in the conflict, i.e.
> mbox files require different merge strategy compared to opendocument
> files.

OK, thanks, please ignore this patch then.

> I'm not sure what you are trying to do.

Please look at http://marc.info/?l=linux-kernel&m=123240297918186

And I'd like to kill task_pgrp_nr(). Can't alloc_upcall() use
task_pgrp_nr_ns(current, &init_pid_ns) instead? This is equivalent.


But if this pid_t is used in the user-space to identify the process,
then I think Eric is right.

Oleg.