I found this when my system crashed long after the mapping failure. The expected behavior should have been driver exit.
Since this is almost exclusively used for drm, I am posting to the dri mailing list. Should this go to another list as well?
Thanks,
Mike
Sometimes it is good to know when your mapping failed.
Fixes: cafaf14a5d8f ("io-mapping: Always create a struct to hold metadata about the io-mapping" Cc: Andrew Morton akpm@linux-foundation.org Cc: Mike Rapoport rppt@linux.ibm.com Cc: Andy Shevchenko andriy.shevchenko@linux.intel.com Cc: Chris Wilson chris@chris-wilson.co.uk Cc: stable@vger.kernel.org Signed-off-by: Michael J. Ruhl michael.j.ruhl@intel.com --- include/linux/io-mapping.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/io-mapping.h b/include/linux/io-mapping.h index 0beaa3eba155..5641e06cbcf7 100644 --- a/include/linux/io-mapping.h +++ b/include/linux/io-mapping.h @@ -118,7 +118,7 @@ io_mapping_init_wc(struct io_mapping *iomap, iomap->prot = pgprot_noncached(PAGE_KERNEL); #endif
- return iomap; + return iomap->iomem ? iomap : NULL; }
static inline void
On Tue, Jul 21, 2020 at 10:16:41AM -0400, Michael J. Ruhl wrote:
Sometimes it is good to know when your mapping failed.
Can you elaborate...
Fixes: cafaf14a5d8f ("io-mapping: Always create a struct to hold metadata about the io-mapping"
...especially taking into account that Fixes implies regression / bug?
-----Original Message----- From: Andy Shevchenko andriy.shevchenko@linux.intel.com Sent: Tuesday, July 21, 2020 10:47 AM To: Ruhl, Michael J michael.j.ruhl@intel.com Cc: dri-devel@lists.freedesktop.org; Andrew Morton <akpm@linux- foundation.org>; Mike Rapoport rppt@linux.ibm.com; Chris Wilson chris@chris-wilson.co.uk; stable@vger.kernel.org Subject: Re: [PATCH] io-mapping: Indicate mapping failure
On Tue, Jul 21, 2020 at 10:16:41AM -0400, Michael J. Ruhl wrote:
Sometimes it is good to know when your mapping failed.
Can you elaborate...
Sure, guess I was too glib. 😊
Currently the io_mapping_init_wc (the !ATOMIC_IOMAP version), function will always return success.
If the setting of the iomem (from ioremap_wc) fails, the only way for the caller to know is to check the value of iomap->iomem.
Since all of the callers expect a NULL return on error, and check for a NULL, I felt this needed a fixes (i.e. unexpected behavior).
Fixes: cafaf14a5d8f ("io-mapping: Always create a struct to hold metadata
about the io-mapping"
...especially taking into account that Fixes implies regression / bug?
The failure (in my case a crash) is not revealed until the address is accessed long after the init.
I will update the commit.
Mike
-- With Best Regards, Andy Shevchenko
On Tue, Jul 21, 2020 at 03:00:41PM +0000, Ruhl, Michael J wrote:
-----Original Message----- From: Andy Shevchenko andriy.shevchenko@linux.intel.com Sent: Tuesday, July 21, 2020 10:47 AM To: Ruhl, Michael J michael.j.ruhl@intel.com Cc: dri-devel@lists.freedesktop.org; Andrew Morton <akpm@linux- foundation.org>; Mike Rapoport rppt@linux.ibm.com; Chris Wilson chris@chris-wilson.co.uk; stable@vger.kernel.org Subject: Re: [PATCH] io-mapping: Indicate mapping failure
On Tue, Jul 21, 2020 at 10:16:41AM -0400, Michael J. Ruhl wrote:
Sometimes it is good to know when your mapping failed.
I was going to say it's always a good idea ;-)
Can you elaborate...
Sure, guess I was too glib. 😊
Currently the io_mapping_init_wc (the !ATOMIC_IOMAP version), function will always return success.
If the setting of the iomem (from ioremap_wc) fails, the only way for the caller to know is to check the value of iomap->iomem.
Since all of the callers expect a NULL return on error, and check for a NULL, I felt this needed a fixes (i.e. unexpected behavior).
Fixes: cafaf14a5d8f ("io-mapping: Always create a struct to hold metadata
about the io-mapping"
...especially taking into account that Fixes implies regression / bug?
The failure (in my case a crash) is not revealed until the address is accessed long after the init.
I will update the commit.
Mike
-- With Best Regards, Andy Shevchenko
dri-devel@lists.freedesktop.org
-
Andy Shevchenko -
Michael J. Ruhl -
Mike Rapoport -
Ruhl, Michael J