From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Andrew F. Davis" Date: Mon, 27 Nov 2017 21:33:13 +0000 Subject: Re: [PATCH] omapfb/dss: Delete an error message for a failed memory allocation in three functions Message-Id: <15209a13-c53b-068b-c0b2-f073117d39e2@ti.com> List-Id: References: <1511809633.32426.70.camel@perches.com> In-Reply-To: <1511809633.32426.70.camel@perches.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Joe Perches , SF Markus Elfring , linux-omap@vger.kernel.org, linux-fbdev@vger.kernel.org, dri-devel@lists.freedesktop.org, Arvind Yadav , Bartlomiej Zolnierkiewicz , Tomi Valkeinen Cc: LKML , kernel-janitors@vger.kernel.org On 11/27/2017 01:07 PM, Joe Perches wrote: > On Mon, 2017-11-27 at 10:43 -0600, Andrew F. Davis wrote: >> On 11/26/2017 12:55 PM, SF Markus Elfring wrote: >>> From: Markus Elfring >>> Date: Sun, 26 Nov 2017 19:46:09 +0100 >>> >>> Omit an extra message for a memory allocation failure in these functions. >>> >>> This issue was detected by using the Coccinelle software. >>> >>> Signed-off-by: Markus Elfring >>> --- >> >> nak, unlike many others, these message give extra info on which >> allocation failed, that can be useful. > > Not really. There are tradeoffs. > > There is the generic stack dump on OOM so the module/line > is already known. > If that is the case then I have no strong feelings either way. > The existence of these messages increases code size which > also make the OOM condition slightly more likely. > > These are generally used only at initialization and those > if you are OOM at initialization, bad things happen anyway > so where the specific OOM occurred doesn't really matter. > True, these messages will probably only ever get displayed if someone is messing with the allocated structs and accidentally balloons their size, so these are more debug statements than anything. > Markus' commit messages are always really poor descriptions > of why these removals are somewhat useful and the commit > could/should/might be applied. > > Your choice. > >>> drivers/video/fbdev/omap2/omapfb/dss/dispc.c | 4 +--- >>> drivers/video/fbdev/omap2/omapfb/dss/dss.c | 4 +--- >>> drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c | 4 +--- >>> 3 files changed, 3 insertions(+), 9 deletions(-) >>> >>> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c >>> index 7a75dfda9845..10164a3bae4a 100644 >>> --- a/drivers/video/fbdev/omap2/omapfb/dss/dispc.c >>> +++ b/drivers/video/fbdev/omap2/omapfb/dss/dispc.c >>> @@ -3982,10 +3982,8 @@ static int dispc_init_features(struct platform_device *pdev) >>> struct dispc_features *dst; >>> >>> dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL); >>> - if (!dst) { >>> - dev_err(&pdev->dev, "Failed to allocate DISPC Features\n"); >>> + if (!dst) >>> return -ENOMEM; >>> - } >>> >>> switch (omapdss_get_version()) { >>> case OMAPDSS_VER_OMAP24xx: >>> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/dss.c b/drivers/video/fbdev/omap2/omapfb/dss/dss.c >>> index 48c6500c24e1..a5de13777e2b 100644 >>> --- a/drivers/video/fbdev/omap2/omapfb/dss/dss.c >>> +++ b/drivers/video/fbdev/omap2/omapfb/dss/dss.c >>> @@ -893,10 +893,8 @@ static int dss_init_features(struct platform_device *pdev) >>> struct dss_features *dst; >>> >>> dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL); >>> - if (!dst) { >>> - dev_err(&pdev->dev, "Failed to allocate local DSS Features\n"); >>> + if (!dst) >>> return -ENOMEM; >>> - } >>> >>> switch (omapdss_get_version()) { >>> case OMAPDSS_VER_OMAP24xx: >>> diff --git a/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c b/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c >>> index 9a13c35fd6d8..d25eea10c665 100644 >>> --- a/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c >>> +++ b/drivers/video/fbdev/omap2/omapfb/dss/hdmi_phy.c >>> @@ -195,10 +195,8 @@ static int hdmi_phy_init_features(struct platform_device *pdev) >>> const struct hdmi_phy_features *src; >>> >>> dst = devm_kzalloc(&pdev->dev, sizeof(*dst), GFP_KERNEL); >>> - if (!dst) { >>> - dev_err(&pdev->dev, "Failed to allocate HDMI PHY Features\n"); >>> + if (!dst) >>> return -ENOMEM; >>> - } >>> >>> switch (omapdss_get_version()) { >>> case OMAPDSS_VER_OMAP4430_ES1: >>>