From: Linus Torvalds <torvalds@linux-foundation.org>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: Vincent Pelletier <plr.vincent@gmail.com>,
Simon Guinot <simon.guinot@sequanux.org>,
Alan Cox <alan@linux.intel.com>,
Giel van Schijndel <me@mortis.eu>,
"linux-gpio@vger.kernel.org" <linux-gpio@vger.kernel.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Vincent Donnefort <vdonnefort@gmail.com>,
Yoann Sculo <yoann@sculo.fr>
Subject: Re: [PATCH] kernel/resource.c: fix muxed resource handling in __request_region()
Date: Sat, 20 Feb 2016 09:11:54 -0800 [thread overview]
Message-ID: <CA+55aFx9-itKjqahUnXAoeDNgbi1MeNH5gDQB8Sw9AguPwGefQ@mail.gmail.com> (raw)
In-Reply-To: <56C7A459.1090801@virtuousgeek.org>
On Fri, Feb 19, 2016 at 3:25 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> +Linus (the de-facto resource guy).
>
> On 02/19/2016 01:10 PM, Vincent Pelletier wrote:
>>
>> I finally got around to rebasing some patches, and realised that the
>> patch from Simon Guinot below still gets rebased over torvalds' v4.4 .
>>
>> Any reason it was not applied ?
>> Or was the issue fixed in another, non-git-conflicting way ? (I see
>> nothing recent in git log kernel/resource.c)
>>
>> I do not find a trace of a mail confirming that I tested it and that it
>> fixes the issue. So here goes:
>> Tested-by: Vincent Pelletier <plr.vincent@gmail.com>
Hmm.
So I'm not entirely happy with the patch, because I think the problem
with using a possibly free'd parent resource at restart still exists.
As far as I can tell, if we hit the IORESOURCE_MUXED case *after* we
have successfully delved into a resource that wasn't busy, we will
have updated "parent" in a previous iteration of the loop, and we'll
not use the original parent when we then re-start after the sleep. So
quite frankly, I suspect any user of MUXED memory regions is still
fundamentally buggy, and IORESOURCE_MUXED has always been a hacky and
broken thing.
That said, I ended up applying the patch anyway, even if I despise it.
For all I know, muxed users never end up having those non-busy
sub-resources in practice, and maybe there is some serialization at
the top level for the drivers that use it. So if testing has shown
that it helps some actual case, I'll believe the testing. But the code
still looks rather debatable, and the whole IORESOURCE_MUXED approach
looks broken.
Jesse, that came in through you and the drm tree, I think. Alan is
marked as author, and there are other people who actually use and can
test the code. Can you guys think about the code a bit more.
I'm wondering if the *real* fix ends up being to reset the 'parent'
pointer to the original top-level parent after the sleep?
To recap: the patch is in my tree, but I'm not all that happy about it.
Linus
next prev parent reply other threads:[~2016-02-20 17:11 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1440093809-18234-1-git-send-email-plr.vincent@gmail.com>
[not found] ` <7d1a2156ddabe0b72964e88734adba307a472067.1440093298.git.plr.vincent@gmail.com>
[not found] ` <20150821175216.GE1729@kw.sim.vm.gnt>
[not found] ` <20150821224824.3406caa0@x2>
[not found] ` <20150903200540.399a96b8@x2>
2015-09-09 22:01 ` [1/4] gpio: gpio-f7188x: Use mutex for access serialisation Simon Guinot
2015-09-09 22:15 ` [PATCH] kernel/resource.c: fix muxed resource handling in __request_region() Simon Guinot
2016-02-19 21:10 ` Vincent Pelletier
2016-02-19 23:25 ` Jesse Barnes
2016-02-20 17:11 ` Linus Torvalds [this message]
2016-02-20 22:15 ` Jesse Barnes
2016-02-22 13:49 ` Alan Cox
2016-02-22 20:46 ` Jesse Barnes
2016-02-23 16:19 ` Simon Guinot
2016-02-23 17:19 ` Jesse Barnes
2016-02-23 21:38 ` One Thousand Gnomes
2016-02-24 14:25 ` [PATCH] kernel/resource.c: ensure parent is not freed " Simon Guinot
2016-02-23 8:00 ` [PATCH] kernel/resource.c: fix muxed resource handling " Vincent Pelletier
2015-09-12 13:26 ` [1/4] gpio: gpio-f7188x: Use mutex for access serialisation Vincent Pelletier
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=CA+55aFx9-itKjqahUnXAoeDNgbi1MeNH5gDQB8Sw9AguPwGefQ@mail.gmail.com \
--to=torvalds@linux-foundation.org \
--cc=alan@linux.intel.com \
--cc=jbarnes@virtuousgeek.org \
--cc=linux-gpio@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=me@mortis.eu \
--cc=plr.vincent@gmail.com \
--cc=simon.guinot@sequanux.org \
--cc=vdonnefort@gmail.com \
--cc=yoann@sculo.fr \
/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;
as well as URLs for NNTP newsgroup(s).