All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Hildenbrand <dahi@linux.vnet.ibm.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Heiko Carstens <heiko.carstens@de.ibm.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	linuxppc-dev@lists.ozlabs.org, linux-arch@vger.kernel.org,
	linux-kernel@vger.kernel.org, benh@kernel.crashing.org,
	paulus@samba.org, akpm@linux-foundation.org,
	schwidefsky@de.ibm.com, mingo@kernel.org
Subject: Re: [RFC 0/2] Reenable might_sleep() checks for might_fault() when atomic
Date: Fri, 28 Nov 2014 08:34:54 +0100	[thread overview]
Message-ID: <20141128083454.403d5620@thinkpad-w530> (raw)
In-Reply-To: <alpine.DEB.2.11.1411272246110.3961@nanos>

> On Thu, 27 Nov 2014, David Hildenbrand wrote:
> > > OTOH, there is no reason why we need to disable preemption over that
> > > page_fault_disabled() region. There are code pathes which really do
> > > not require to disable preemption for that.
> > > 
> > > We have that seperated in preempt-rt for obvious reasons and IIRC
> > > Peter Zijlstra tried to distangle it in mainline some time ago. I
> > > forgot why that never got merged.
> > > 
> > 
> > Of course, we can completely separate that in our page fault code by doing
> > pagefault_disabled() checks instead of in_atomic() checks (even in add on
> > patches later).
> > 
> > > We tie way too much stuff on the preemption count already, which is a
> > > mightmare because we have no clear distinction of protection
> > > scopes. 
> > 
> > Although it might not be optimal, but keeping a separate counter for
> > pagefault_disable() as part of the preemption counter seems to be the only
> > doable thing right now.
> 
> It needs to be seperate, if it should be useful. Otherwise we just
> have a extra accounting in preempt_count() which does exactly the same
> thing as we have now: disabling preemption.
> 
> Now you might say, that we could mask out that part when checking
> preempt_count, but that wont work on x86 as x86 has the preempt
> counter as a per cpu variable and not as a per thread one.

Ah right, it's per cpu on x86. So it really belongs to a thread if we want to
demangle preemption and pagefault_disable.

Would work for now, but for x86 not on the long run.

> 
> But if you want to distangle pagefault disable from preempt disable
> then you must move it to the thread, because it is a property of the
> thread. preempt count is very much a per cpu counter as you can only
> go through schedule when it becomes 0.

Thinking about it, this makes perfect sense!

> 
> Btw, I find the x86 representation way more clear, because it
> documents that preempt count is a per cpu BKL and not a magic thread
> property. And sadly that is how preempt count is used ...
> 
> > I am not sure if a completely separated counter is even possible,
> > increasing the size of thread_info.
> 
> And adding a ulong to thread_info is going to create exactly which
> problem?

If we're allowed to increase the size of thread_info - absolutely fine with me!
(I am not sure if some archs have special constraints on the size)

Will see what I can come up with.

Thanks!

> 
> Thanks,
> 
> 	tglx
> 


WARNING: multiple messages have this Message-ID (diff)
From: David Hildenbrand <dahi@linux.vnet.ibm.com>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-arch@vger.kernel.org, "Michael S. Tsirkin" <mst@redhat.com>,
	Heiko Carstens <heiko.carstens@de.ibm.com>,
	linux-kernel@vger.kernel.org,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	paulus@samba.org, schwidefsky@de.ibm.com,
	akpm@linux-foundation.org, linuxppc-dev@lists.ozlabs.org,
	mingo@kernel.org
Subject: Re: [RFC 0/2] Reenable might_sleep() checks for might_fault() when atomic
Date: Fri, 28 Nov 2014 08:34:54 +0100	[thread overview]
Message-ID: <20141128083454.403d5620@thinkpad-w530> (raw)
In-Reply-To: <alpine.DEB.2.11.1411272246110.3961@nanos>

> On Thu, 27 Nov 2014, David Hildenbrand wrote:
> > > OTOH, there is no reason why we need to disable preemption over that
> > > page_fault_disabled() region. There are code pathes which really do
> > > not require to disable preemption for that.
> > > 
> > > We have that seperated in preempt-rt for obvious reasons and IIRC
> > > Peter Zijlstra tried to distangle it in mainline some time ago. I
> > > forgot why that never got merged.
> > > 
> > 
> > Of course, we can completely separate that in our page fault code by doing
> > pagefault_disabled() checks instead of in_atomic() checks (even in add on
> > patches later).
> > 
> > > We tie way too much stuff on the preemption count already, which is a
> > > mightmare because we have no clear distinction of protection
> > > scopes. 
> > 
> > Although it might not be optimal, but keeping a separate counter for
> > pagefault_disable() as part of the preemption counter seems to be the only
> > doable thing right now.
> 
> It needs to be seperate, if it should be useful. Otherwise we just
> have a extra accounting in preempt_count() which does exactly the same
> thing as we have now: disabling preemption.
> 
> Now you might say, that we could mask out that part when checking
> preempt_count, but that wont work on x86 as x86 has the preempt
> counter as a per cpu variable and not as a per thread one.

Ah right, it's per cpu on x86. So it really belongs to a thread if we want to
demangle preemption and pagefault_disable.

Would work for now, but for x86 not on the long run.

> 
> But if you want to distangle pagefault disable from preempt disable
> then you must move it to the thread, because it is a property of the
> thread. preempt count is very much a per cpu counter as you can only
> go through schedule when it becomes 0.

Thinking about it, this makes perfect sense!

> 
> Btw, I find the x86 representation way more clear, because it
> documents that preempt count is a per cpu BKL and not a magic thread
> property. And sadly that is how preempt count is used ...
> 
> > I am not sure if a completely separated counter is even possible,
> > increasing the size of thread_info.
> 
> And adding a ulong to thread_info is going to create exactly which
> problem?

If we're allowed to increase the size of thread_info - absolutely fine with me!
(I am not sure if some archs have special constraints on the size)

Will see what I can come up with.

Thanks!

> 
> Thanks,
> 
> 	tglx
> 

  reply	other threads:[~2014-11-28  7:35 UTC|newest]

Thread overview: 89+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-11-25 11:43 [RFC 0/2] Reenable might_sleep() checks for might_fault() when atomic David Hildenbrand
2014-11-25 11:43 ` David Hildenbrand
2014-11-25 11:43 ` [RFC 1/2] powerpc/fsl-pci: atomic get_user when pagefault_disabled David Hildenbrand
2014-11-25 11:43   ` David Hildenbrand
2015-01-30  5:15   ` [RFC,1/2] " Scott Wood
2015-01-30  7:58     ` David Hildenbrand
2014-11-25 11:43 ` [RFC 2/2] mm, sched: trigger might_sleep() in might_fault() when atomic David Hildenbrand
2014-11-25 11:43   ` David Hildenbrand
2014-11-26  7:02 ` [RFC 0/2] Reenable might_sleep() checks for " Michael S. Tsirkin
2014-11-26  7:02   ` Michael S. Tsirkin
2014-11-26 10:05   ` David Hildenbrand
2014-11-26 10:05     ` David Hildenbrand
2014-11-26 15:17     ` Michael S. Tsirkin
2014-11-26 15:17       ` Michael S. Tsirkin
2014-11-26 15:23       ` Michael S. Tsirkin
2014-11-26 15:23         ` Michael S. Tsirkin
2014-11-26 15:23         ` Michael S. Tsirkin
2014-11-26 15:32         ` David Hildenbrand
2014-11-26 15:32           ` David Hildenbrand
2014-11-26 15:47           ` Michael S. Tsirkin
2014-11-26 15:47             ` Michael S. Tsirkin
2014-11-26 16:02             ` David Hildenbrand
2014-11-26 16:02               ` David Hildenbrand
2014-11-26 16:19               ` Michael S. Tsirkin
2014-11-26 16:19                 ` Michael S. Tsirkin
2014-11-26 16:30                 ` Christian Borntraeger
2014-11-26 16:30                   ` Christian Borntraeger
2014-11-26 16:50                   ` Michael S. Tsirkin
2014-11-26 16:50                     ` Michael S. Tsirkin
2014-11-26 16:07             ` Christian Borntraeger
2014-11-26 16:07               ` Christian Borntraeger
2014-11-26 16:32               ` Michael S. Tsirkin
2014-11-26 16:32                 ` Michael S. Tsirkin
2014-11-26 16:51                 ` Christian Borntraeger
2014-11-26 16:51                   ` Christian Borntraeger
2014-11-26 17:04                   ` Michael S. Tsirkin
2014-11-26 17:04                     ` Michael S. Tsirkin
2014-11-26 17:21                     ` Michael S. Tsirkin
2014-11-26 17:21                       ` Michael S. Tsirkin
2014-11-27  7:09                     ` Heiko Carstens
2014-11-27  7:09                       ` Heiko Carstens
2014-11-27  7:40                       ` Michael S. Tsirkin
2014-11-27  7:40                         ` Michael S. Tsirkin
2014-11-27  8:03                       ` David Hildenbrand
2014-11-27  8:03                         ` David Hildenbrand
2014-11-27 12:04                         ` Heiko Carstens
2014-11-27 12:04                           ` Heiko Carstens
2014-11-27 12:08                           ` David Hildenbrand
2014-11-27 12:08                             ` David Hildenbrand
2014-11-27 15:07                           ` Thomas Gleixner
2014-11-27 15:07                             ` Thomas Gleixner
2014-11-27 15:19                             ` David Hildenbrand
2014-11-27 15:19                               ` David Hildenbrand
2014-11-27 15:37                               ` David Laight
2014-11-27 15:37                                 ` David Laight
2014-11-27 15:37                                 ` David Laight
2014-11-27 15:45                                 ` David Hildenbrand
2014-11-27 15:45                                   ` David Hildenbrand
2014-11-27 16:27                                   ` David Laight
2014-11-27 16:27                                     ` David Laight
2014-11-27 16:49                                     ` David Hildenbrand
2014-11-27 16:49                                       ` David Hildenbrand
2014-11-27 16:49                                       ` David Hildenbrand
2014-11-27 21:52                               ` Thomas Gleixner
2014-11-27 21:52                                 ` Thomas Gleixner
2014-11-28  7:34                                 ` David Hildenbrand [this message]
2014-11-28  7:34                                   ` David Hildenbrand
2014-11-26 15:30       ` Christian Borntraeger
2014-11-26 15:30         ` Christian Borntraeger
2014-11-26 15:37         ` Michael S. Tsirkin
2014-11-26 15:37           ` Michael S. Tsirkin
2014-11-26 16:02           ` Christian Borntraeger
2014-11-26 16:02             ` Christian Borntraeger
2014-11-26 15:22     ` Michael S. Tsirkin
2014-11-26 15:22       ` Michael S. Tsirkin
2014-11-27 17:10 ` [PATCH RFC " David Hildenbrand
2014-11-27 17:10   ` David Hildenbrand
2014-11-27 17:10   ` [PATCH RFC 1/2] preempt: track pagefault_disable() calls in the preempt counter David Hildenbrand
2014-11-27 17:10     ` David Hildenbrand
2014-11-27 17:10   ` [PATCH RFC 2/2] mm, sched: trigger might_sleep() in might_fault() when pagefaults are disabled David Hildenbrand
2014-11-27 17:10     ` David Hildenbrand
2014-11-27 17:24     ` Michael S. Tsirkin
2014-11-27 17:24       ` Michael S. Tsirkin
2014-11-27 17:32       ` Michael S. Tsirkin
2014-11-27 17:32         ` Michael S. Tsirkin
2014-11-27 18:08         ` David Hildenbrand
2014-11-27 18:08           ` David Hildenbrand
2014-11-27 18:27           ` Michael S. Tsirkin
2014-11-27 18:27             ` Michael S. Tsirkin

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=20141128083454.403d5620@thinkpad-w530 \
    --to=dahi@linux.vnet.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=benh@kernel.crashing.org \
    --cc=borntraeger@de.ibm.com \
    --cc=heiko.carstens@de.ibm.com \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=mingo@kernel.org \
    --cc=mst@redhat.com \
    --cc=paulus@samba.org \
    --cc=schwidefsky@de.ibm.com \
    --cc=tglx@linutronix.de \
    /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.