Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1221758 > unrolled thread

Re: [1/4] gpio: gpio-f7188x: Use mutex for access serialisation.

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [1/4] gpio: gpio-f7188x: Use mutex for access serialisation. Simon Guinot <simon.guinot@sequanux.org> - 2015-09-10 00:00 +0200
    [PATCH] kernel/resource.c: fix muxed resource handling in __request_region() Simon Guinot <simon.guinot@sequanux.org> - 2015-09-10 00:20 +0200
    Re: [1/4] gpio: gpio-f7188x: Use mutex for access serialisation. Vincent Pelletier <plr.vincent@gmail.com> - 2015-09-12 15:30 +0200

#1221758 — Re: [1/4] gpio: gpio-f7188x: Use mutex for access serialisation.

[Multipart message — attachments visible in raw view] — view raw

On Thu, Sep 03, 2015 at 08:05:40PM +0200, Vincent Pelletier wrote:
> Hi,
> 
> On Fri, 21 Aug 2015 22:48:24 +0200, Vincent Pelletier
> <plr.vincent@gmail.com> wrote:
> > On Fri, 21 Aug 2015 19:52:16 +0200, Simon Guinot
> > <simon.guinot@sequanux.org> wrote:
> > > Please, could you describe a setup (as simple as possible) allowing to
> > > reproduce the issue ? I'll try it on my side.
> > [...]
> > > Please try to make the module list needed to reproduce the issue as
> > > short as possible. Ideally only gpio_f7188x would be needed.
> > 
> > The simplest I could find so far needs gpio-input-polled with 20ms
> > polling period (I didn't try to change polling period), and a shell loop
> > writing to gpioXX/value.
> 
> Have you had a chance to reproduce this issue ?
> Can I do something to help ?

Hi Vincent,

Vincent (Donnefort) finally succeeds to reproduce the issue. The setup
is quite simple. You only have to flood the gpio-f7188x driver via the
sysfs GPIO interface. Nothing more is needed.

After some debugging we discovered that the problem comes from the
__request_region function which don't handle very well concurrent
requests on a muxed region.

I will send a patch as a reply to this email. Please, can you test it ?

Thanks,

Simon

[toc] | [next] | [standalone]

#1221769 — [PATCH] kernel/resource.c: fix muxed resource handling in __request_region()

In __request_region, if a conflict with a BUSY and MUXED resource is
detected, then the caller goes to sleep and waits for the resource to
be released. A pointer on the conflicting resource is kept. At wake-up
this pointer is used as a parent to retry to request the region. A first
problem is that this pointer might well be invalid (if for example the
conflicting resource have already been freed). An another problem is
that the next call to __request_region() fails to detect a remaining
conflict. The previously conflicting resource is passed as a parameter
and __request_region() will look for a conflict among the children of
this resource and not at the resource itself. It is likely to succeed
anyway, even if there is still a conflict. Instead, the parent of the
conflicting resource should be passed to __request_region().

As a fix attempt, this patch don't update the parent resource pointer in
the case we have to wait for a muxed region right after.

Reported-by: Vincent Pelletier <plr.vincent@gmail.com>
Signed-off-by: Simon Guinot <simon.guinot@sequanux.org>
Tested-by: Vincent Donnefort <vdonnefort@gmail.com>
---
 kernel/resource.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/resource.c b/kernel/resource.c
index fed052a1bc9f..b8c84804db6a 100644
--- a/kernel/resource.c
+++ b/kernel/resource.c
@@ -1072,9 +1072,10 @@ struct resource * __request_region(struct resource *parent,
 		if (!conflict)
 			break;
 		if (conflict != parent) {
-			parent = conflict;
-			if (!(conflict->flags & IORESOURCE_BUSY))
+			if (!(conflict->flags & IORESOURCE_BUSY)) {
+				parent = conflict;
 				continue;
+			}
 		}
 		if (conflict->flags & flags & IORESOURCE_MUXED) {
 			add_wait_queue(&muxed_resource_wait, &wait);
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1223414

Hello Simon,

On Thu, 10 Sep 2015 00:01:40 +0200, Simon Guinot
<simon.guinot@sequanux.org> wrote:
> Vincent (Donnefort) finally succeeds to reproduce the issue. The setup
> is quite simple. You only have to flood the gpio-f7188x driver via the
> sysfs GPIO interface. Nothing more is needed.
> 
> After some debugging we discovered that the problem comes from the
> __request_region function which don't handle very well concurrent
> requests on a muxed region.
> 
> I will send a patch as a reply to this email. Please, can you test it ?

I reverted my mutex-adding commit, applied given patch, and could not
reproduce the error after a few minutes with my test-case, so I think
this solves the issue.

Tested-by: Vincent Pelletier <plr.vincent@gmail.com>


I rebased my others gpio patches, unrelated to this issue:

gpio: gpio-f7188x: Implement get_direction.
gpio: gpio-f7188x: "get" should retrieve sensed level when available.
gpio: gpio-f7188x: GPIO bank 0 bit 0 is not available on f71869a

Should I resend ?
I have not checked other model's datasheets, FWIW.

Regards,
-- 
Vincent Pelletier
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web