public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jakub Sitnicki <jsitnicki@gmail.com>
To: rafael.j.wysocki@intel.com
Cc: linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org,
	Jakub Sitnicki <jsitnicki@gmail.com>
Subject: [PATCH] PNP: Switch from __check_region() to __request_region()
Date: Mon,  8 Dec 2014 22:01:57 +0100	[thread overview]
Message-ID: <1418072517-5413-1-git-send-email-jsitnicki@gmail.com> (raw)

PNP core is the last user of the __check_region() which has been
deprecated for almost 12 years (since v2.5.54). Replace it with a combo
of __request_region() followed by __release_region().

pnp_check_port() and pnp_check_mem() remain racy after this change.

Signed-off-by: Jakub Sitnicki <jsitnicki@gmail.com>
---

There was a previous attempt at making this change 3 years ago but the
author has never addressed the review comments:

  https://lkml.org/lkml/2011/8/12/216

The end goal here is to get rid of __check_region() which lands in
every kernel image because of the PNP core.

It has been previously pointed out that replacing __check_region()
with request_region() obscures the fact that pnp_check_port() is racy:

  https://lkml.org/lkml/2011/8/11/466

Because of that I've also considered just moving __check_region() to
PNP core. However, that would require making free_resource() an
exported symbol in kernel/resource.c.

On the other hand, a switch to request/release_region() makes
pnp_check_port() and pnp_check_mem() follow the same pattern as found
in pnp_check_irq() and pnp_check_dma():

	if (!dev->active) {
		if (request_<resource type>(...))
			return 0;
		free_<resource type>(...);
	}
  
Admittedly, I was not able to exercise the touched code paths on a
commodity x86_64 laptop or under QEMU / VirtualBox (which lack ISA PnP
support, AFAIK).

To my understanding, the correct way to test pnp_check_port() or
pnp_check_mem() would be by issuing either:

  $ echo fill >/sys/bus/pnp/devices/XX:YY/resources
or
  $ echo auto >/sys/bus/pnp/devices/XX:YY/resources

... but only if the device is not attached or active, which is not the
case for ACPI PnP devices on my machines. If anyone can provide hints
on steps to test this, I will be glad to do so.

 drivers/pnp/resource.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pnp/resource.c b/drivers/pnp/resource.c
index 782e822..f980ff7 100644
--- a/drivers/pnp/resource.c
+++ b/drivers/pnp/resource.c
@@ -179,8 +179,9 @@ int pnp_check_port(struct pnp_dev *dev, struct resource *res)
 	/* check if the resource is already in use, skip if the
 	 * device is active because it itself may be in use */
 	if (!dev->active) {
-		if (__check_region(&ioport_resource, *port, length(port, end)))
+		if (!request_region(*port, length(port, end), "pnp"))
 			return 0;
+		release_region(*port, length(port, end));
 	}
 
 	/* check if the resource is reserved */
@@ -241,8 +242,9 @@ int pnp_check_mem(struct pnp_dev *dev, struct resource *res)
 	/* check if the resource is already in use, skip if the
 	 * device is active because it itself may be in use */
 	if (!dev->active) {
-		if (check_mem_region(*addr, length(addr, end)))
+		if (!request_mem_region(*addr, length(addr, end), "pnp"))
 			return 0;
+		release_mem_region(*addr, length(addr, end));
 	}
 
 	/* check if the resource is reserved */
-- 
1.9.3


             reply	other threads:[~2014-12-08 21:02 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-08 21:01 Jakub Sitnicki [this message]
2014-12-10 23:32 ` [PATCH] PNP: Switch from __check_region() to __request_region() Rafael J. Wysocki
2014-12-12  7:47   ` Jakub Sitnicki
2014-12-22 11:19     ` Jakub Sitnicki
2014-12-22 22:34       ` Rafael J. Wysocki
2015-01-30  0:01         ` Rafael J. Wysocki
2015-02-18 20:51           ` Jakub Sitnicki
2015-02-19  1:04             ` Rafael J. Wysocki

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=1418072517-5413-1-git-send-email-jsitnicki@gmail.com \
    --to=jsitnicki@gmail.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rafael.j.wysocki@intel.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