Re: [PATCH] soc: fsl: guts: us devm_kstrdup_const() for RO data
On Thu, Jan 10, 2019 at 01:43:01PM -0600, Li Yang wrote: > On Sat, Dec 22, 2018 at 2:02 AM Nicholas Mc Guire wrote: > > > > On Fri, Dec 21, 2018 at 08:29:56PM -0600, Scott Wood wrote: > > > On Fri, 2018-12-07 at 09:22 +0100, Nicholas Mc Guire wrote: > > > > devm_kstrdup() may return NULL if internal allocation failed, but > > > > as machine is from the device tree, and thus RO, devm_kstrdup_const() > > > > can be used here, which will only copy the reference. > > > > > > Is it really going to only copy the reference? That would require that > > > is_kernel_rodata(machine) be true, which it shouldn't be since it's not > > > part > > > of the kernel image. > > > > > I had tried to figure out what is RO and what not but was not > > able to determine that - from the discussion it seemed that the > > assumption of RO is correct though I did not ask if it would > > satisfy is_kernel_rodata() so that explains the incorrect assertion. > > see https://lkml.org/lkml/2018/12/6/42 > > So then the only option is to check the return and cleanup > > on allocation failure as the orriginal patch proposed. > > Thanks for the good discussion. I will drop the previous patch. But > would it also be good to just have "soc_dev_attr.machine = machine" > directly? > I think that the intent is to switch to managed devm API so that the cleanup is handled properly currently you would get "machine" from of_property_read_string_index -> of_property_read_string_helper -> of_find_property which does not do any allocation - so there would actually not be anything to cleanup here - donĀ“t see why your solution would not be suitable given the current API. the only advantage of the devm_kstrdup() is that underlying APIs internal changes would have no effect. thx! hofrat
Re: [PATCH] soc: fsl: guts: us devm_kstrdup_const() for RO data
On Fri, Dec 21, 2018 at 08:29:56PM -0600, Scott Wood wrote: > On Fri, 2018-12-07 at 09:22 +0100, Nicholas Mc Guire wrote: > > devm_kstrdup() may return NULL if internal allocation failed, but > > as machine is from the device tree, and thus RO, devm_kstrdup_const() > > can be used here, which will only copy the reference. > > Is it really going to only copy the reference? That would require that > is_kernel_rodata(machine) be true, which it shouldn't be since it's not part > of the kernel image. > I had tried to figure out what is RO and what not but was not able to determine that - from the discussion it seemed that the assumption of RO is correct though I did not ask if it would satisfy is_kernel_rodata() so that explains the incorrect assertion. see https://lkml.org/lkml/2018/12/6/42 So then the only option is to check the return and cleanup on allocation failure as the orriginal patch proposed. thanks for clearifying this ! hofrat
[PATCH] soc: fsl: guts: us devm_kstrdup_const() for RO data
devm_kstrdup() may return NULL if internal allocation failed, but as machine is from the device tree, and thus RO, devm_kstrdup_const() can be used here, which will only copy the reference. Signed-off-by: Nicholas Mc Guire Fixes: a6fc3b698130 ("soc: fsl: add GUTS driver for QorIQ platforms") --- Problem located by experimental coccinelle script Patch was compile tested with: multi_v7_defconfig (implies FSL_GUTS=y) Patch is against 4.20-rc5 (localversion-next is next-20181207) drivers/soc/fsl/guts.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c index 302e0c8..15071ec 100644 --- a/drivers/soc/fsl/guts.c +++ b/drivers/soc/fsl/guts.c @@ -157,7 +157,8 @@ static int fsl_guts_probe(struct platform_device *pdev) of_property_read_string_index(root, "compatible", 0, ); of_node_put(root); if (machine) - soc_dev_attr.machine = devm_kstrdup(dev, machine, GFP_KERNEL); + soc_dev_attr.machine = devm_kstrdup_const(dev, machine, + GFP_KERNEL); svr = fsl_guts_get_svr(); soc_die = fsl_soc_die_match(svr, fsl_soc_die); -- 2.1.4
Re: [RFC PATCH] soc: fsl: guts: handle devm_kstrdup() failure
On Wed, Dec 05, 2018 at 02:42:55PM -0600, Li Yang wrote: > On Sun, Dec 2, 2018 at 3:07 AM Nicholas Mc Guire wrote: > > > > devm_kstrdup() may return NULL if internal allocation failed. > > soc_dev_attr.machine should be checked (although its only use > > in pr_info() would be safe even with a NULL). Therefor > > in the unlikely case of allocation failure, fsl_guts_probe() returns > > -ENOMEM as this allocating failing is an indication of something > > more serious going wrong at system level. > > > > As machine is from the device tree which I assume to be RO - if > > that assumption is always correct - a better alternative would be > > to use devm_kstrdup_const() here. That would then simply copy the > > reference to the RO data and not perform any allocation at all. > > I think your assumption is correct. Do you want to send a new and > better version? :) The issue was actually more general that I did not find a reliable method to assure that some object is *always* RO. Even for device tree data it was not clear to me if there could be systems where it is not RO. Anyway - will send the version using devm_kstrdup_const() then. thx! hofrat > > > > > Signed-off-by: Nicholas Mc Guire > > Fixes: a6fc3b698130 ("soc: fsl: add GUTS driver for QorIQ platforms") > > --- > > > > Problem located by experimental coccinelle script > > > > Patch was compile tested with: multi_v7_defconfig (implies FSL_GUTS=y) > > > > Patch is against 4.20-rc4 (localversion-next is next-20181130) > > > > drivers/soc/fsl/guts.c | 5 - > > 1 file changed, 4 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c > > index 302e0c8..a0c751b 100644 > > --- a/drivers/soc/fsl/guts.c > > +++ b/drivers/soc/fsl/guts.c > > @@ -156,8 +156,11 @@ static int fsl_guts_probe(struct platform_device *pdev) > > if (of_property_read_string(root, "model", )) > > of_property_read_string_index(root, "compatible", 0, > > ); > > of_node_put(root); > > - if (machine) > > + if (machine) { > > soc_dev_attr.machine = devm_kstrdup(dev, machine, > > GFP_KERNEL); > > + if (!soc_dev_attr.machine) > > + return -ENOMEM; > > + } > > > > svr = fsl_guts_get_svr(); > > soc_die = fsl_soc_die_match(svr, fsl_soc_die); > > -- > > 2.1.4 > >
[RFC PATCH] soc: fsl: guts: handle devm_kstrdup() failure
devm_kstrdup() may return NULL if internal allocation failed. soc_dev_attr.machine should be checked (although its only use in pr_info() would be safe even with a NULL). Therefor in the unlikely case of allocation failure, fsl_guts_probe() returns -ENOMEM as this allocating failing is an indication of something more serious going wrong at system level. As machine is from the device tree which I assume to be RO - if that assumption is always correct - a better alternative would be to use devm_kstrdup_const() here. That would then simply copy the reference to the RO data and not perform any allocation at all. Signed-off-by: Nicholas Mc Guire Fixes: a6fc3b698130 ("soc: fsl: add GUTS driver for QorIQ platforms") --- Problem located by experimental coccinelle script Patch was compile tested with: multi_v7_defconfig (implies FSL_GUTS=y) Patch is against 4.20-rc4 (localversion-next is next-20181130) drivers/soc/fsl/guts.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/soc/fsl/guts.c b/drivers/soc/fsl/guts.c index 302e0c8..a0c751b 100644 --- a/drivers/soc/fsl/guts.c +++ b/drivers/soc/fsl/guts.c @@ -156,8 +156,11 @@ static int fsl_guts_probe(struct platform_device *pdev) if (of_property_read_string(root, "model", )) of_property_read_string_index(root, "compatible", 0, ); of_node_put(root); - if (machine) + if (machine) { soc_dev_attr.machine = devm_kstrdup(dev, machine, GFP_KERNEL); + if (!soc_dev_attr.machine) + return -ENOMEM; + } svr = fsl_guts_get_svr(); soc_die = fsl_soc_die_match(svr, fsl_soc_die); -- 2.1.4
[PATCH 1/2] usb: gadget: fsl_udc_core: check allocation return value and cleanup on failure
The allocation with fsl_alloc_request() and kmalloc() were unchecked fixed this up with a NULL check and appropriate cleanup. Additionally udc->ep_qh_size was reset to 0 on failure of allocation. Similar udc->phy_mode is initially 0 (as udc_controller was allocated with kzalloc in fsl_udc_probe()) so reset it to 0 as well so that this function is side-effect free on failure. Not clear if this is necessary or sensible as fsl_udc_release() probably can not be called if fsl_udc_probe() failed - but it should not hurt. Signed-off-by: Nicholas Mc Guire Fixes: b504882da5 ("USB: add Freescale high-speed USB SOC device controller driver") --- Problem located with experimental coccinelle script Patch was compile tested with: imx_v6_v7_defconfig (implies USB_FSL_USB2=y) (with a large number of sparse warnings not related to the proposed change and one smatch warning) Patch is against 4.19-rc1 (localversion-next is next-20180830) drivers/usb/gadget/udc/fsl_udc_core.c | 30 ++ 1 file changed, 26 insertions(+), 4 deletions(-) diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c index be59309..e637afb 100644 --- a/drivers/usb/gadget/udc/fsl_udc_core.c +++ b/drivers/usb/gadget/udc/fsl_udc_core.c @@ -2247,8 +2247,10 @@ static int struct_udc_setup(struct fsl_udc *udc, udc->phy_mode = pdata->phy_mode; udc->eps = kcalloc(udc->max_ep, sizeof(struct fsl_ep), GFP_KERNEL); - if (!udc->eps) - return -1; + if (!udc->eps) { + ERR("kmalloc udc endpoint status failed\n"); + goto eps_alloc_failed; + } /* initialized QHs, take care of alignment */ size = udc->max_ep * sizeof(struct ep_queue_head); @@ -2262,8 +2264,7 @@ static int struct_udc_setup(struct fsl_udc *udc, >ep_qh_dma, GFP_KERNEL); if (!udc->ep_qh) { ERR("malloc QHs for udc failed\n"); - kfree(udc->eps); - return -1; + goto ep_queue_alloc_failed; } udc->ep_qh_size = size; @@ -2272,8 +2273,17 @@ static int struct_udc_setup(struct fsl_udc *udc, /* FIXME: fsl_alloc_request() ignores ep argument */ udc->status_req = container_of(fsl_alloc_request(NULL, GFP_KERNEL), struct fsl_req, req); + if (!udc->status_req) { + ERR("kzalloc for udc status request failed\n"); + goto udc_status_alloc_failed; + } + /* allocate a small amount of memory to get valid address */ udc->status_req->req.buf = kmalloc(8, GFP_KERNEL); + if (!udc->status_req->req.buf) { + ERR("kzalloc for udc request buffer failed\n"); + goto udc_req_buf_alloc_failed; + } udc->resume_state = USB_STATE_NOTATTACHED; udc->usb_state = USB_STATE_POWERED; @@ -2281,6 +2291,18 @@ static int struct_udc_setup(struct fsl_udc *udc, udc->remote_wakeup = 0; /* default to 0 on reset */ return 0; + +udc_req_buf_alloc_failed: + kfree(udc->status_req); +udc_status_alloc_failed: + kfree(udc->ep_qh); + udc->ep_qh_size = 0; +ep_queue_alloc_failed: + kfree(udc->eps); +eps_alloc_failed: + udc->phy_mode = 0; + return -1; + } /* -- 2.1.4
[PATCH 2/2] usb: gadget: fsl_udc_core: fixup struct_udc_setup documentation
The original implementation from commit b504882da539 ("USB: add Freescale high-speed USB SOC device controller driver") returned NULL on failure and an allocated + initialized struct fsl_udc on success. The current code introduced in commit 4365831dadfe ("USB: fsl_usb2_udc: Get max ep number from DCCPARAMS register") only provides partial initialization as well as returning 0 on success and -1 on failures. The function documentation is updated accordingly. Signed-off-by: Nicholas Mc Guire Fixes: 4365831dadfe ("USB: fsl_usb2_udc: Get max ep number from DCCPARAMS register") --- Problem located during code-review Patch is against 4.19-rc1 (localversion-next is next-20180830) drivers/usb/gadget/udc/fsl_udc_core.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c index e637afb..680b46e 100644 --- a/drivers/usb/gadget/udc/fsl_udc_core.c +++ b/drivers/usb/gadget/udc/fsl_udc_core.c @@ -2234,8 +2234,10 @@ static void fsl_udc_release(struct device *dev) Internal structure setup functions ***/ /*-- - * init resource for globle controller - * Return the udc handle on success or NULL on failure + * init resource for global controller called by fsl_udc_probe() + * On success the udc handle is initialized, on failure it is + * unchanged (reset). + * Return 0 on success and -1 on allocation failure --*/ static int struct_udc_setup(struct fsl_udc *udc, struct platform_device *pdev) -- 2.1.4
[PATCH] KVM: PPC: Book3S HV: fix constant size warning
The constants are 64bit but not explicitly declared UL resulting in sparse warnings. Fixed by declaring the constants UL. Signed-off-by: Nicholas Mc Guire --- sparse fallout from compile checking book3s_hv.c: arch/powerpc/kvm/book3s_hv.c:141:9: warning: constant 0x164520C62609AECA is so big it is long arch/powerpc/kvm/book3s_hv.c:142:9: warning: constant 0x164520C62609AECA is so big it is long arch/powerpc/kvm/book3s_hv.c:143:9: warning: constant 0x7FFF2908450D8DA9 is so big it is long arch/powerpc/kvm/book3s_hv.c:144:9: warning: constant 0x164520C62609AECA is so big it is long arch/powerpc/kvm/book3s_hv.c:145:9: warning: constant 0x199A421245058DA9 is so big it is long arch/powerpc/kvm/book3s_hv.c:146:9: warning: constant 0x164520C62609AECA is so big it is long arch/powerpc/kvm/book3s_hv.c:147:9: warning: constant 0x164520C62609AECA is so big it is long arch/powerpc/kvm/book3s_hv.c:148:9: warning: constant 0x164520C62609AECA is so big it is long arch/powerpc/kvm/book3s_hv.c:149:9: warning: constant 0x164520C62609AECA is so big it is long arch/powerpc/kvm/book3s_hv.c:1667:60: warning: constant 0xf3ff is so big it is unsigned long arch/powerpc/kvm/book3s_hv.c:2896:42: warning: constant 0x164520C62609AECA is so big it is long most of the warnings are local constants in book3s_hv.c, PSSCR_GUEST_VIS from reg.h is not. Although PSSCR_GUEST_VIS is not #ifdef CONFIG_PPC_BOOK3S it seems only to be used in that particular platform (check with cscope). The constants fit in the target variables so the size declaration discrepancy does not actually cause a problem here. Patch was compiletested with: ppc64_defconfig (implies CONFIG_KVM=y, CONFIG_KVM_BOOK3S_64_HV=m) Patch is against 4.18-rc3 (localversion-next is next-20180705) arch/powerpc/include/asm/reg.h | 2 +- arch/powerpc/kvm/book3s_hv.c | 16 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h index 5625684..858aa79 100644 --- a/arch/powerpc/include/asm/reg.h +++ b/arch/powerpc/include/asm/reg.h @@ -161,7 +161,7 @@ #define PSSCR_ESL 0x0020 /* Enable State Loss */ #define PSSCR_SD 0x0040 /* Status Disable */ #define PSSCR_PLS 0xf000 /* Power-saving Level Status */ -#define PSSCR_GUEST_VIS0xf3ff /* Guest-visible PSSCR fields */ +#define PSSCR_GUEST_VIS0xf3ffUL /* Guest-visible PSSCR fields */ #define PSSCR_FAKE_SUSPEND 0x0400 /* Fake-suspend bit (P9 DD2.2) */ #define PSSCR_FAKE_SUSPEND_LG 10 /* Fake-suspend bit position */ diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index ee4a885..c517859 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -128,14 +128,14 @@ static int kvmppc_hv_setup_htab_rma(struct kvm_vcpu *vcpu); * and SPURR count and should be set according to the number of * online threads in the vcore being run. */ -#define RWMR_RPA_P8_1THREAD0x164520C62609AECA -#define RWMR_RPA_P8_2THREAD0x7FFF2908450D8DA9 -#define RWMR_RPA_P8_3THREAD0x164520C62609AECA -#define RWMR_RPA_P8_4THREAD0x199A421245058DA9 -#define RWMR_RPA_P8_5THREAD0x164520C62609AECA -#define RWMR_RPA_P8_6THREAD0x164520C62609AECA -#define RWMR_RPA_P8_7THREAD0x164520C62609AECA -#define RWMR_RPA_P8_8THREAD0x164520C62609AECA +#define RWMR_RPA_P8_1THREAD0x164520C62609AECAUL +#define RWMR_RPA_P8_2THREAD0x7FFF2908450D8DA9UL +#define RWMR_RPA_P8_3THREAD0x164520C62609AECAUL +#define RWMR_RPA_P8_4THREAD0x199A421245058DA9UL +#define RWMR_RPA_P8_5THREAD0x164520C62609AECAUL +#define RWMR_RPA_P8_6THREAD0x164520C62609AECAUL +#define RWMR_RPA_P8_7THREAD0x164520C62609AECAUL +#define RWMR_RPA_P8_8THREAD0x164520C62609AECAUL static unsigned long p8_rwmr_values[MAX_SMT_THREADS + 1] = { RWMR_RPA_P8_1THREAD, -- 2.1.4
[PATCH] KVM: PPC: Book3S HV: add of_node_put() in success path
The call to of_find_compatible_node() is returning a pointer with incremented refcount so it must be explicitly decremented after the last use. As here it is only being used for checking of node presence but the result is not actually used in the success path it can be dropped immediately. Signed-off-by: Nicholas Mc Guire Fixes: commit f725758b899f ("KVM: PPC: Book3S HV: Use OPAL XICS emulation on POWER9") --- Problem found by experimental coccinelle script Patch was compiletested with: ppc64_defconfig (implies CONFIG_KVM=y, CONFIG_KVM_BOOK3S_64_HV=m) with many sparse warnings though not related to the proposed change Patch is against 4.18-rc3 (localversion-next is next-20180705) arch/powerpc/kvm/book3s_hv.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c index ee4a885..8680fb9 100644 --- a/arch/powerpc/kvm/book3s_hv.c +++ b/arch/powerpc/kvm/book3s_hv.c @@ -4561,6 +4561,8 @@ static int kvmppc_book3s_init_hv(void) pr_err("KVM-HV: Cannot determine method for accessing XICS\n"); return -ENODEV; } + /* presence of intc confirmed - node can be dropped again */ + of_node_put(np); } #endif -- 2.1.4
[PATCH] powerpc: icp-hv: fix missing of_node_put in success path
Both of_find_compatible_node() and of_find_node_by_type() will return a refcounted node on success - thus for the success path the node must be explicitly released with a of_node_put(). Signed-off-by: Nicholas Mc Guire Fixes: commit 0b05ac6e2480 ("powerpc/xics: Rewrite XICS driver") --- Problem found by experimental coccinelle script Patch was compiletested with: ppc64_defconfig (implies CONFIG_PPC_ICP_HV=y) with sparse warnings though not related to the proposed change Patch is against 4.18-rc3 (localversion-next is next-20180704) arch/powerpc/sysdev/xics/icp-hv.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/sysdev/xics/icp-hv.c b/arch/powerpc/sysdev/xics/icp-hv.c index bbc839a..003deaa 100644 --- a/arch/powerpc/sysdev/xics/icp-hv.c +++ b/arch/powerpc/sysdev/xics/icp-hv.c @@ -179,6 +179,7 @@ int icp_hv_init(void) icp_ops = _hv_ops; + of_node_put(np); return 0; } -- 2.1.4
[PATCH] powerpc: hwrng; fix missing of_node_put()
The call to of_find_compatible_node() returns a node pointer with refcount incremented thus it must be explicitly decremented here before returning. Signed-off-by: Nicholas Mc Guire Fixes: commit a489043f4626 ("powerpc/pseries: Implement arch_get_random_long() based on H_RANDOM") --- Problem found with experimental coccinelle script Patch was compiletested with: ppc64_defconfig (implies CONFIG_PPC_PSERIES=y) with some unrelated sparse warnings (which I did not understand) ./arch/powerpc/include/asm/head-64.h:13:36: warning: Unknown escape '(' ./arch/powerpc/include/asm/head-64.h:16:36: warning: Unknown escape '(' ./arch/powerpc/include/asm/head-64.h:19:36: warning: Unknown escape '(' Patch is aginst 4.18-rc2 (localversion-next is next-20180702) arch/powerpc/platforms/pseries/rng.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/platforms/pseries/rng.c b/arch/powerpc/platforms/pseries/rng.c index 31ca557..262b8c5 100644 --- a/arch/powerpc/platforms/pseries/rng.c +++ b/arch/powerpc/platforms/pseries/rng.c @@ -40,6 +40,7 @@ static __init int rng_init(void) ppc_md.get_random_seed = pseries_get_random_long; + of_node_put(dn); return 0; } machine_subsys_initcall(pseries, rng_init); -- 2.1.4
[PATCH] ALSA: snd-aoa: add of_node_put() in error path
Both calls to of_find_node_by_name() and of_get_next_child() return a node pointer with refcount incremented thus it must be explicidly decremented here after the last usage. As we are assured to have a refcounted np either from the initial of_find_node_by_name(NULL, name); or from the of_get_next_child(gpio, np) in the while loop if we reached the error code path below, an x of_node_put(np) is needed. Signed-off-by: Nicholas Mc Guire Fixes: commit f3d9478b2ce4 ("[ALSA] snd-aoa: add snd-aoa") --- Problem located by an experimental coccinelle script Patch was compiletested with: ppc64_defconfig (implies CONFIG_SND_AOA=m) Patch is against 4.18-rc2 (localversion-next is next-20180629) sound/aoa/core/gpio-feature.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sound/aoa/core/gpio-feature.c b/sound/aoa/core/gpio-feature.c index 7196008..6555742 100644 --- a/sound/aoa/core/gpio-feature.c +++ b/sound/aoa/core/gpio-feature.c @@ -88,8 +88,10 @@ static struct device_node *get_gpio(char *name, } reg = of_get_property(np, "reg", NULL); - if (!reg) + if (!reg) { + of_node_put(np); return NULL; + } *gpioptr = *reg; -- 2.1.4
Re: linux-next: build warnings after merge of the kbuild tree
On Fri, Aug 26, 2016 at 01:58:03PM +1000, Nicholas Piggin wrote: > On Mon, 22 Aug 2016 20:47:58 +1000 > Nicholas Pigginwrote: > > > On Fri, 19 Aug 2016 20:44:55 +1000 > > Nicholas Piggin wrote: > > > > > On Fri, 19 Aug 2016 10:37:00 +0200 > > > Michal Marek wrote: > > > > > > > On 2016-08-19 07:09, Stephen Rothwell wrote: > > > > [snip] > > > > > > > > > > > > I may be missing something, but genksyms generates the crc's off the > > > > > preprocessed C source code and we don't have any for the asm files > > > > > ... > > > > > > > > Of course you are right. Which means that we are losing type information > > > > for these exports for CONFIG_MODVERSIONS purposes. I guess it's > > > > acceptable, since the asm functions are pretty basic and their > > > > signatures do not change. > > > > > > I don't completely agree. It would be nice to have the functionality > > > still there. > > > > > > What happens if you just run cmd_modversions on the as rule? It relies on > > > !defined(__ASSEMBLY__), but we're feeding the result to genksyms, not as. > > > It would require the header be included in the .S file and be protected > > > for > > > asm builds. > > > > > > This seems like it *could* be made to work, but there's a few problems. > > > > - .h files are not made for C consumption. Matter of manually adding the > > ifdef guards, which isn't terrible. > > > > - .S files do not all include their .h where the C declaration is. Also > > will cause some churn but doable and maybe not completely unreasonable. > > > > - genksyms parser barfs when it hits the assembly of the .S file. Best > > way to fix that seems just send the #include and EXPORT_SYMBOL lines > > from the .S to the preprocessor. That's a bit of a rabbit hole too, with > > some .S files being included, etc. > > > > I'm not sure what to do here. If nobody cares and we lose CRCs for .S > > exports, then okay we can whitelist those relocs easily. If we don't want > > to lose the functionality, the above might work but it's a bit intrusive > > an is going to require another cycle of prep patches to go through arch > > code first. > > > > Or suggestions for alternative approach? > > Here is a quick patch that I think should catch missing CRCs in > architecture independent way. If we merge something like this, we > can whitelist the symbols in arch/powerpc so people get steered to > the right place. > > Powerpc seems to be the only one really catching this, and it's > only as a side effect of a test run for CONFIG_RELOCATABLE kernels, > which means version failures probably slipped through other archs. > > I'll clean it up, do some more testing, and submit it unless > anybody dislikes it or has a better way to do it. > > Thanks, > Nick > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c > index 4b8ffd3..1efc454 100644 > --- a/scripts/mod/modpost.c > +++ b/scripts/mod/modpost.c > @@ -609,6 +609,7 @@ static void handle_modversions(struct module *mod, struct > elf_info *info, > { > unsigned int crc; > enum export export; > + int is_crc = 0; should that not be a bool here ? > > if ((!is_vmlinux(mod->name) || mod->is_dot_o) && > strncmp(symname, "__ksymtab", 9) == 0) > @@ -618,6 +619,7 @@ static void handle_modversions(struct module *mod, struct > elf_info *info, > > /* CRC'd symbol */ > if (strncmp(symname, CRC_PFX, strlen(CRC_PFX)) == 0) { > + is_crc = 1; is_crc = true; > crc = (unsigned int) sym->st_value; > sym_update_crc(symname + strlen(CRC_PFX), mod, crc, > export); thx! hofrat
[PATCH] macintosh: ams: cleanup on stack DECLARE_COMPLETION
fix-up for incorrect use of DECLARE_COMPLETION. see also commit 6e9a4738 (completions: lockdep annotate on stack completions) V2: split out patch for individual files and (hopefully) proper labeling this time patch is against linux-next 3.19.0-rc1 -next-20141226 patch was compile tested with ppc6xx_defconfig, CONFIG_SENSORS_AMS=m, CONFIG_SENSORS_AMS_PMU=y Signed-off-by: Nicholas Mc Guire der.h...@hofr.at --- drivers/macintosh/ams/ams-pmu.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/macintosh/ams/ams-pmu.c b/drivers/macintosh/ams/ams-pmu.c index 4f61b3e..c2b178f 100644 --- a/drivers/macintosh/ams/ams-pmu.c +++ b/drivers/macintosh/ams/ams-pmu.c @@ -52,7 +52,7 @@ static void ams_pmu_req_complete(struct adb_request *req) static void ams_pmu_set_register(u8 reg, u8 value) { static struct adb_request req; - DECLARE_COMPLETION(req_complete); + DECLARE_COMPLETION_ONSTACK(req_complete); req.arg = req_complete; if (pmu_request(req, ams_pmu_req_complete, 4, ams_pmu_cmd, 0x00, reg, value)) @@ -65,7 +65,7 @@ static void ams_pmu_set_register(u8 reg, u8 value) static u8 ams_pmu_get_register(u8 reg) { static struct adb_request req; - DECLARE_COMPLETION(req_complete); + DECLARE_COMPLETION_ONSTACK(req_complete); req.arg = req_complete; if (pmu_request(req, ams_pmu_req_complete, 3, ams_pmu_cmd, 0x01, reg)) -- 1.7.10.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] cleanup on stack DECLARE_COMPLETIONs
fixups for incorrect use of DECLARE_COMPLETION. see also commit 6e9a4738 (completions: lockdep annotate on stack completions) The only somewhat special case being drivers/misc/sgi-gru/grukservices.c:quicktest2 which had a static qualifier in the original DECLARE_COMPLETION() but that seems to be wrong (why should the completion persisted between successive calls ?) so the conversion to DECLARE_COMPLETION_ONSTACK was also applied and the static qualifier removed. Not sure if this is suitable in this form or if it should go out as 5 seperate patches ? This was only code reviewed and compile tested Signed-off-by: Nicholas Mc Guire der.h...@hofr.at --- drivers/macintosh/ams/ams-pmu.c |4 ++-- drivers/misc/sgi-gru/grukservices.c |2 +- drivers/scsi/aha152x.c|2 +- drivers/usb/gadget/udc/fsl_qe_udc.c |2 +- drivers/usb/gadget/udc/fsl_udc_core.c |2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/macintosh/ams/ams-pmu.c b/drivers/macintosh/ams/ams-pmu.c index 4f61b3e..c2b178f 100644 --- a/drivers/macintosh/ams/ams-pmu.c +++ b/drivers/macintosh/ams/ams-pmu.c @@ -52,7 +52,7 @@ static void ams_pmu_req_complete(struct adb_request *req) static void ams_pmu_set_register(u8 reg, u8 value) { static struct adb_request req; - DECLARE_COMPLETION(req_complete); + DECLARE_COMPLETION_ONSTACK(req_complete); req.arg = req_complete; if (pmu_request(req, ams_pmu_req_complete, 4, ams_pmu_cmd, 0x00, reg, value)) @@ -65,7 +65,7 @@ static void ams_pmu_set_register(u8 reg, u8 value) static u8 ams_pmu_get_register(u8 reg) { static struct adb_request req; - DECLARE_COMPLETION(req_complete); + DECLARE_COMPLETION_ONSTACK(req_complete); req.arg = req_complete; if (pmu_request(req, ams_pmu_req_complete, 3, ams_pmu_cmd, 0x01, reg)) diff --git a/drivers/misc/sgi-gru/grukservices.c b/drivers/misc/sgi-gru/grukservices.c index 913de07..4e412fe 100644 --- a/drivers/misc/sgi-gru/grukservices.c +++ b/drivers/misc/sgi-gru/grukservices.c @@ -1044,7 +1044,7 @@ done: static int quicktest2(unsigned long arg) { - static DECLARE_COMPLETION(cmp); + DECLARE_COMPLETION_ONSTACK(cmp); unsigned long han; int blade_id = 0; int numcb = 4; diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c index 2b960b3..b16afb9 100644 --- a/drivers/scsi/aha152x.c +++ b/drivers/scsi/aha152x.c @@ -1055,7 +1055,7 @@ static int aha152x_abort(Scsi_Cmnd *SCpnt) static int aha152x_device_reset(Scsi_Cmnd * SCpnt) { struct Scsi_Host *shpnt = SCpnt-device-host; - DECLARE_COMPLETION(done); + DECLARE_COMPLETION_ONSTACK(done); int ret, issued, disconnected; unsigned char old_cmd_len = SCpnt-cmd_len; unsigned long flags; diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c index 795c99c..e0822f1 100644 --- a/drivers/usb/gadget/udc/fsl_qe_udc.c +++ b/drivers/usb/gadget/udc/fsl_qe_udc.c @@ -2630,7 +2630,7 @@ static int qe_udc_remove(struct platform_device *ofdev) struct qe_udc *udc = platform_get_drvdata(ofdev); struct qe_ep *ep; unsigned int size; - DECLARE_COMPLETION(done); + DECLARE_COMPLETION_ONSTACK(done); usb_del_gadget_udc(udc-gadget); diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c index 2df8074..c3830ad 100644 --- a/drivers/usb/gadget/udc/fsl_udc_core.c +++ b/drivers/usb/gadget/udc/fsl_udc_core.c @@ -2529,7 +2529,7 @@ static int __exit fsl_udc_remove(struct platform_device *pdev) struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); struct fsl_usb2_platform_data *pdata = dev_get_platdata(pdev-dev); - DECLARE_COMPLETION(done); + DECLARE_COMPLETION_ONSTACK(done); if (!udc_controller) return -ENODEV; -- 1.7.10.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH] gadget: cleanup on stack DECLARE_COMPLETIONs
fixups for incorrect use of DECLARE_COMPLETION. see also commit 6e9a4738 (completions: lockdep annotate on stack completions) patch is against 3.18.0 linux-next This was only code reviewed and compile tested Signed-off-by: Nicholas Mc Guire der.h...@hofr.at --- drivers/usb/gadget/udc/fsl_qe_udc.c |2 +- drivers/usb/gadget/udc/fsl_udc_core.c |2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c index 795c99c..e0822f1 100644 --- a/drivers/usb/gadget/udc/fsl_qe_udc.c +++ b/drivers/usb/gadget/udc/fsl_qe_udc.c @@ -2630,7 +2630,7 @@ static int qe_udc_remove(struct platform_device *ofdev) struct qe_udc *udc = platform_get_drvdata(ofdev); struct qe_ep *ep; unsigned int size; - DECLARE_COMPLETION(done); + DECLARE_COMPLETION_ONSTACK(done); usb_del_gadget_udc(udc-gadget); diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c b/drivers/usb/gadget/udc/fsl_udc_core.c index 2df8074..c3830ad 100644 --- a/drivers/usb/gadget/udc/fsl_udc_core.c +++ b/drivers/usb/gadget/udc/fsl_udc_core.c @@ -2529,7 +2529,7 @@ static int __exit fsl_udc_remove(struct platform_device *pdev) struct resource *res = platform_get_resource(pdev, IORESOURCE_MEM, 0); struct fsl_usb2_platform_data *pdata = dev_get_platdata(pdev-dev); - DECLARE_COMPLETION(done); + DECLARE_COMPLETION_ONSTACK(done); if (!udc_controller) return -ENODEV; -- 1.7.10.4 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: How to build the kernel without any optimization?
On Fri, 20 Aug 2010, Shawn Jin wrote: Hi, I'm tracing the execution of ds1307_probe() and find that some of variables or function arguments cannot be printed in gdb because they are optimized out or not in the current context. This really gives some headache. Is there a way to build the kernel without any optimization? What gcc option shall I disable or add? I already added the following to arch/powerpc/Makefile. # Prevent GDB from jumping around in the code when trying to single step ifeq ($(CONFIG_DEBUG_KERNEL),y) KBUILD_CFLAGS += -fno-schedule-insns -fno-schedule-insns2 endif much of the kernel can not be build without optimization - what you can do though is slectively try to disable optimization for specific files by putting CFLAGS_REMOVE_objfilenam.o = -SOME_OPT in the Makefile. I think that is safer than what you did above as this would always depend on the order of options that ultimately get passed to gcc. hofrat ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: can the kernel show user task stack backtrace ?
On Thu, 30 Jul 2009, Alessandro Rubini wrote: We're dealing with some complex (3rd party) applications and I like to see a user task stack backtrace. (Of course the way to go here is to use a debugger (gdb) and do a backtrace (with the coredump file). Actually, you can intercept SIGSEGV and print your own stack from within the signal handler. You can also open /proc/self/maps and print it, to ease understanding the various pointers in there, especially if the application is using a number of shared libs. This is usually easier than getting to a core dump, although there is less information than what the core offers. I have the code for ARM and I've it on ppc once, but I must dig for the actual code. I think libSegFault.so (part of glibc) can do that by simply preloading it LD_PRELOAD=/lib/libSegFault.so ./your_segfaulting_app should do the trick. hofrat ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Memory usage of a process
On Fri, 10 Jul 2009, G?nter Leonhardt wrote: Hello, analysing the memory usage of a process, I found some questions. I'am using a system with 128 MB physical RAM, no disk, 2.6.27 kernel. Running top, I see 38 MB in use, 90 MB free, but a VSZ for my process of 158 MB. Looking at /proc/pid/maps, I see that the process uses 140 MB without shared libs. So how it is possible that the process can allocate more memory than there is, without posibility of swapping? Why said top that 90 MB free? Does the kernel serve the allocation only if ist really used? an allocation does not actually set aside any physical ram (unless you do an mlock and actually touch the memory) - it only reserves virtual memory areas (vma's) and when your process accesses the respective address the OS page-faults and prvides the physical RAM as needed (and possible) - if you have no swap and you allow overcommitting of memory then you are potentially going to see this application failing as soon as it tries to utilize the memory. This can be set in the system via /proc/sys/vm/overcommit_memory (see linux/Documentation/vm/overcommit-accounting for details). hofrat ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: Device node - How does kernel know about it
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 * Ramdisk is also executing fine, just that prints are not coming out of serial. I can see the execution of various user programs with a printk in sys_execve() routine. Ramdisk has all the required files like /dev/console, /dev/ttyS0, etc. * Looking further into tty driver, I noticed that call to tty_write() or do_tty_write() is not happening at all. So, somewhere the interface between kernel and user program is lost. * Just to check it out, I tried to write a small kernel module and a test program. - Attached memtest.c module (not really testing memory there. :-)) - Attached testmemtest.c user program, that just open's it and reads the information - Created a device node using mknod /dev/memtest c 168 0 - When I do insmod memtest.ko inside the ramdisk bootup scripts, I could see all the printk's on the console - When I execute testmemtest next in the same script, it does not display the printk inside of memtest.c module. This only indicates that read call did not really go to the kernel side. - Just to check my program's validity, I checked on a similar machine and all the code works fine. - uname -r also matches with what I built. So, chances of exiting from open call because of mismatch is remote. Since userland cannot print, I have no idea what exactly is happening there. The kernel will simply look at the major:minor numbers - so maybe you simply have a wrong major/minor for /dev/ttyS0 ? in that case you will see nothing but other than that most things will go on working. hofrat -BEGIN PGP SIGNATURE- Version: GnuPG v1.2.4 (GNU/Linux) iD8DBQFHdLY2nU7rXZKfY2oRApFpAKCKfGanKHGuFFJmUFy3aQtjmWNjEACfU7uK hrfpn2RMn5l23ZqCOXV5rd8= =GfsF -END PGP SIGNATURE- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: rtlinux rtai interrupt latency
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 On Sun, 2007-10-21 at 10:45 +0800, Bai Shuwei wrote: all, hi does anyone knows RTLinux, RTAI interrupt latency and schedule latency on the PPC440, PPC405? You'd have to ask Wind River. There are free variants around as well - XtratuM/PPC (RTLinux-4.0) is running on 440EP/GR and 405 Octobus, im quite sur RTAI also is available for AMCC CPUs - check RTAI.org (mailing list link is to be found there) and rtlinux-gpl.org (mailing list link) - those lists are most likely the best source for the free-software hard-realtime Linux variants. hofrat -BEGIN PGP SIGNATURE- Version: GnuPG v1.2.4 (GNU/Linux) iD4DBQFHGmZZnU7rXZKfY2oRArrjAJ9WXETFjALA52TRuRtDRSm4x3DcZACYgylW 60r4W0r6+LtMv+UBK+5h9A== =Eb2V -END PGP SIGNATURE- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: ppc manual paging question
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 I've got some qeustion about ppc(ppc44x) paging: how can I manually map a virtual address to a physical address through a specific pgd? How does ppc translate virt address to physical one? I think besides from tlb, the CPU will search the page table entries via the pgd, can I alter the pgd value to change the memory translation? under i386, it's very simple, we can just rewrite %%cr3, it even could invalidate all tlb entries automatically, how can I do this under ppc? I've tried rewrite current-mm-pgd and current-thread.pgdir, but sounds like it still not insufficiant, am I missing something vital? sur ! same thing flush the tlb or you will be totally inconsistant - actually the box should not have survived this treatment in a stable manner. You might want to look at kernel/ppc-stub.c as a good reference - its a similar problem gdb also writes into memory without the kernel knowing about it - that is comparable to what XM is doing - the solution flush the cache/tlb all over the place - now flushing the cache is evil - but I would do it for now and we can check alternatives and optimizations later - for now brute force is the way to go. for 4xx dont forget to isync after fidling with pgd/pte (see set_context in entry_4xx.S . As far as I understand 4xx all you would really need to do is clear the TLB entry and then you get an exception and that is handled via 0x1100/0x1200 D/I respectively (head_4xx.S) - admitedly Im not up to this task - need to give the manuals a lock my self to understand whats going on there ;) hofrat -BEGIN PGP SIGNATURE- Version: GnuPG v1.2.4 (GNU/Linux) iD8DBQFHHCmpnU7rXZKfY2oRAjLmAJ90QwCBHLaglOfJ5QAnJyCCIZDmGwCgh/fD E76Ki1FdfofUSuVBXL1tG0M= =/1C5 -END PGP SIGNATURE- ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev