All of lore.kernel.org
 help / color / mirror / Atom feed
From: Heiko Carstens <heiko.carstens@de.ibm.com>
To: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: "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: Thu, 27 Nov 2014 13:04:41 +0100	[thread overview]
Message-ID: <20141127120441.GB4390@osiris> (raw)
In-Reply-To: <20141127090301.3ddc3077@thinkpad-w530>

On Thu, Nov 27, 2014 at 09:03:01AM +0100, David Hildenbrand wrote:
> > Code like
> > 	spin_lock(&lock);
> > 	if (copy_to_user(...))
> > 		rc = ...
> > 	spin_unlock(&lock);
> > really *should* generate warnings like it did before.
> > 
> > And *only* code like
> > 	spin_lock(&lock);
> 
> Is only code like this valid or also with the spin_lock() dropped?
> (e.g. the access in patch1 if I remember correctly)
> 
> So should page_fault_disable() increment the pagefault counter and the preempt
> counter or only the first one?

Given that a sequence like

	page_fault_disable();
 	if (copy_to_user(...))
 		rc = ...
 	page_fault_enable();

is correct code right now I think page_fault_disable() should increase both.
No need for surprising semantic changes.

> So we would have pagefault code rely on:
> 
> in_disabled_pagefault() ( pagefault_disabled() ... whatever ) instead of
> in_atomic().

No, let's be more defensive: the page fault handler should do nothing if
in_atomic() just like now. But it could have a quick check and emit a one
time warning if page faults aren't disabled in addition.
That might help debugging but keeps the system more likely alive.

might_fault() however should call might_sleep() if page faults aren't
disabled, but that's what you proposed anyway I think.

WARNING: multiple messages have this Message-ID (diff)
From: Heiko Carstens <heiko.carstens@de.ibm.com>
To: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: linux-arch@vger.kernel.org, "Michael S. Tsirkin" <mst@redhat.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: Thu, 27 Nov 2014 13:04:41 +0100	[thread overview]
Message-ID: <20141127120441.GB4390@osiris> (raw)
In-Reply-To: <20141127090301.3ddc3077@thinkpad-w530>

On Thu, Nov 27, 2014 at 09:03:01AM +0100, David Hildenbrand wrote:
> > Code like
> > 	spin_lock(&lock);
> > 	if (copy_to_user(...))
> > 		rc = ...
> > 	spin_unlock(&lock);
> > really *should* generate warnings like it did before.
> > 
> > And *only* code like
> > 	spin_lock(&lock);
> 
> Is only code like this valid or also with the spin_lock() dropped?
> (e.g. the access in patch1 if I remember correctly)
> 
> So should page_fault_disable() increment the pagefault counter and the preempt
> counter or only the first one?

Given that a sequence like

	page_fault_disable();
 	if (copy_to_user(...))
 		rc = ...
 	page_fault_enable();

is correct code right now I think page_fault_disable() should increase both.
No need for surprising semantic changes.

> So we would have pagefault code rely on:
> 
> in_disabled_pagefault() ( pagefault_disabled() ... whatever ) instead of
> in_atomic().

No, let's be more defensive: the page fault handler should do nothing if
in_atomic() just like now. But it could have a quick check and emit a one
time warning if page faults aren't disabled in addition.
That might help debugging but keeps the system more likely alive.

might_fault() however should call might_sleep() if page faults aren't
disabled, but that's what you proposed anyway I think.

  reply	other threads:[~2014-11-27 12:04 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 [this message]
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
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=20141127120441.GB4390@osiris \
    --to=heiko.carstens@de.ibm.com \
    --cc=akpm@linux-foundation.org \
    --cc=benh@kernel.crashing.org \
    --cc=borntraeger@de.ibm.com \
    --cc=dahi@linux.vnet.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 \
    /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.