All of lore.kernel.org
 help / color / mirror / Atom feed
From: Roland McGrath <roland@redhat.com>
To: Oleg Nesterov <oleg@redhat.com>
Cc: Jeff Dike <jdike@addtoit.com>, linux-kernel@vger.kernel.org
Subject: Re: copy_process() && ti->flags (Was: PT_DTRACE && uml)
Date: Sun, 26 Apr 2009 19:10:56 -0700 (PDT)	[thread overview]
Message-ID: <20090427021057.045DBFC3B6@magilla.sf.frob.com> (raw)
In-Reply-To: Oleg Nesterov's message of  Monday, 27 April 2009 01:18:20 +0200 <20090426231820.GB6580@redhat.com>

> dup_task_struct()->setup_thread_stack() copies parent's ti->flags.
> 
> Why? Which flags should be actually copied? I must have missed
> something, but whats wrong with the patch below?

I suspect it just evolved that way as the default case of how
copy_process() is written: copy whole structs, and then fix them up.

Almost all the details of struct thread_info are arch-specific, so
whether copy-and-fix or start-from-zero is better really has to be
decided by each arch maintainer.

Your next two questions are not about UML, but about arch/x86 code.
Those should be directed to the x86 maintainers, whom you did not CC.

> OK, it is wrong. On x86 we should at least copy TIF_IA32. But
> why should we copy, say, TIF_DEBUG?

TIF_DEBUG is set when task->thread.debugregN fields have interesting
values.  The semantics today are that those values are copied, so
copying TIF_DEBUG too makes the child's context switches do what they
should.

This illustrates the general point: since the overall policy defaults
to copying, then what actually keeps the code simpler overall is to
have the same default at each substructure (and so it is for
thread_struct and thread_info).  If some things should be cleared,
they are cleared explicitly.  Hence, to clear TIF_DEBUG one would have
to do it on purpose, and would be required to think about what
TIF_DEBUG means and that clearing thread->debugregN goes along with
it.  (And at this point, one would then realize that one can't do it
without changing the user-visible semantics.)

> Actually, I don't understand why don't we use TS_IA32 instead of
> TIF_IA32. Only current can change this flag, perhaps it makes sense
> to move it in thread_info->status.

However, it is tested by other threads asynchronously.  The stated use
of TS_* flags is things only tested by current or when current is
stopped (e.g. TS_COMPAT).  In other uses like TASK_SIZE_OF(),
get_gate_vma(), etc., that might not be so.  It might be so on x86
that there can never be any modification to ti->status that could
momentarily clear some bit, but that is not formally said to be
required.

> copy_process()->clear_tsk_thread_flag(TIF_SIGPENDING) looks unneeded
> in any case...

It goes along with init_sigpending().  But it's actually potentially
wrong in case of shared_pending signals.  Do recalc_sigpending_tsk()
if anything.  As I think you intend to point out, it is always pretty
harmless to leave TIF_SIGPENDING set.  (get_signal_to_deliver will
just figure it out.)  So I think that should just be removed,
independent of your other questions.


Thanks,
Roland

  reply	other threads:[~2009-04-27  2:11 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20090330185146.D525AFC3AB@magilla.sf.frob.com>
     [not found] ` <20090408203954.GA26816@redhat.com>
     [not found]   ` <20090416204004.GA28013@redhat.com>
     [not found]     ` <20090416232430.4DAE4FC3C6@magilla.sf.frob.com>
     [not found]       ` <20090420183718.GC32527@redhat.com>
     [not found]         ` <20090421011354.4B19EFC3C7@magilla.sf.frob.com>
     [not found]           ` <20090421214819.GA22845@redhat.com>
     [not found]             ` <20090422032205.B8D39FC3C7@magilla.sf.frob.com>
2009-04-22 22:04               ` ptracee data structures cleanup Oleg Nesterov
2009-04-22 22:06                 ` remove PT_DTRACE from arch/* except arch/um Oleg Nesterov
2009-04-23  6:36                   ` Roland McGrath
2009-04-22 22:17                 ` PT_DTRACE && uml Oleg Nesterov
2009-04-23  6:39                   ` Roland McGrath
2009-04-23 16:02                   ` Jeff Dike
2009-04-26 22:09                     ` Oleg Nesterov
2009-04-26 23:18                       ` copy_process() && ti->flags (Was: PT_DTRACE && uml) Oleg Nesterov
2009-04-27  2:10                         ` Roland McGrath [this message]
2009-04-22 23:01                 ` ptracee data structures cleanup Mike Frysinger
2009-04-23  6:41                   ` Roland McGrath

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=20090427021057.045DBFC3B6@magilla.sf.frob.com \
    --to=roland@redhat.com \
    --cc=jdike@addtoit.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@redhat.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 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.