All of lore.kernel.org
 help / color / mirror / Atom feed
From: Oleg Nesterov <oleg@redhat.com>
To: Ian Kent <raven@themaw.net>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	hpa@zytor.com, Pavel Emelyanov <xemul@openvz.org>,
	Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org, "SergeE.Hallyn" <serue@us.ibm.com>
Subject: Re: [PATCH] autofs: fix the wrong usage of the deprecated task_pgrp_nr()
Date: Mon, 19 Jan 2009 08:08:01 +0100	[thread overview]
Message-ID: <20090119070801.GA21686@redhat.com> (raw)
In-Reply-To: <1232331602.3136.19.camel@zeus.themaw.net>

On 01/19, Ian Kent wrote:
>
> On Sun, 2009-01-18 at 08:34 +0100, Oleg Nesterov wrote:
> > parse_options(&pgid) sets pgid = task_pgrp_nr() which uses the global
> > namespace. This is wrong, we use this pgid to find "struct pid" in the
> > current's namespace. Change parse_options() to use task_pgrp_vnr().
> >
> > Also do s/task_pgrp_nr/task_pgrp_vnr/ in the debugging printks.
> > checkpatch.pl complains about "line over 80 characters", but it should
> > blame the cuurent code, not the patch.
>
> This changelog entry doesn't really have anything that I can use to work
> out if this change might introduce regressions.
>
> It would be helpful to me if you could include:
> 1) A brief statement about what your trying to achieve and why.

First of all, I think this patch fixes a bug.

What we are doing in autofs_fill_super()->parse_options() path
is find_get_pid(task_pgrp_vnr(current)), this is wrong.

task_pgrp_vnr() reporst the pid_t in the global namespace, but
find_get_pid() searches "struct pid" in the current namespace.
We can get the wrong pid. I tried to document this in changelog.

Another reason is that task_pgrp_nr() is deprecated. We are wasting
2 words in signal_struct without good reason. Please look at

	http://marc.info/?l=linux-kernel&m=123228947019281

> 2) The reason why task_pgrp_nr() has changed to task_pgrp_vnr() since
> you made the change (that is someone working on pid namespaces) to
> task_pgrp_nr().

see above.

> 3) Why you believe this change won't introduce a regression.

Ah, sorry. I forgot to mention this patch is only compile tested
(like the next one, but at least it has the note in changelog).

But again, I think this is bugfix.

Oleg.


  parent reply	other threads:[~2009-01-19  7:10 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-01-18  7:34 [PATCH] autofs: fix the wrong usage of the deprecated task_pgrp_nr() Oleg Nesterov
2009-01-19  2:20 ` Ian Kent
2009-01-19  6:35   ` H. Peter Anvin
2009-01-19  7:45     ` Ian Kent
2009-01-19 17:09       ` H. Peter Anvin
2009-01-20  1:18         ` Ian Kent
2009-01-19  7:08   ` Oleg Nesterov [this message]
2009-01-19  8:11     ` Ian Kent
2009-01-19  8:32       ` Oleg Nesterov
2009-01-19 11:15         ` Ian Kent
2009-01-19 12:42           ` Oleg Nesterov
2009-01-19 13:33             ` Ian Kent
2009-01-19 14:30               ` Oleg Nesterov
2009-01-19 17:48                 ` Serge E. Hallyn
2009-01-19 18:05                   ` Oleg Nesterov
2009-01-19 18:24                     ` Serge E. Hallyn
2009-01-19 19:17                       ` Oleg Nesterov
2009-01-19 19:20                         ` H. Peter Anvin
2009-01-19 19:32                           ` Oleg Nesterov
2009-01-19 19:35                         ` Serge E. Hallyn
2009-01-19 20:04                           ` Oleg Nesterov
2009-01-19 20:48                             ` Serge E. Hallyn
2009-01-19 21:31                               ` Oleg Nesterov
2009-01-19 22:11                                 ` Serge E. Hallyn
2009-01-20  2:07                                 ` Ian Kent
2009-01-20  1:35                         ` Ian Kent
2009-01-20  1:38                           ` H. Peter Anvin
2009-01-20  7:08                           ` Oleg Nesterov
2009-01-23  4:48 ` Ian Kent
2009-01-23  8:13   ` Oleg Nesterov
2009-01-23  9:09     ` Ian Kent

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=20090119070801.GA21686@redhat.com \
    --to=oleg@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=raven@themaw.net \
    --cc=serue@us.ibm.com \
    --cc=sukadev@linux.vnet.ibm.com \
    --cc=xemul@openvz.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.