* Re: [PATCH v3 0/3] sched: Always check the integrity of the canary
From: Aaron Tomlin @ 2014-09-11 15:59 UTC (permalink / raw)
To: Peter Zijlstra
Cc: dzickus, jcastillo, riel, x86, akpm, mingo, bmr, prarit, oleg,
rostedt, linux-kernel, minchan, mingo, aneesh.kumar, akpm, hannes,
jgh, linuxppc-dev, tglx, pzijlstr
In-Reply-To: <20140911155303.GI4783@worktop.ger.corp.intel.com>
On Thu, Sep 11, 2014 at 05:53:03PM +0200, Peter Zijlstra wrote:
>
> What's with the threading all versions together? Please don't do that --
> also don't post a new version just for this though.
Sorry about that. Noted.
--
Aaron Tomlin
^ permalink raw reply
* RE: [PATCH v3 0/3] sched: Always check the integrity of the canary
From: David Laight @ 2014-09-11 16:02 UTC (permalink / raw)
To: 'Aaron Tomlin', peterz@infradead.org
Cc: dzickus@redhat.com, jcastillo@redhat.com, riel@redhat.com,
prarit@redhat.com, pzijlstr@redhat.com, mingo@kernel.com,
bmr@redhat.com, x86@kernel.org, oleg@redhat.com,
rostedt@goodmis.org, linux-kernel@vger.kernel.org,
minchan@kernel.org, mingo@redhat.com, tglx@linutronix.de,
aneesh.kumar@linux.vnet.ibm.com, hannes@cmpxchg.org,
akpm@linux-foundation.org, linuxppc-dev@lists.ozlabs.org,
jgh@redhat.com, akpm@google.com
In-Reply-To: <1410450088-18236-1-git-send-email-atomlin@redhat.com>
RnJvbTogQWFyb24gVG9tbGluDQo+IEN1cnJlbnRseSBpbiB0aGUgZXZlbnQgb2YgYSBzdGFjayBv
dmVycnVuIGEgY2FsbCB0byBzY2hlZHVsZSgpDQo+IGRvZXMgbm90IGNoZWNrIGZvciB0aGlzIHR5
cGUgb2YgY29ycnVwdGlvbi4gVGhpcyBjb3JydXB0aW9uIGlzDQo+IG9mdGVuIHNpbGVudCBhbmQg
Y2FuIGdvIHVubm90aWNlZC4gSG93ZXZlciBvbmNlIHRoZSBjb3JydXB0ZWQNCj4gcmVnaW9uIGlz
IGV4YW1pbmVkIGF0IGEgbGF0ZXIgc3RhZ2UsIHRoZSBvdXRjb21lIGlzIHVuZGVmaW5lZA0KPiBh
bmQgb2Z0ZW4gcmVzdWx0cyBpbiBhIHNwb3JhZGljIHBhZ2UgZmF1bHQgd2hpY2ggY2Fubm90IGJl
DQo+IGhhbmRsZWQuDQo+IA0KPiBUaGUgZmlyc3QgcGF0Y2ggYWRkcyBhIGNhbmFyeSB0byBpbml0
X3Rhc2sncyBlbmQgb2Ygc3RhY2suDQo+IFdoaWxlIHRoZSBzZWNvbmQgcGF0Y2ggcHJvdmlkZXMg
YSBoZWxwZXIgdG8gZGV0ZXJtaW5lIHRoZQ0KPiBpbnRlZ3JpdHkgb2YgdGhlIGNhbmFyeS4gVGhl
IHRoaXJkIGNoZWNrcyBmb3IgYSBzdGFjaw0KPiBvdmVycnVuIGFuZCB0YWtlcyBhcHByb3ByaWF0
ZSBhY3Rpb24gc2luY2UgdGhlIGRhbWFnZQ0KPiBpcyBhbHJlYWR5IGRvbmUsIHRoZXJlIGlzIG5v
IHBvaW50IGluIGNvbnRpbnVpbmcuDQoNCkNsZWFybHkgeW91J3ZlIGp1c3QgYmVlbiAnYml0dGVu
JyBieSBhIGtlcm5lbCBzdGFjayBvdmVyZmxvdy4NCkJ1dCBhIHNpbXBsZSAnY2FuYXJ5JyBpc24n
dCBnb2luZyB0byBmaW5kIG1vc3Qgb2YgdGhlIG92ZXJmbG93cw0KYW5kIHdpbGwgZ2l2ZSBhbiBp
bmNvcnJlY3QgJ3NlbnNlIG9mIHNlY3VyaXR5Jy4NCg0KVGhlIGNhbmFyeSB3aWxsIG9ubHkgd29y
ayBpZiB0aGUgc3RhY2sgaXMgZGVuc2VseSB3cml0dGVuLg0KSW4gcHJhY3Rpc2UgdGhlIHN0YWNr
IGFsaWdubWVudCBydWxlcyBjcmVhdGUgZ2FwcywgYW5kIHRoZQ0KbW9zdCBsaWtlbHkgcmVhc29u
IGZvciBvdmVyZmxvdyBpcyBhIGxhcmdlIG9uLXN0YWNrIGJ1ZmZlcg0KdGhhdCBpc24ndCBhY3R1
YWxseSB3cml0dGVuIHRvLg0KDQpUaGUgb25seSByZWFsIHdheSB0byBkZXRlY3Qga2VybmVsIHN0
YWNrIG92ZXJmbG93IGlzIHRvIGFycmFuZ2UNCmZvciBhbiB1bm1hcHBlZCBwYWdlIGJleW9uZCB0
aGUgc3RhY2suDQpUaGF0IGNvc3RzIEtWQSwgYnV0IG5vdCBtdWNoIGVsc2UuDQoNCglEYXZpZA0K
DQo=
^ permalink raw reply
* Re: bit fields && data tearing
From: Paul E. McKenney @ 2014-09-11 16:16 UTC (permalink / raw)
To: One Thousand Gnomes
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, Peter Hurley, linux-alpha,
Oleg Nesterov, linux-kernel@vger.kernel.org, David Laight,
Paul Mackerras, H. Peter Anvin, linuxppc-dev@lists.ozlabs.org,
Miroslav Franc, Richard Henderson
In-Reply-To: <20140911110411.2de01944@alan.etchedpixels.co.uk>
On Thu, Sep 11, 2014 at 11:04:11AM +0100, One Thousand Gnomes wrote:
> > > Is *that* what we are talking about? I was added to this conversation
> > > in the middle where it had already generalized, so I had no idea.
> >
> > No, this is just what brought this craziness to my attention.
>
> None of it is craziness. It's the real world leaking into the crazy
> delusional world of sequential programming. Machines are going to get
> more not less parallel.
Amen to that!!!
> > For example, byte- and short-sized circular buffers could not possibly
> > be safe either, when the head nears the tail.
> >
> > Who has audited global storage and ensured that _every_ byte-sized write
> > doesn't happen to be adjacent to some other storage that may not happen
> > to be protected by the same (or any) lock?
>
> Thats a meaningless question. Have you audited it all for correctness of
> any other form. Have you mathematically verified the functionality as a
> set of formal proofs ? If you can't prove its formally mathematically
> functionally correct why are you worried about this ?
>
> Alpha works, maybe it has a near theoretical race on that point. It's not
> any worse than it was 15 years ago and nobody has really hit a problem
> with it. So from that you can usefully infer that those buffer cases are
> not proving a real problem.
Fair enough, I guess.
But Alpha's limitations were given as a reason to restrict
smp_store_release() and smp_load_acquire() from providing one-byte and
two-byte variants. Of course, I am OK "probabilistically supporting"
pre-EV56 Alpha CPUs, but only if they don't get in the way of us doing
smp_store_release() and smp_load_acquire() on chars and shorts. So if
pre-EV56 support has to go in order to allow smp_store_release() and
smp_load_acquire() on small data types, then pre-EV56 support simply
has to go.
Alternatively, one way to support this old hardware on a more
deterministic basis is to make the compiler use ll/sc sequences to do
byte and short accesses. That would be fine as well.
Thanx, Paul
> The tty locks together on the other hand are asking to hit it, and the
> problem you were trying to fix were the ones that need set_bit() to make
> the guarantees.
>
> Alan
>
^ permalink raw reply
* Re: [PATCH v3 0/3] sched: Always check the integrity of the canary
From: Chuck Ebbert @ 2014-09-11 17:26 UTC (permalink / raw)
To: David Laight
Cc: jcastillo@redhat.com, akpm@google.com, peterz@infradead.org,
bmr@redhat.com, linux-kernel@vger.kernel.org, prarit@redhat.com,
x86@kernel.org, mingo@redhat.com, 'Aaron Tomlin',
dzickus@redhat.com, riel@redhat.com, mingo@kernel.com,
rostedt@goodmis.org, tglx@linutronix.de, jgh@redhat.com,
pzijlstr@redhat.com, oleg@redhat.com, minchan@kernel.org,
aneesh.kumar@linux.vnet.ibm.com, hannes@cmpxchg.org,
akpm@linux-foundation.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D17490245@AcuExch.aculab.com>
On Thu, 11 Sep 2014 16:02:45 +0000
David Laight <David.Laight@ACULAB.COM> wrote:
> From: Aaron Tomlin
> > Currently in the event of a stack overrun a call to schedule()
> > does not check for this type of corruption. This corruption is
> > often silent and can go unnoticed. However once the corrupted
> > region is examined at a later stage, the outcome is undefined
> > and often results in a sporadic page fault which cannot be
> > handled.
> >
> > The first patch adds a canary to init_task's end of stack.
> > While the second patch provides a helper to determine the
> > integrity of the canary. The third checks for a stack
> > overrun and takes appropriate action since the damage
> > is already done, there is no point in continuing.
>
> Clearly you've just been 'bitten' by a kernel stack overflow.
> But a simple 'canary' isn't going to find most of the overflows
> and will give an incorrect 'sense of security'.
>
> The canary will only work if the stack is densely written.
> In practise the stack alignment rules create gaps, and the
> most likely reason for overflow is a large on-stack buffer
> that isn't actually written to.
>
> The only real way to detect kernel stack overflow is to arrange
> for an unmapped page beyond the stack.
> That costs KVA, but not much else.
>
That doesn't work either, because the threadinfo sits between the end of the
stack and the beginning of the next page, making it possible to corrupt critical
data without running off the page.
^ permalink raw reply
* Re: [PATCH v3 0/3] sched: Always check the integrity of the canary
From: Aaron Tomlin @ 2014-09-11 17:44 UTC (permalink / raw)
To: David Laight
Cc: dzickus@redhat.com, jcastillo@redhat.com, riel@redhat.com,
prarit@redhat.com, mingo@kernel.com, pzijlstr@redhat.com,
peterz@infradead.org, bmr@redhat.com, x86@kernel.org,
oleg@redhat.com, rostedt@goodmis.org,
linux-kernel@vger.kernel.org, minchan@kernel.org,
mingo@redhat.com, tglx@linutronix.de,
aneesh.kumar@linux.vnet.ibm.com, hannes@cmpxchg.org,
akpm@linux-foundation.org, linuxppc-dev@lists.ozlabs.org,
jgh@redhat.com, akpm@google.com
In-Reply-To: <063D6719AE5E284EB5DD2968C1650D6D17490245@AcuExch.aculab.com>
On Thu, Sep 11, 2014 at 04:02:45PM +0000, David Laight wrote:
> From: Aaron Tomlin
> > Currently in the event of a stack overrun a call to schedule()
> > does not check for this type of corruption. This corruption is
> > often silent and can go unnoticed. However once the corrupted
> > region is examined at a later stage, the outcome is undefined
> > and often results in a sporadic page fault which cannot be
> > handled.
> >
> > The first patch adds a canary to init_task's end of stack.
> > While the second patch provides a helper to determine the
> > integrity of the canary. The third checks for a stack
> > overrun and takes appropriate action since the damage
> > is already done, there is no point in continuing.
>
> Clearly you've just been 'bitten' by a kernel stack overflow.
> But a simple 'canary' isn't going to find most of the overflows
> and will give an incorrect 'sense of security'.
Please note that this is not suppose to be a 'perfect' solution.
Rather a worth while check in this particular code path.
Let's assume that the canary is damaged. In this situation it is
rather likely that the thread_info object has been compromised too.
--
Aaron Tomlin
^ permalink raw reply
* Re: bit fields && data tearing
From: Peter Hurley @ 2014-09-11 20:01 UTC (permalink / raw)
To: One Thousand Gnomes
Cc: Jakub Jelinek, linux-arch@vger.kernel.org, Tony Luck,
linux-ia64@vger.kernel.org, linux-alpha, Oleg Nesterov,
linux-kernel@vger.kernel.org, David Laight, Paul Mackerras,
H. Peter Anvin, Paul E. McKenney, linuxppc-dev@lists.ozlabs.org,
Miroslav Franc, Richard Henderson
In-Reply-To: <20140911110411.2de01944@alan.etchedpixels.co.uk>
On 09/11/2014 06:04 AM, One Thousand Gnomes wrote:
>>> Is *that* what we are talking about? I was added to this conversation
>>> in the middle where it had already generalized, so I had no idea.
>>
>> No, this is just what brought this craziness to my attention.
>
> None of it is craziness. It's the real world leaking into the crazy
> delusional world of sequential programming. Machines are going to get
> more not less parallel.
>
>> For example, byte- and short-sized circular buffers could not possibly
>> be safe either, when the head nears the tail.
>>
>> Who has audited global storage and ensured that _every_ byte-sized write
>> doesn't happen to be adjacent to some other storage that may not happen
>> to be protected by the same (or any) lock?
>
> Thats a meaningless question. Have you audited it all for correctness of
> any other form. Have you mathematically verified the functionality as a
> set of formal proofs ? If you can't prove its formally mathematically
> functionally correct why are you worried about this ?
>
> Alpha works, maybe it has a near theoretical race on that point. It's not
> any worse than it was 15 years ago and nobody has really hit a problem
> with it. So from that you can usefully infer that those buffer cases are
> not proving a real problem.
>
> The tty locks together on the other hand are asking to hit it, and the
> problem you were trying to fix were the ones that need set_bit() to make
> the guarantees.
So a problem that no one has ever complained about on _any_ arch is suddenly
a problem on a subset of Alpha cpus, but a problem I know exists on Alpha
isn't important because no one's filed a bug about it?
The only Alpha person in this discussion has come out clearly in favor
of dropping EV4/5 support.
The fact is that the kernel itself is much more parallel than it was
15 years ago, and that trend is going to continue. Paired with the fact
that the Alpha is the least-parallel-friendly arch, makes improving
parallelism and correctness even harder within kernel subsystems; harder
than it has to be and harder than it should be.
Linus has repeatedly stated that non-arch code should be as
arch-independent as possible, so I believe that working around problems
created by a cpu from 1995 _which no other arch exhibits_ is ludicrous.
Especially in generic kernel code.
That said, if the Alpha community wants to keep _actively_ supporting
the Alpha arch, fine. They could be working toward solutions for
making Alpha workarounds in generic kernel code unnecessary. If that means
compiler changes, ok. If that means arch-independent macros, well, they
can float that idea.
Or if they're comfortable with the status quo, also fine. By that, I mean
the Alpha arch gets no workarounds in generic kernel code, and if something
goes a little sideways only on Alpha, that's to be expected.
As Paul pointed out, a good first step would be for the Alpha community
to contribute byte and short versions of smp_load_acquire() and
smp_store_release() so that the rest of the kernel community can make
forward progress on more parallelism without Alpha-only limitations.
Regards,
Peter Hurley
^ permalink raw reply
* Re: [PATCH 6/6] powerpc: Separate ppc32 symbol exports into ppc_ksyms_32.c
From: Anton Blanchard @ 2014-09-11 21:50 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: paulus, linuxppc-dev
In-Reply-To: <20140823012509.37afb89e@canb.auug.org.au>
Hi Stephen,
> You removed export.h ...
>
> > +EXPORT_SYMBOL(flush_dcache_range);
> > +EXPORT_SYMBOL(flush_icache_range);
>
> But still use EXPORT_SYMBOL ...
Thanks, fixed!
Anton
^ permalink raw reply
* Re: [PATCH 4/4] powerpc: Move htab_remove_mapping function prototype into header file
From: Anton Blanchard @ 2014-09-11 21:55 UTC (permalink / raw)
To: Stephen Rothwell; +Cc: paulus, linuxppc-dev
In-Reply-To: <20140823012940.303e3ebc@canb.auug.org.au>
Hi Stephen,
> Please be consistent about "extern" use (unless this file is already
> inconsistent, I guess). (I know that the current trend is to remove
> "extern" in header files - I just happen to disagree with that
> trend. :-))
Good idea, fixed this for the next rev.
Anton
^ permalink raw reply
* Re: [PATCH V2] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Nicolin Chen @ 2014-09-11 22:57 UTC (permalink / raw)
To: Shengjiu Wang
Cc: alsa-devel, lgirdwood, tiwai, Li.Xiubo, timur, perex, broonie,
linuxppc-dev, linux-kernel
In-Reply-To: <27584da8e3ab291ab8dbcbb411579613f8384a59.1410413734.git.shengjiu.wang@freescale.com>
On Thu, Sep 11, 2014 at 01:38:29PM +0800, Shengjiu Wang wrote:
> Move the ipg clock enable and disable operation to startup and shutdown,
> that is only enable ipg clock when ssi is working. Keep clock is disabled
> when ssi is in idle.
> otherwise, _fsl_ssi_set_dai_fmt function need to be called in probe,
> so add ipg clock control for it.
It seems to be no objection so far against my last suggestion to
use regmap's mmio_clk() for named ipg clk only. So you may still
consider about that.
Anyway, I'd like to do thing in parallel. So I just simply tested
it on my side and its works fine, it may still need to be tested
by others though.
Nicolin
^ permalink raw reply
* Re: [PATCH V2] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Shengjiu Wang @ 2014-09-12 2:01 UTC (permalink / raw)
To: Nicolin Chen, mpa
Cc: alsa-devel, lgirdwood, tiwai, Li.Xiubo, timur, perex, broonie,
linuxppc-dev, linux-kernel
In-Reply-To: <20140911225737.GA13926@Alpha>
On Thu, Sep 11, 2014 at 03:57:37PM -0700, Nicolin Chen wrote:
> On Thu, Sep 11, 2014 at 01:38:29PM +0800, Shengjiu Wang wrote:
> > Move the ipg clock enable and disable operation to startup and shutdown,
> > that is only enable ipg clock when ssi is working. Keep clock is disabled
> > when ssi is in idle.
> > otherwise, _fsl_ssi_set_dai_fmt function need to be called in probe,
> > so add ipg clock control for it.
>
> It seems to be no objection so far against my last suggestion to
> use regmap's mmio_clk() for named ipg clk only. So you may still
> consider about that.
>
I think mmio_clk() can be put to another patch. and this patch only for clk_enable()
and clk_disable() operation.
> Anyway, I'd like to do thing in parallel. So I just simply tested
> it on my side and its works fine, it may still need to be tested
> by others though.
>
> Nicolina
Hi Markus
could you please review it, and share your comments?
wang shengjiu
^ permalink raw reply
* Re: [PATCH V2] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Timur Tabi @ 2014-09-12 2:43 UTC (permalink / raw)
To: Shengjiu Wang, nicoleotsuka, Li.Xiubo, lgirdwood, broonie, perex,
tiwai
Cc: alsa-devel, linuxppc-dev, linux-kernel
In-Reply-To: <27584da8e3ab291ab8dbcbb411579613f8384a59.1410413734.git.shengjiu.wang@freescale.com>
Shengjiu Wang wrote:
> + ret = clk_prepare_enable(ssi_private->clk);
> + if (ret)
> + return ret;
Will this work on PowerPC, where ssi_private->clk is always NULL?
^ permalink raw reply
* Re: [PATCH V2] powerpc/eeh: Fix kernel crash when passing through VF
From: Michael Ellerman @ 2014-09-12 3:55 UTC (permalink / raw)
To: Wei Yang; +Cc: linuxppc-dev, gwshan
In-Reply-To: <1410406921-8557-1-git-send-email-weiyang@linux.vnet.ibm.com>
On Thu, 2014-09-11 at 11:42 +0800, Wei Yang wrote:
> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
> index 4a45ba8..403445e 100644
> --- a/arch/powerpc/kernel/eeh.c
> +++ b/arch/powerpc/kernel/eeh.c
> @@ -625,7 +625,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
> int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
> {
> struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
> - struct eeh_pe *pe = edev->pe;
> + struct eeh_pe *pe = edev ? edev->pe : NULL;
>
> if (!pe) {
> pr_err("%s: No PE found on PCI device %s\n",
We seem to do this or something similar in a few places. Is it worth having a
pci_dev_to_eeh_pe() inline?
cheers
^ permalink raw reply
* Re: [PATCH v3 3/3] sched: BUG when stack end location is over written
From: Michael Ellerman @ 2014-09-12 4:06 UTC (permalink / raw)
To: Aaron Tomlin
Cc: dzickus, jcastillo, riel, prarit, mingo, pzijlstr, peterz, bmr,
x86, oleg, rostedt, linux-kernel, minchan, mingo, tglx,
aneesh.kumar, hannes, akpm, linuxppc-dev, jgh, akpm
In-Reply-To: <1410450088-18236-4-git-send-email-atomlin@redhat.com>
On Thu, 2014-09-11 at 16:41 +0100, Aaron Tomlin wrote:
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index a285900..2a8280a 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -824,6 +824,18 @@ config SCHEDSTATS
> application, you can say N to avoid the very slight overhead
> this adds.
>
> +config SCHED_STACK_END_CHECK
> + bool "Detect stack corruption on calls to schedule()"
> + depends on DEBUG_KERNEL
> + default y
Did you really mean default y?
Doing so means it will be turned on more or less everywhere, which defeats the
purpose of having a config option in the first place.
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index ec1a286..0b70b73 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2660,6 +2660,9 @@ static noinline void __schedule_bug(struct task_struct *prev)
> */
> static inline void schedule_debug(struct task_struct *prev)
> {
> +#ifdef CONFIG_SCHED_STACK_END_CHECK
> + BUG_ON(unlikely(task_stack_end_corrupted(prev)))
> +#endif
If this was my code I'd make you put that in a static inline.
cheers
^ permalink raw reply
* Re: [PATCH V2] powerpc/eeh: Fix kernel crash when passing through VF
From: Gavin Shan @ 2014-09-12 5:05 UTC (permalink / raw)
To: Michael Ellerman; +Cc: Wei Yang, linuxppc-dev, gwshan
In-Reply-To: <1410494123.17540.2.camel@concordia>
On Fri, Sep 12, 2014 at 01:55:23PM +1000, Michael Ellerman wrote:
>On Thu, 2014-09-11 at 11:42 +0800, Wei Yang wrote:
>> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>> index 4a45ba8..403445e 100644
>> --- a/arch/powerpc/kernel/eeh.c
>> +++ b/arch/powerpc/kernel/eeh.c
>> @@ -625,7 +625,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
>> int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
>> {
>> struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
>> - struct eeh_pe *pe = edev->pe;
>> + struct eeh_pe *pe = edev ? edev->pe : NULL;
>>
>> if (!pe) {
>> pr_err("%s: No PE found on PCI device %s\n",
>
>
>We seem to do this or something similar in a few places. Is it worth having a
>pci_dev_to_eeh_pe() inline?
>
Yes, maybe we just need a eeh_dev_to_pe() because converting
pci_dev to eeh_dev is already coverred by pci_dev_to_eeh_dev().
With eeh_dev_to_pe(), it looks like this:
struct pci_dev *pdev;
struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
struct eeh_pe *pe = eeh_dev_to_pe(edev);
Or another case:
struct device_node *dn;
struct eeh_dev *edev = of_node_to_eeh_dev(dn);
struct eeh_pe *pe = eeh_dev_to_pe(edev);
Thanks,
Gavin
>cheers
>
>
^ permalink raw reply
* Re: [PATCH V2] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Shengjiu Wang @ 2014-09-12 5:11 UTC (permalink / raw)
To: Timur Tabi
Cc: alsa-devel, tiwai, Li.Xiubo, lgirdwood, perex, nicoleotsuka,
broonie, linuxppc-dev, linux-kernel
In-Reply-To: <54125DEF.4090101@tabi.org>
On Thu, Sep 11, 2014 at 09:43:59PM -0500, Timur Tabi wrote:
> Shengjiu Wang wrote:
> >+ ret = clk_prepare_enable(ssi_private->clk);
> >+ if (ret)
> >+ return ret;
>
> Will this work on PowerPC, where ssi_private->clk is always NULL?
When ssi_private->clk is NULL, then ret = 0, so here will not return,
the code in below will be continuously executed.
I think it work on PowerPC.
wang shengjiu
^ permalink raw reply
* Re: [PATCH v3 3/3] sched: BUG when stack end location is over written
From: Michael Ellerman @ 2014-09-12 6:04 UTC (permalink / raw)
To: Aaron Tomlin
Cc: dzickus, jcastillo, riel, prarit, pzijlstr, peterz, bmr, x86,
oleg, rostedt, linux-kernel, minchan, mingo, tglx, aneesh.kumar,
hannes, akpm, linuxppc-dev, jgh, akpm
In-Reply-To: <1410450088-18236-4-git-send-email-atomlin@redhat.com>
On Thu, 2014-09-11 at 16:41 +0100, Aaron Tomlin wrote:
> Currently in the event of a stack overrun a call to schedule()
> does not check for this type of corruption. This corruption is
> often silent and can go unnoticed. However once the corrupted
> region is examined at a later stage, the outcome is undefined
> and often results in a sporadic page fault which cannot be
> handled.
>
> This patch checks for a stack overrun and takes appropriate
> action since the damage is already done, there is no point
> in continuing.
>
> Signed-off-by: Aaron Tomlin <atomlin@redhat.com>
> ---
> kernel/sched/core.c | 3 +++
> lib/Kconfig.debug | 12 ++++++++++++
> 2 files changed, 15 insertions(+)
>
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index ec1a286..0b70b73 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2660,6 +2660,9 @@ static noinline void __schedule_bug(struct task_struct *prev)
> */
> static inline void schedule_debug(struct task_struct *prev)
> {
> +#ifdef CONFIG_SCHED_STACK_END_CHECK
> + BUG_ON(unlikely(task_stack_end_corrupted(prev)))
> +#endif
Spot the bug? Please compile your code in future.
../kernel/sched/core.c: In function ‘schedule_debug’:
../kernel/sched/core.c:2671:2: error: expected ‘;’ before ‘if’
if (unlikely(in_atomic_preempt_off() && prev->state != TASK_DEAD))
^
../kernel/sched/core.c: At top level:
../kernel/sched/core.c:2635:22: warning: ‘__schedule_bug’ defined but not used [-Wunused-function]
static noinline void __schedule_bug(struct task_struct *prev)
^
make[3]: *** [kernel/sched/core.o] Error 1
make[2]: *** [kernel/sched] Error 2
make[1]: *** [kernel] Error 2
make: *** [sub-make] Error 2
cheers
^ permalink raw reply
* Re: [PATCH V2] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Markus Pargmann @ 2014-09-12 6:17 UTC (permalink / raw)
To: Shengjiu Wang
Cc: alsa-devel, lgirdwood, tiwai, Li.Xiubo, timur, perex,
Nicolin Chen, broonie, linuxppc-dev, linux-kernel
In-Reply-To: <20140912020110.GA414@audiosh1>
[-- Attachment #1: Type: text/plain, Size: 1794 bytes --]
Hi,
On Fri, Sep 12, 2014 at 10:01:12AM +0800, Shengjiu Wang wrote:
> On Thu, Sep 11, 2014 at 03:57:37PM -0700, Nicolin Chen wrote:
> > On Thu, Sep 11, 2014 at 01:38:29PM +0800, Shengjiu Wang wrote:
> > > Move the ipg clock enable and disable operation to startup and shutdown,
> > > that is only enable ipg clock when ssi is working. Keep clock is disabled
> > > when ssi is in idle.
> > > otherwise, _fsl_ssi_set_dai_fmt function need to be called in probe,
> > > so add ipg clock control for it.
> >
> > It seems to be no objection so far against my last suggestion to
> > use regmap's mmio_clk() for named ipg clk only. So you may still
> > consider about that.
> >
> I think mmio_clk() can be put to another patch. and this patch only for clk_enable()
> and clk_disable() operation.
I would also prefer Nicolin's suggestion using regmap's mmio clk. I
think it may be better to not add this particular patch at all and just
go with the mmio_clk patch. It should be easy enough to just add the
clock names to the devicetrees. That way we can avoid all those clock
enable/disable function calls.
>
> > Anyway, I'd like to do thing in parallel. So I just simply tested
> > it on my side and its works fine, it may still need to be tested
> > by others though.
> >
> > Nicolina
>
> Hi Markus
>
> could you please review it, and share your comments?
I think the clock enabling for AC97 is missing in your patch.
Best regards,
Markus
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH V2] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Shengjiu Wang @ 2014-09-12 7:14 UTC (permalink / raw)
To: Markus Pargmann
Cc: alsa-devel, lgirdwood, tiwai, Li.Xiubo, timur, perex,
Nicolin Chen, broonie, linuxppc-dev, linux-kernel
In-Reply-To: <20140912061706.GA10680@pengutronix.de>
On Fri, Sep 12, 2014 at 08:17:06AM +0200, Markus Pargmann wrote:
> Hi,
>
> On Fri, Sep 12, 2014 at 10:01:12AM +0800, Shengjiu Wang wrote:
> > On Thu, Sep 11, 2014 at 03:57:37PM -0700, Nicolin Chen wrote:
> > > On Thu, Sep 11, 2014 at 01:38:29PM +0800, Shengjiu Wang wrote:
> > > > Move the ipg clock enable and disable operation to startup and shutdown,
> > > > that is only enable ipg clock when ssi is working. Keep clock is disabled
> > > > when ssi is in idle.
> > > > otherwise, _fsl_ssi_set_dai_fmt function need to be called in probe,
> > > > so add ipg clock control for it.
> > >
> > > It seems to be no objection so far against my last suggestion to
> > > use regmap's mmio_clk() for named ipg clk only. So you may still
> > > consider about that.
> > >
> > I think mmio_clk() can be put to another patch. and this patch only for clk_enable()
> > and clk_disable() operation.
>
> I would also prefer Nicolin's suggestion using regmap's mmio clk. I
> think it may be better to not add this particular patch at all and just
> go with the mmio_clk patch. It should be easy enough to just add the
> clock names to the devicetrees. That way we can avoid all those clock
> enable/disable function calls.
>
I considered if use Nicolin's suggestion, I still need ot add those
enable/disable function calls, because I want to remove the enable/disable
function call in fsl_ssi_imx_probe, then I need to add enable/disable
function call in _fsl_ssi_set_dai_fmt(), which is called in fsl_ssi_probe().
_fsl_ssi_set_dai_fmt() need to access the registers.
> >
> > > Anyway, I'd like to do thing in parallel. So I just simply tested
> > > it on my side and its works fine, it may still need to be tested
> > > by others though.
> > >
> > > Nicolina
> >
> > Hi Markus
> >
> > could you please review it, and share your comments?
>
> I think the clock enabling for AC97 is missing in your patch.
>
I add clock enable in fsl_ssi_startup(), I think it is enough for AC97, does it?
wang shengjiu
> Best regards,
>
> Markus
>
> --
> Pengutronix e.K. | |
> Industrial Linux Solutions | http://www.pengutronix.de/ |
> Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
> Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH v3 1/3] init/main.c: Give init_task a canary
From: Michael Ellerman @ 2014-09-12 7:28 UTC (permalink / raw)
To: Aaron Tomlin
Cc: dzickus, jcastillo, riel, prarit, pzijlstr, peterz, bmr, x86,
oleg, rostedt, linux-kernel, minchan, mingo, tglx, aneesh.kumar,
hannes, akpm, linuxppc-dev, jgh, akpm
In-Reply-To: <1410450088-18236-2-git-send-email-atomlin@redhat.com>
On Thu, 2014-09-11 at 16:41 +0100, Aaron Tomlin wrote:
> Tasks get their end of stack set to STACK_END_MAGIC with the
> aim to catch stack overruns. Currently this feature does not
> apply to init_task. This patch removes this restriction.
>
> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
> index 51ab9e7..35d0760c 100644
> --- a/arch/powerpc/mm/fault.c
> +++ b/arch/powerpc/mm/fault.c
> @@ -30,7 +30,6 @@
> #include <linux/kprobes.h>
> #include <linux/kdebug.h>
> #include <linux/perf_event.h>
> -#include <linux/magic.h>
> #include <linux/ratelimit.h>
> #include <linux/context_tracking.h>
>
> @@ -538,7 +537,7 @@ void bad_page_fault(struct pt_regs *regs, unsigned long address, int sig)
> regs->nip);
>
> stackend = end_of_stack(current);
> - if (current != &init_task && *stackend != STACK_END_MAGIC)
> + if (*stackend != STACK_END_MAGIC)
> printk(KERN_ALERT "Thread overran stack, or stack corrupted\n");
This part looks fine.
Acked-by: Michael Ellerman <mpe@ellerman.id.au>
cheers
^ permalink raw reply
* Re: [PATCH] Avoid bashisms
From: Michael Ellerman @ 2014-09-12 8:25 UTC (permalink / raw)
To: av1474; +Cc: linuxppc-dev
In-Reply-To: <87zjfkvyha.fsf@comtv.ru>
On Mon, 2014-08-04 at 23:36 +0400, av1474@comtv.ru wrote:
> From 300a98f895dc7e2167cf379408322a9607907761 Mon Sep 17 00:00:00 2001
> From: malc <av1474@comtv.ru>
> Date: Mon, 4 Aug 2014 23:28:05 +0400
> Subject: [PATCH] Avoid bashisms
Hi malc,
If you want us to take your patch, please resend with:
- a better subject, eg:
powerpc: Avoid bashisms in prom_init_check.sh
- a Signed-off-by line.
cheers
^ permalink raw reply
* Re: [PATCH] powerpc: make of_device_ids const
From: Michael Ellerman @ 2014-09-12 8:34 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: devicetree, Rob Herring, Paul Mackerras, kernel, Grant Likely,
linuxppc-dev
In-Reply-To: <1410378998-8823-1-git-send-email-u.kleine-koenig@pengutronix.de>
On Wed, 2014-09-10 at 21:56 +0200, Uwe Kleine-König wrote:
> of_device_ids (i.e. compatible strings and the respective data) are not
> supposed to change at runtime. All functions working with of_device_ids
> provided by <linux/of.h> work with const of_device_ids. This allows to
> mark all struct of_device_id const, too.
>
> While touching these line also put the __init annotation at the right
> position where necessary.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
>
> I don't know how arch/powerpc is maintained. So please tell me if I
> should split this patch further.
>
> I manually checked that all const annotations are OK, and the 0day build
> bot didn't find a regression.
Thanks.
We'll take it as-is, it seems to merge cleanly.
We track patches via patchwork, this one is at:
http://patchwork.ozlabs.org/patch/387959/
cheers
^ permalink raw reply
* RE: [PATCH v3 0/3] sched: Always check the integrity of the canary
From: David Laight @ 2014-09-12 8:43 UTC (permalink / raw)
To: 'Chuck Ebbert'
Cc: jcastillo@redhat.com, akpm@google.com, peterz@infradead.org,
bmr@redhat.com, linux-kernel@vger.kernel.org, prarit@redhat.com,
x86@kernel.org, mingo@redhat.com, 'Aaron Tomlin',
dzickus@redhat.com, riel@redhat.com, mingo@kernel.com,
rostedt@goodmis.org, tglx@linutronix.de, jgh@redhat.com,
pzijlstr@redhat.com, oleg@redhat.com, minchan@kernel.org,
aneesh.kumar@linux.vnet.ibm.com, hannes@cmpxchg.org,
akpm@linux-foundation.org, linuxppc-dev@lists.ozlabs.org
In-Reply-To: <20140911122619.51ed1918@as>
From: Chuck Ebbert=20
> David Laight <David.Laight@ACULAB.COM> wrote:
>=20
> > From: Aaron Tomlin
> > > Currently in the event of a stack overrun a call to schedule()
> > > does not check for this type of corruption. This corruption is
> > > often silent and can go unnoticed. However once the corrupted
> > > region is examined at a later stage, the outcome is undefined
> > > and often results in a sporadic page fault which cannot be
> > > handled.
> > >
> > > The first patch adds a canary to init_task's end of stack.
> > > While the second patch provides a helper to determine the
> > > integrity of the canary. The third checks for a stack
> > > overrun and takes appropriate action since the damage
> > > is already done, there is no point in continuing.
> >
> > Clearly you've just been 'bitten' by a kernel stack overflow.
> > But a simple 'canary' isn't going to find most of the overflows
> > and will give an incorrect 'sense of security'.
> >
> > The canary will only work if the stack is densely written.
> > In practise the stack alignment rules create gaps, and the
> > most likely reason for overflow is a large on-stack buffer
> > that isn't actually written to.
> >
> > The only real way to detect kernel stack overflow is to arrange
> > for an unmapped page beyond the stack.
> > That costs KVA, but not much else.
> >
>=20
> That doesn't work either, because the threadinfo sits between the end of =
the
> stack and the beginning of the next page, making it possible to corrupt c=
ritical
> data without running off the page.
Then flip the order of the allocations so that the threadinfo is at the oth=
er end.
I'm not sure how many per-lwp structures the linux kernel has, but
I know that on netbsd the main thing the kernel stack hits is the fpu
save area - the end of that is the avx area which won't be needed when
the stack usage is large.
Everything else could be moved from the stack pages to the lwp struct itsel=
f.
David
^ permalink raw reply
* Re: [PATCH 2/2] pseries: Fix endian issues in cpu hot-removal
From: Michael Ellerman @ 2014-09-12 8:53 UTC (permalink / raw)
To: Thomas Falcon; +Cc: linuxppc-dev
In-Reply-To: <1410388904-20061-1-git-send-email-tlfalcon@linux.vnet.ibm.com>
On Wed, 2014-09-10 at 17:41 -0500, Thomas Falcon wrote:
> When removing a cpu, this patch makes sure that values
> gotten from or passed to firmware are in the correct
> endian format.
>
> Signed-off-by: Thomas Falcon <tlfalcon@linux.vnet.ibm.com>
> ---
> arch/powerpc/platforms/pseries/dlpar.c | 14 +++++++-------
> arch/powerpc/platforms/pseries/hotplug-cpu.c | 8 ++++----
> 2 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/dlpar.c b/arch/powerpc/platforms/pseries/dlpar.c
> index cd425dc..c5ecfdb 100644
> --- a/arch/powerpc/platforms/pseries/dlpar.c
> +++ b/arch/powerpc/platforms/pseries/dlpar.c
> @@ -442,7 +442,7 @@ static int dlpar_offline_cpu(struct device_node *dn)
> int rc = 0;
> unsigned int cpu;
> int len, nthreads, i;
> - const u32 *intserv;
> + const __be32 *intserv;
>
> intserv = of_get_property(dn, "ibm,ppc-interrupt-server#s", &len);
> if (!intserv)
> @@ -453,7 +453,7 @@ static int dlpar_offline_cpu(struct device_node *dn)
> cpu_maps_update_begin();
> for (i = 0; i < nthreads; i++) {
Can you please do the conversion once here for each value of i.
You can call the converted value "thread" ?
> for_each_present_cpu(cpu) {
> - if (get_hard_smp_processor_id(cpu) != intserv[i])
> + if (get_hard_smp_processor_id(cpu) != be32_to_cpu(intserv[i]))
> continue;
Rather than doing it for every cpu in the system for every value of i.
Not that performance is really an issue, but it's just ugly.
And obviously the other places that use it in the loop should use the converted
value.
> @@ -494,7 +494,7 @@ out:
> static ssize_t dlpar_cpu_release(const char *buf, size_t count)
> {
> struct device_node *dn;
> - const u32 *drc_index;
> + const __be32 *drc_index;
> int rc;
>
> dn = of_find_node_by_path(buf);
> @@ -513,7 +513,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
> return -EINVAL;
> }
Here again you should do the conversion once.
Better still use of_property_read_u32().
> - rc = dlpar_release_drc(*drc_index);
> + rc = dlpar_release_drc(be32_to_cpup(drc_index));
> if (rc) {
> of_node_put(dn);
> return rc;
> @@ -521,7 +521,7 @@ static ssize_t dlpar_cpu_release(const char *buf, size_t count)
>
> rc = dlpar_detach_node(dn);
> if (rc) {
> - dlpar_acquire_drc(*drc_index);
> + dlpar_acquire_drc(be32_to_cpup(drc_index));
> return rc;
> }
cheers
^ permalink raw reply
* Re: [PATCH V2] ASoC: fsl_ssi: refine ipg clock usage in this module
From: Markus Pargmann @ 2014-09-12 8:54 UTC (permalink / raw)
To: Shengjiu Wang
Cc: alsa-devel, lgirdwood, tiwai, Li.Xiubo, timur, perex,
Nicolin Chen, broonie, linuxppc-dev, linux-kernel
In-Reply-To: <20140912071426.GA30261@audiosh1>
[-- Attachment #1: Type: text/plain, Size: 3129 bytes --]
On Fri, Sep 12, 2014 at 03:14:28PM +0800, Shengjiu Wang wrote:
> On Fri, Sep 12, 2014 at 08:17:06AM +0200, Markus Pargmann wrote:
> > Hi,
> >
> > On Fri, Sep 12, 2014 at 10:01:12AM +0800, Shengjiu Wang wrote:
> > > On Thu, Sep 11, 2014 at 03:57:37PM -0700, Nicolin Chen wrote:
> > > > On Thu, Sep 11, 2014 at 01:38:29PM +0800, Shengjiu Wang wrote:
> > > > > Move the ipg clock enable and disable operation to startup and shutdown,
> > > > > that is only enable ipg clock when ssi is working. Keep clock is disabled
> > > > > when ssi is in idle.
> > > > > otherwise, _fsl_ssi_set_dai_fmt function need to be called in probe,
> > > > > so add ipg clock control for it.
> > > >
> > > > It seems to be no objection so far against my last suggestion to
> > > > use regmap's mmio_clk() for named ipg clk only. So you may still
> > > > consider about that.
> > > >
> > > I think mmio_clk() can be put to another patch. and this patch only for clk_enable()
> > > and clk_disable() operation.
> >
> > I would also prefer Nicolin's suggestion using regmap's mmio clk. I
> > think it may be better to not add this particular patch at all and just
> > go with the mmio_clk patch. It should be easy enough to just add the
> > clock names to the devicetrees. That way we can avoid all those clock
> > enable/disable function calls.
> >
> I considered if use Nicolin's suggestion, I still need ot add those
> enable/disable function calls, because I want to remove the enable/disable
> function call in fsl_ssi_imx_probe, then I need to add enable/disable
> function call in _fsl_ssi_set_dai_fmt(), which is called in fsl_ssi_probe().
> _fsl_ssi_set_dai_fmt() need to access the registers.
I think Nicolin's suggestion was to check for a clock named "ipg". If it
exists, you can simply use devm_regmap_init_mmio_clk(). Otherwise you
could fallback to the old behaviour and get the first clock and enable
it without disabling it after the probe function.
After that, you could add the clock-names property to the devicetrees
and have the same result.
>
> > >
> > > > Anyway, I'd like to do thing in parallel. So I just simply tested
> > > > it on my side and its works fine, it may still need to be tested
> > > > by others though.
> > > >
> > > > Nicolina
> > >
> > > Hi Markus
> > >
> > > could you please review it, and share your comments?
> >
> > I think the clock enabling for AC97 is missing in your patch.
> >
> I add clock enable in fsl_ssi_startup(), I think it is enough for AC97, does it?
No. We export ac97 read/write ops for the whole AC97 stuff. These may be
used outside of the usual startup/shutdown which is done for substreams.
It may work to have the ipg clock enabled in
fsl_ssi_ac97_read/fsl_ssi_ac97_write.
Best regards,
Markus
--
Pengutronix e.K. | |
Industrial Linux Solutions | http://www.pengutronix.de/ |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* Re: [PATCH V2] powerpc/eeh: Fix kernel crash when passing through VF
From: Wei Yang @ 2014-09-12 9:35 UTC (permalink / raw)
To: Gavin Shan; +Cc: Wei Yang, linuxppc-dev
In-Reply-To: <20140912050518.GA17933@shangw>
On Fri, Sep 12, 2014 at 03:05:18PM +1000, Gavin Shan wrote:
>On Fri, Sep 12, 2014 at 01:55:23PM +1000, Michael Ellerman wrote:
>>On Thu, 2014-09-11 at 11:42 +0800, Wei Yang wrote:
>>> diff --git a/arch/powerpc/kernel/eeh.c b/arch/powerpc/kernel/eeh.c
>>> index 4a45ba8..403445e 100644
>>> --- a/arch/powerpc/kernel/eeh.c
>>> +++ b/arch/powerpc/kernel/eeh.c
>>> @@ -625,7 +625,7 @@ int eeh_pci_enable(struct eeh_pe *pe, int function)
>>> int pcibios_set_pcie_reset_state(struct pci_dev *dev, enum pcie_reset_state state)
>>> {
>>> struct eeh_dev *edev = pci_dev_to_eeh_dev(dev);
>>> - struct eeh_pe *pe = edev->pe;
>>> + struct eeh_pe *pe = edev ? edev->pe : NULL;
>>>
>>> if (!pe) {
>>> pr_err("%s: No PE found on PCI device %s\n",
>>
>>
>>We seem to do this or something similar in a few places. Is it worth having a
>>pci_dev_to_eeh_pe() inline?
>>
>
>Yes, maybe we just need a eeh_dev_to_pe() because converting
>pci_dev to eeh_dev is already coverred by pci_dev_to_eeh_dev().
>
>With eeh_dev_to_pe(), it looks like this:
>
>struct pci_dev *pdev;
>struct eeh_dev *edev = pci_dev_to_eeh_dev(pdev);
>struct eeh_pe *pe = eeh_dev_to_pe(edev);
>
>Or another case:
>
>struct device_node *dn;
>struct eeh_dev *edev = of_node_to_eeh_dev(dn);
>struct eeh_pe *pe = eeh_dev_to_pe(edev);
With these helper, it would be more consolidate to jump between those data.
Gavin,
You would add these helpers? Or would like me to add them?
>
>Thanks,
>Gavin
>
>>cheers
>>
>>
--
Richard Yang
Help you, Help me
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox