mirror of https://lore.kernel.org/lkml/
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome