From: Andrew Morton on
On Wed, 12 May 2010 08:22:49 +1000
Dave Airlie <airlied(a)gmail.com> wrote:

> On Wed, May 12, 2010 at 5:57 AM, Chris Wilson <chris(a)chris-wilson.co.uk> wrote:
> > On Tue, 11 May 2010 12:10:01 -0700, Andrew Morton <akpm(a)linux-foundation.org> wrote:
> >> On Tue, 11 May 2010 19:52:31 +0100
> >> Chris Wilson <chris(a)chris-wilson.co.uk> wrote:
> >>
> >> > On Tue, 11 May 2010 11:35:55 -0400, Andrew Morton <akpm(a)linux-foundation.org> wrote:
> >> > > No, io_mapping_map_atomic_wc() cannot be used from [soft]irq context:
> >> > > it hardwires use of KM_USER0. __I suggest that io_mapping_create_wc(),
> >> > > io_mapping_map_atomic_wc() etc be changed so that the caller passes in the
> >> > > KM_foo kmap slot index.
> >> >
> >> > Argh, sorry for the noise, read the mail in the wrong order. Thanks for
> >> > the review. It would be sensible to go with your simpler patch whilst
> >> > io_mapping_map_atomic_wc() is improved.
> >>
> >> OK. __I'll be sending a bunch of fixes Linuswards in an hour or two.
> >> Should I include this?
> >
> > Yes.
> >
> > Acked-by: Chris Wilson <chris(a)chris-wilson.co.uk>
> >
>
> I'm not sure pushing this in at this point is a good idea, if I'm
> reading it correctly we've no idea what KM_IRQ is being used for,

It's used for taking kmaps from IRQ contexts.

> and
> this codepath is called from non-irq contexts just as much as irq
> contexts.

That's fine. As long as we do a local_irq_disable(), KM_IRQ0 can be
used from both irq- and non-irq contexts. All we need to do is to
ensure that some interrupt cannot come along on this CPU and corrupt
the slot.

> I'd rather we just backout the hangcheck stuff touching copies at all
> at this point, and try again doing it properly with a slow work or
> something for later.
>
> Dave.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Chris Wilson on
On Wed, 12 May 2010 08:22:49 +1000, Dave Airlie <airlied(a)gmail.com> wrote:
> I'd rather we just backout the hangcheck stuff touching copies at all
> at this point, and try again doing it properly with a slow work or
> something for later.

From my point of view, the information provided by the hangcheck has been
invaluable for delving into and fixing some obnoxious driver bugs. I
suspect its honeymoon period is now over - those bugs that it could
detect easily have been fixed (I hope). In order to capture the relevant
information for later chipset generations, we will need to parse the
command stream and include auxiliary buffers. So whilst I would prefer to
see this in a release so that I can easily diagnose bug reports, I accept
that there is more work to be done and will HTFU.

--
Chris Wilson, Intel Open Source Technology Centre
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Andrew Morton on
On Wed, 12 May 2010 08:51:05 +1000
Dave Airlie <airlied(a)gmail.com> wrote:

> On Wed, May 12, 2010 at 8:32 AM, Andrew Morton
> <akpm(a)linux-foundation.org> wrote:
> > On Wed, 12 May 2010 08:22:49 +1000
> > Dave Airlie <airlied(a)gmail.com> wrote:
> >
> >> On Wed, May 12, 2010 at 5:57 AM, Chris Wilson <chris(a)chris-wilson.co.uk> wrote:
> >> > On Tue, 11 May 2010 12:10:01 -0700, Andrew Morton <akpm(a)linux-foundation.org> wrote:
> >> >> On Tue, 11 May 2010 19:52:31 +0100
> >> >> Chris Wilson <chris(a)chris-wilson.co.uk> wrote:
> >> >>
> >> >> > On Tue, 11 May 2010 11:35:55 -0400, Andrew Morton <akpm(a)linux-foundation.org> wrote:
> >> >> > > No, io_mapping_map_atomic_wc() cannot be used from [soft]irq context:
> >> >> > > it hardwires use of KM_USER0. __I suggest that io_mapping_create_wc(),
> >> >> > > io_mapping_map_atomic_wc() etc be changed so that the caller passes in the
> >> >> > > KM_foo kmap slot index.
> >> >> >
> >> >> > Argh, sorry for the noise, read the mail in the wrong order. Thanks for
> >> >> > the review. It would be sensible to go with your simpler patch whilst
> >> >> > io_mapping_map_atomic_wc() is improved.
> >> >>
> >> >> OK. __I'll be sending a bunch of fixes Linuswards in an hour or two.
> >> >> Should I include this?
> >> >
> >> > Yes.
> >> >
> >> > Acked-by: Chris Wilson <chris(a)chris-wilson.co.uk>
> >> >
> >>
> >> I'm not sure pushing this in at this point is a good idea, if I'm
> >> reading it correctly we've no idea what KM_IRQ is being used for,
> >
> > It's used for taking kmaps from IRQ contexts.
> >
> >> and
> >> this codepath is called from non-irq contexts just as much as irq
> >> contexts.
> >
> > That's fine. __As long as we do a local_irq_disable(), KM_IRQ0 can be
> > used from both irq- and non-irq contexts. __All we need to do is to
> > ensure that some interrupt cannot come along on this CPU and corrupt
> > the slot.
>
> I don't think we do that in a lot of places, and I'd rather not add
> that in to fix this problem at this point in the release cycle, as
> we've no idea what it might break/regress.

What is "that"? The switch to irq-protected KM_IRQ0? That won't break
anything.

> Its easier to just disable the hangcheck copy and try again for 2.6.35
> with a workqueue or slow work.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Dave Airlie on
On Wed, May 12, 2010 at 8:32 AM, Andrew Morton
<akpm(a)linux-foundation.org> wrote:
> On Wed, 12 May 2010 08:22:49 +1000
> Dave Airlie <airlied(a)gmail.com> wrote:
>
>> On Wed, May 12, 2010 at 5:57 AM, Chris Wilson <chris(a)chris-wilson.co.uk> wrote:
>> > On Tue, 11 May 2010 12:10:01 -0700, Andrew Morton <akpm(a)linux-foundation.org> wrote:
>> >> On Tue, 11 May 2010 19:52:31 +0100
>> >> Chris Wilson <chris(a)chris-wilson.co.uk> wrote:
>> >>
>> >> > On Tue, 11 May 2010 11:35:55 -0400, Andrew Morton <akpm(a)linux-foundation.org> wrote:
>> >> > > No, io_mapping_map_atomic_wc() cannot be used from [soft]irq context:
>> >> > > it hardwires use of KM_USER0. __I suggest that io_mapping_create_wc(),
>> >> > > io_mapping_map_atomic_wc() etc be changed so that the caller passes in the
>> >> > > KM_foo kmap slot index.
>> >> >
>> >> > Argh, sorry for the noise, read the mail in the wrong order. Thanks for
>> >> > the review. It would be sensible to go with your simpler patch whilst
>> >> > io_mapping_map_atomic_wc() is improved.
>> >>
>> >> OK. __I'll be sending a bunch of fixes Linuswards in an hour or two.
>> >> Should I include this?
>> >
>> > Yes.
>> >
>> > Acked-by: Chris Wilson <chris(a)chris-wilson.co.uk>
>> >
>>
>> I'm not sure pushing this in at this point is a good idea, if I'm
>> reading it correctly we've no idea what KM_IRQ is being used for,
>
> It's used for taking kmaps from IRQ contexts.
>
>> and
>> this codepath is called from non-irq contexts just as much as irq
>> contexts.
>
> That's fine. �As long as we do a local_irq_disable(), KM_IRQ0 can be
> used from both irq- and non-irq contexts. �All we need to do is to
> ensure that some interrupt cannot come along on this CPU and corrupt
> the slot.

I don't think we do that in a lot of places, and I'd rather not add
that in to fix this problem at this point in the release cycle, as
we've no idea what it might break/regress.

Its easier to just disable the hangcheck copy and try again for 2.6.35
with a workqueue or slow work.

Dave



>
>> I'd rather we just backout the hangcheck stuff touching copies at all
>> at this point, and try again doing it properly with a slow work or
>> something for later.
>>
>> Dave.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo(a)vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
From: Dave Airlie on
On Wed, May 12, 2010 at 8:56 AM, Andrew Morton
<akpm(a)linux-foundation.org> wrote:
> On Wed, 12 May 2010 08:51:05 +1000
> Dave Airlie <airlied(a)gmail.com> wrote:
>
>> On Wed, May 12, 2010 at 8:32 AM, Andrew Morton
>> <akpm(a)linux-foundation.org> wrote:
>> > On Wed, 12 May 2010 08:22:49 +1000
>> > Dave Airlie <airlied(a)gmail.com> wrote:
>> >
>> >> On Wed, May 12, 2010 at 5:57 AM, Chris Wilson <chris(a)chris-wilson.co.uk> wrote:
>> >> > On Tue, 11 May 2010 12:10:01 -0700, Andrew Morton <akpm(a)linux-foundation.org> wrote:
>> >> >> On Tue, 11 May 2010 19:52:31 +0100
>> >> >> Chris Wilson <chris(a)chris-wilson.co.uk> wrote:
>> >> >>
>> >> >> > On Tue, 11 May 2010 11:35:55 -0400, Andrew Morton <akpm(a)linux-foundation.org> wrote:
>> >> >> > > No, io_mapping_map_atomic_wc() cannot be used from [soft]irq context:
>> >> >> > > it hardwires use of KM_USER0. __I suggest that io_mapping_create_wc(),
>> >> >> > > io_mapping_map_atomic_wc() etc be changed so that the caller passes in the
>> >> >> > > KM_foo kmap slot index.
>> >> >> >
>> >> >> > Argh, sorry for the noise, read the mail in the wrong order. Thanks for
>> >> >> > the review. It would be sensible to go with your simpler patch whilst
>> >> >> > io_mapping_map_atomic_wc() is improved.
>> >> >>
>> >> >> OK. __I'll be sending a bunch of fixes Linuswards in an hour or two.
>> >> >> Should I include this?
>> >> >
>> >> > Yes.
>> >> >
>> >> > Acked-by: Chris Wilson <chris(a)chris-wilson.co.uk>
>> >> >
>> >>
>> >> I'm not sure pushing this in at this point is a good idea, if I'm
>> >> reading it correctly we've no idea what KM_IRQ is being used for,
>> >
>> > It's used for taking kmaps from IRQ contexts.
>> >
>> >> and
>> >> this codepath is called from non-irq contexts just as much as irq
>> >> contexts.
>> >
>> > That's fine. __As long as we do a local_irq_disable(), KM_IRQ0 can be
>> > used from both irq- and non-irq contexts. __All we need to do is to
>> > ensure that some interrupt cannot come along on this CPU and corrupt
>> > the slot.
>>
>> I don't think we do that in a lot of places, and I'd rather not add
>> that in to fix this problem at this point in the release cycle, as
>> we've no idea what it might break/regress.
>
> What is "that"? �The switch to irq-protected KM_IRQ0? �That won't break
> anything.
>

disabling local cpu irqs around all these kmap mappings.

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