linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Mike McCormack <mikem@ring3k.org>
Cc: akpm@linux-foundation.org, kosaki.motohiro@jp.fujitsu.com,
	serue@us.ibm.com, jmorris@namei.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH] proc: Add complete process group list
Date: Tue, 22 Jun 2010 18:04:42 +0200	[thread overview]
Message-ID: <20100622160442.GA9072@redhat.com> (raw)
In-Reply-To: <4C20D1AE.5000205@ring3k.org>

On 06/23, Mike McCormack wrote:
>
> The new file /proc/<pid>/groups consists of a single group id per line,
>  with each line being 11 characters long.  This should be enough space
>  for 16bit or 32bit group ids.
>
> This feature might be useful for a server listening on a unix domain pipe
>  to determine the list of groups that a client process is in from its pid.

As usual, I can never comment whether we need this or not. Just a minor
nit about the code,

> +static int groups_proc_show(struct seq_file *m, void *v)
> +{
> +	struct inode *inode = m->private;
> +	struct group_info *group_info;
> +	struct task_struct *task;
> +	const struct cred *cred;
> +	struct pid *pid;
> +	unsigned int g;
> +
> +	pid = proc_pid(inode);
> +	task = get_pid_task(pid, PIDTYPE_PID);

get_proc_task() ?

> +	if (!task)
> +		return -ESRCH;
> +
> +	cred = get_task_cred(task);

OTOH. Not that I think this is terribly important, but I don't think
this needs get_task_struct + get_cred,

	group_info = NULL;

	rcu_read_lock();
	task = pid_task(proc_pid(inode), PIDTYPE_PID);
	if (task) {
		group_info = _task_cred(task)->group_info;
		get_group_info(group_info);
	}
	rcu_read_unlock();

	if (!group_info)
		retrn ESRCH;

Feel free to ignore though.

Oleg.


  reply	other threads:[~2010-06-22 16:06 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-22 15:07 [PATCH] proc: Add complete process group list Mike McCormack
2010-06-22 16:04 ` Oleg Nesterov [this message]
2010-06-22 22:37 ` Andrew Morton
2010-06-23 14:49   ` Mike McCormack
2010-06-24  2:41     ` KOSAKI Motohiro
2010-06-23  0:10 ` KOSAKI Motohiro

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=20100622160442.GA9072@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=jmorris@namei.org \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mikem@ring3k.org \
    --cc=serue@us.ibm.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).