The site-specific OOM messages are unnecessary, because they
duplicate the MM subsystem generic OOM message.
Signed-off-by: Jingoo Han <[email protected]>
---
drivers/regulator/bcm590xx-regulator.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/drivers/regulator/bcm590xx-regulator.c b/drivers/regulator/bcm590xx-regulator.c
index 57544e2..fb8c6f7 100644
--- a/drivers/regulator/bcm590xx-regulator.c
+++ b/drivers/regulator/bcm590xx-regulator.c
@@ -326,10 +326,8 @@ static struct bcm590xx_board *bcm590xx_parse_dt_reg_data(
}
data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
- if (!data) {
- dev_err(&pdev->dev, "failed to allocate regulator board data\n");
+ if (!data)
return NULL;
- }
np = of_node_get(np);
regulators = of_get_child_by_name(np, "regulators");
@@ -374,10 +372,8 @@ static int bcm590xx_probe(struct platform_device *pdev)
&bcm590xx_reg_matches);
pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
- if (!pmu) {
- dev_err(&pdev->dev, "Memory allocation failed for pmu\n");
+ if (!pmu)
return -ENOMEM;
- }
pmu->mfd = bcm590xx;
@@ -385,17 +381,13 @@ static int bcm590xx_probe(struct platform_device *pdev)
pmu->desc = devm_kzalloc(&pdev->dev, BCM590XX_NUM_REGS *
sizeof(struct regulator_desc), GFP_KERNEL);
- if (!pmu->desc) {
- dev_err(&pdev->dev, "Memory alloc fails for desc\n");
+ if (!pmu->desc)
return -ENOMEM;
- }
pmu->info = devm_kzalloc(&pdev->dev, BCM590XX_NUM_REGS *
sizeof(struct bcm590xx_info *), GFP_KERNEL);
- if (!pmu->info) {
- dev_err(&pdev->dev, "Memory alloc fails for info\n");
+ if (!pmu->info)
return -ENOMEM;
- }
info = bcm590xx_regs;
--
1.7.10.4
The site-specific OOM messages are unnecessary, because they
duplicate the MM subsystem generic OOM message.
Signed-off-by: Jingoo Han <[email protected]>
---
drivers/regulator/max8649.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/regulator/max8649.c b/drivers/regulator/max8649.c
index 3172da8..c8bddcc 100644
--- a/drivers/regulator/max8649.c
+++ b/drivers/regulator/max8649.c
@@ -161,10 +161,8 @@ static int max8649_regulator_probe(struct i2c_client *client,
info = devm_kzalloc(&client->dev, sizeof(struct max8649_regulator_info),
GFP_KERNEL);
- if (!info) {
- dev_err(&client->dev, "No enough memory\n");
+ if (!info)
return -ENOMEM;
- }
info->regmap = devm_regmap_init_i2c(client, &max8649_regmap_config);
if (IS_ERR(info->regmap)) {
--
1.7.10.4
The site-specific OOM messages are unnecessary, because they
duplicate the MM subsystem generic OOM message.
Signed-off-by: Jingoo Han <[email protected]>
---
drivers/regulator/pbias-regulator.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/regulator/pbias-regulator.c b/drivers/regulator/pbias-regulator.c
index 708ddbb..6d02d68 100644
--- a/drivers/regulator/pbias-regulator.c
+++ b/drivers/regulator/pbias-regulator.c
@@ -122,10 +122,8 @@ static int pbias_regulator_probe(struct platform_device *pdev)
drvdata = devm_kzalloc(&pdev->dev, sizeof(struct pbias_regulator_data)
* count, GFP_KERNEL);
- if (drvdata == NULL) {
- dev_err(&pdev->dev, "Failed to allocate device data\n");
+ if (!drvdata)
return -ENOMEM;
- }
syscon = syscon_regmap_lookup_by_phandle(np, "syscon");
if (IS_ERR(syscon))
--
1.7.10.4
On Sun, Jun 1, 2014 at 11:27 PM, Jingoo Han <[email protected]> wrote:
> The site-specific OOM messages are unnecessary, because they
> duplicate the MM subsystem generic OOM message.
>
> Signed-off-by: Jingoo Han <[email protected]>
> ---
> drivers/regulator/bcm590xx-regulator.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/regulator/bcm590xx-regulator.c b/drivers/regulator/bcm590xx-regulator.c
> index 57544e2..fb8c6f7 100644
> --- a/drivers/regulator/bcm590xx-regulator.c
> +++ b/drivers/regulator/bcm590xx-regulator.c
> @@ -326,10 +326,8 @@ static struct bcm590xx_board *bcm590xx_parse_dt_reg_data(
> }
>
> data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> - if (!data) {
> - dev_err(&pdev->dev, "failed to allocate regulator board data\n");
> + if (!data)
> return NULL;
> - }
>
> np = of_node_get(np);
> regulators = of_get_child_by_name(np, "regulators");
> @@ -374,10 +372,8 @@ static int bcm590xx_probe(struct platform_device *pdev)
> &bcm590xx_reg_matches);
>
> pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
> - if (!pmu) {
> - dev_err(&pdev->dev, "Memory allocation failed for pmu\n");
> + if (!pmu)
> return -ENOMEM;
> - }
>
> pmu->mfd = bcm590xx;
>
> @@ -385,17 +381,13 @@ static int bcm590xx_probe(struct platform_device *pdev)
>
> pmu->desc = devm_kzalloc(&pdev->dev, BCM590XX_NUM_REGS *
> sizeof(struct regulator_desc), GFP_KERNEL);
> - if (!pmu->desc) {
> - dev_err(&pdev->dev, "Memory alloc fails for desc\n");
> + if (!pmu->desc)
> return -ENOMEM;
> - }
>
> pmu->info = devm_kzalloc(&pdev->dev, BCM590XX_NUM_REGS *
> sizeof(struct bcm590xx_info *), GFP_KERNEL);
> - if (!pmu->info) {
> - dev_err(&pdev->dev, "Memory alloc fails for info\n");
> + if (!pmu->info)
> return -ENOMEM;
> - }
>
> info = bcm590xx_regs;
>
For the other two drivers touched by this patch series, the probe
methods only include a single dynamic memory allocation. As such, the
stack trace provided by the generic memory code is sufficient to
quickly identify where the failure occurred.
The probe method of this driver, on the other hand, performs several
allocations and the error messages you intend to remove conveniently
pinpoint which one failed. While the offsets in the trace could be
used to derive the same information, I am skeptical that is enough to
justify removing the messages.
Thanks,
Tim Kryger
On Mon, Jun 02, 2014 at 12:12:20AM -0700, Tim Kryger wrote:
> The probe method of this driver, on the other hand, performs several
> allocations and the error messages you intend to remove conveniently
> pinpoint which one failed. While the offsets in the trace could be
> used to derive the same information, I am skeptical that is enough to
> justify removing the messages.
On the other hand how likely is anyone to care which particular
allocation triggered the OOM?
On Mon, Jun 02, 2014 at 03:30:44PM +0900, Jingoo Han wrote:
> The site-specific OOM messages are unnecessary, because they
> duplicate the MM subsystem generic OOM message.
Applied, thanks.
On Mon, Jun 02, 2014 at 03:29:57PM +0900, Jingoo Han wrote:
> The site-specific OOM messages are unnecessary, because they
> duplicate the MM subsystem generic OOM message.
Applied, thanks.
On Mon, Jun 2, 2014 at 5:50 AM, Mark Brown <[email protected]> wrote:
> On Mon, Jun 02, 2014 at 12:12:20AM -0700, Tim Kryger wrote:
>
>> The probe method of this driver, on the other hand, performs several
>> allocations and the error messages you intend to remove conveniently
>> pinpoint which one failed. While the offsets in the trace could be
>> used to derive the same information, I am skeptical that is enough to
>> justify removing the messages.
>
> On the other hand how likely is anyone to care which particular
> allocation triggered the OOM?
I suppose you would only care if a failure was due to something unique
about a specific allocation. For example, if someone made a foolish
change to a structure that resulted in a request for substantially
more memory, the messages would lead directly to the problem.
The real question is whether this change improves the driver. To me,
it seems like a draw.
On Mon, Jun 02, 2014 at 11:41:01PM -0700, Tim Kryger wrote:
> The real question is whether this change improves the driver. To me,
> it seems like a draw.
It's not really for the benefit of the driver, it's for the benefit of
the kernel as a whole - the individually hand crafted error messages all
take up space in the kernel so if we just remove them and rely on the
OOM messages being loud we will in aggregate save some not completely
trivial amount of space in the kernel image. Not a *massive* amount but
somewhat helpful.
On Mon, Jun 02, 2014 at 03:27:21PM +0900, Jingoo Han wrote:
> The site-specific OOM messages are unnecessary, because they
> duplicate the MM subsystem generic OOM message.
>
> Signed-off-by: Jingoo Han <[email protected]>
> ---
> drivers/regulator/bcm590xx-regulator.c | 16 ++++------------
> 1 file changed, 4 insertions(+), 12 deletions(-)
Looks good, thanks.
Acked-by: Matt Porter <[email protected]>
> diff --git a/drivers/regulator/bcm590xx-regulator.c b/drivers/regulator/bcm590xx-regulator.c
> index 57544e2..fb8c6f7 100644
> --- a/drivers/regulator/bcm590xx-regulator.c
> +++ b/drivers/regulator/bcm590xx-regulator.c
> @@ -326,10 +326,8 @@ static struct bcm590xx_board *bcm590xx_parse_dt_reg_data(
> }
>
> data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> - if (!data) {
> - dev_err(&pdev->dev, "failed to allocate regulator board data\n");
> + if (!data)
> return NULL;
> - }
>
> np = of_node_get(np);
> regulators = of_get_child_by_name(np, "regulators");
> @@ -374,10 +372,8 @@ static int bcm590xx_probe(struct platform_device *pdev)
> &bcm590xx_reg_matches);
>
> pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
> - if (!pmu) {
> - dev_err(&pdev->dev, "Memory allocation failed for pmu\n");
> + if (!pmu)
> return -ENOMEM;
> - }
>
> pmu->mfd = bcm590xx;
>
> @@ -385,17 +381,13 @@ static int bcm590xx_probe(struct platform_device *pdev)
>
> pmu->desc = devm_kzalloc(&pdev->dev, BCM590XX_NUM_REGS *
> sizeof(struct regulator_desc), GFP_KERNEL);
> - if (!pmu->desc) {
> - dev_err(&pdev->dev, "Memory alloc fails for desc\n");
> + if (!pmu->desc)
> return -ENOMEM;
> - }
>
> pmu->info = devm_kzalloc(&pdev->dev, BCM590XX_NUM_REGS *
> sizeof(struct bcm590xx_info *), GFP_KERNEL);
> - if (!pmu->info) {
> - dev_err(&pdev->dev, "Memory alloc fails for info\n");
> + if (!pmu->info)
> return -ENOMEM;
> - }
>
> info = bcm590xx_regs;
>
> --
> 1.7.10.4
>
>
On Mon, Jun 02, 2014 at 03:27:21PM +0900, Jingoo Han wrote:
> The site-specific OOM messages are unnecessary, because they
> duplicate the MM subsystem generic OOM message.
Applied, thanks.