| Message ID | c48a01db-c477-3bd3-5cb4-548e47810ccb@users.sourceforge.net (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 11/26/2017 12:55 PM, SF Markus Elfring wrote: > From: Markus Elfring <elfring@users.sourceforge.net> > 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 <elfring@users.sourceforge.net> > --- nak, unlike many others, these message give extra info on which allocation failed, that can be useful. > 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: > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Omit an extra message for a memory allocation failure in these functions. … > nak, unlike many others, these message give extra info on which > allocation failed, that can be useful. Can a default allocation failure report provide the information which you might expect so far? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 27, 2017 at 06:27:08PM +0100, SF Markus Elfring wrote: > >> Omit an extra message for a memory allocation failure in these functions. > … > > nak, unlike many others, these message give extra info on which > > allocation failed, that can be useful. > > Can a default allocation failure report provide the information > which you might expect so far? You should be able to answer that question yourself. And if you are unable to do so, just do not sent changes pointed by any code analysis tools. As a side note, if you look at whole call chain, you'll find quite some room for optimizations - look how dev and pdev are used. That also applies to other patches. Best regards, ladis -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> Can a default allocation failure report provide the information >> which you might expect so far? > > You should be able to answer that question yourself. I can not answer this detail completely myself because my knowledge is incomplete about your concrete expectations for the exception handling which can lead to different views for the need of additional error messages. > And if you are unable to do so, just do not sent changes pointed > by any code analysis tools. They can point aspects out for further software development considerations, can't they? > As a side note, if you look at whole call chain, you'll find quite some > room for optimizations - look how dev and pdev are used. That also applies > to other patches. Would you prefer to improve the source code in other ways? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Markus, On Mon, Nov 27, 2017 at 7:12 PM, SF Markus Elfring <elfring@users.sourceforge.net> wrote: >>> Can a default allocation failure report provide the information >>> which you might expect so far? >> >> You should be able to answer that question yourself. > > I can not answer this detail completely myself because my knowledge > is incomplete about your concrete expectations for the exception handling > which can lead to different views for the need of additional error messages. It may be a good idea to try to trigger an out-of-memory condition yourself, and see what happens? >> And if you are unable to do so, just do not sent changes pointed >> by any code analysis tools. > > They can point aspects out for further software development considerations, > can't they? Sure. But I think it is a good experience to witness what can happen if you "violate" these "coding standards written by other people", and learn to understand why they were written, increasing your own knowledge. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <elfring@users.sourceforge.net> > > 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 <elfring@users.sourceforge.net> > > --- > > nak, unlike many others, these message give extra info on which > allocation failed, that can be useful. <shrug> Not really. There are tradeoffs. There is the generic stack dump on OOM so the module/line is already known. 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. 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: > > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 <elfring@users.sourceforge.net> >>> 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 <elfring@users.sourceforge.net> >>> --- >> >> nak, unlike many others, these message give extra info on which >> allocation failed, that can be useful. > > <shrug> 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: >>> -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, Nov 27, 2017 at 03:33:13PM -0600, Andrew F. Davis wrote: > 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 <elfring@users.sourceforge.net> > >>> 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 <elfring@users.sourceforge.net> > >>> --- > >> > >> nak, unlike many others, these message give extra info on which > >> allocation failed, that can be useful. > > > > <shrug> 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. All those messages are result of allocation failure. The memory allocated is later used to hold duplicate of static const data. Do we need that copy (and thus allocation) at all? ladis -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> There is the generic stack dump on OOM so the module/line > is already known. Can such an implementation detail become better documented than in C source code? > The existence of these messages increases code size which > also make the OOM condition slightly more likely. Interesting view … > Markus' commit messages are always really poor descriptions > of why these removals are somewhat useful and the commit > could/should/might be applied. I agree that they could be improved for this transformation pattern if other information sources would become clearer for corresponding references. It seems that I got no responses so far for clarification requests according to the documentation in a direction I hoped for. Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2017-11-27 at 22:48 +0100, SF Markus Elfring wrote: > It seems that I got no responses so far for clarification requests > according to the documentation in a direction I hoped for. That's because you are pretty unresponsive to direction. You've gotten _many_ replies to your patches to which you have seemingly decided to ignore. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>> It seems that I got no responses so far for clarification requests >> according to the documentation in a direction I hoped for. > > That's because you are pretty unresponsive to direction. From which places did you get this impression? > You've gotten _many_ replies to your patches I got the usual mixture of disagreements and acceptance for my selection of change possibilities. Some constructive feedback was also provided which might need further software development considerations. Is the situation improvable here? > to which you have seemingly decided to ignore. You might eventually notice that I react to special information occasionally with a big delay. For which concrete details are you still waiting for a better response from me? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 28 Nov 2017, SF Markus Elfring wrote: > >> It seems that I got no responses so far for clarification requests > >> according to the documentation in a direction I hoped for. > > > > That's because you are pretty unresponsive to direction. > > From which places did you get this impression? Perhaps from the text that you have written only four lines below. All comments are dismissed as "the usual mixture of disagreements and acceptance". If you look at the patches sent by others, who learn from the feedback provided to them, there are not so many responses on the disagreements side. So the mixture is not usual. Since you send lots of patches on the same issues, there should be no disagreements at all at this point. julia > > > You've gotten _many_ replies to your patches > > I got the usual mixture of disagreements and acceptance for > my selection of change possibilities. > Some constructive feedback was also provided which might need > further software development considerations. > Is the situation improvable here? > > > > to which you have seemingly decided to ignore. > > You might eventually notice that I react to special information > occasionally with a big delay. > > For which concrete details are you still waiting for a better > response from me? > > Regards, > Markus > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2017-11-28 at 08:41 +0100, SF Markus Elfring wrote: > > > It seems that I got no responses so far for clarification requests > > > according to the documentation in a direction I hoped for. > > > > That's because you are pretty unresponsive to direction. > > From which places did you get this impression? How many times have I told you to include the reason for your patches in your proposed commit message? Too often. For instance, specific to this patch: Many people do not know that a generic kmalloc does a dump_stack() on OOM. That information should be part of the commit message. Also removing the printk code reduces overall code size. The actual size reduction should be described in the commit message too. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>>> It seems that I got no responses so far for clarification requests >>>> according to the documentation in a direction I hoped for. >>> >>> That's because you are pretty unresponsive to direction. >> >> From which places did you get this impression? > > Perhaps from the text that you have written only four lines below. > All comments are dismissed as "the usual mixture of disagreements and acceptance". A mixture will always evolve. * Some acceptance might not need further considerations. * But the disagreements are remembered differently. They have got a potential for further improvements in some areas. > If you look at the patches sent by others, who learn from > the feedback provided to them, I am also learning to some degree continuously. > there are not so many responses on the disagreements side. How do you think about to look at the details for such an observation? > So the mixture is not usual. I find that it can be also a matter of statistics. > Since you send lots of patches on the same issues, Yes. - I am trying to fix some implementation details by the means of source code analysis and corresponding transformation. The patch count is still growing. > there should be no disagreements at all at this point. I got an other impression. The probability for disagreements is increasing in relation to the number of contributors to which I show change possibilities. There are also other open issues remaining which can get another solution somehow. Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Nov 28, 2017 at 12:04:04AM -0800, Joe Perches wrote: > On Tue, 2017-11-28 at 08:41 +0100, SF Markus Elfring wrote: > > > > It seems that I got no responses so far for clarification requests > > > > according to the documentation in a direction I hoped for. > > > > > > That's because you are pretty unresponsive to direction. > > > > From which places did you get this impression? > > How many times have I told you to include the reason for > your patches in > your proposed commit message? Too often. > > For instance, specific to this patch: > > Many people do not know that a generic kmalloc does a > dump_stack() on OOM. That information should be part > of the commit message. > > Also removing the printk code reduces overall code size. > The actual size reduction should be described in the > commit message too. Could we, please, return one step back and reevaluate need for kmalloc. That would eliminate original "problem" as well. ladis -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> How many times have I told you to include the reason for > your patches in your proposed commit message? It might be useful to look again. > Too often. I answered this feedback to some degree. > Many people do not know that a generic kmalloc does a > dump_stack() on OOM. This is another interesting information, isn't it? It is expected that the function “devm_kzalloc” has got a similar property. > That information should be part of the commit message. How do you think about to provide it also in any reference documentation in a clearer way? I would be more confident then to copy it into my messages. Do you want that I take your current answer as an official note for my work (instead of waiting for adjustments of other areas)? > Also removing the printk code reduces overall code size. Such an effect can be nice if the involved contributors can agree on the deletion of questionable error messages at all. > The actual size reduction should be described in the > commit message too. For which hardware and software combinations would you like to see facts there? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 28 Nov 2017, SF Markus Elfring wrote: > >>>> It seems that I got no responses so far for clarification requests > >>>> according to the documentation in a direction I hoped for. > >>> > >>> That's because you are pretty unresponsive to direction. > >> > >> From which places did you get this impression? > > > > Perhaps from the text that you have written only four lines below. > > All comments are dismissed as "the usual mixture of disagreements and acceptance". > > A mixture will always evolve. > > * Some acceptance might not need further considerations. > > * But the disagreements are remembered differently. > They have got a potential for further improvements in some areas. > > > > If you look at the patches sent by others, who learn from > > the feedback provided to them, > > I am also learning to some degree continuously. > > > > there are not so many responses on the disagreements side. > > How do you think about to look at the details for such an observation? > > > > So the mixture is not usual. > > I find that it can be also a matter of statistics. > > > > Since you send lots of patches on the same issues, > > Yes. - I am trying to fix some implementation details by the means > of source code analysis and corresponding transformation. > The patch count is still growing. > > > > there should be no disagreements at all at this point. > > I got an other impression. The probability for disagreements is increasing > in relation to the number of contributors to which I show change possibilities. No. You should learn from the previous submissions what concerns people have and address them up front. julia > > There are also other open issues remaining which can get another > solution somehow. > > Regards, > Markus > -- > To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 28 Nov 2017, SF Markus Elfring wrote: > > How many times have I told you to include the reason for > > your patches in your proposed commit message? > > It might be useful to look again. > > > > Too often. > > I answered this feedback to some degree. > > > > Many people do not know that a generic kmalloc does a > > dump_stack() on OOM. > > This is another interesting information, isn't it? > > It is expected that the function “devm_kzalloc” has got a similar property. You don't have to expect this. Go look at the definition of devm_kzalloc and see whether it has the property or not. > For which hardware and software combinations would you like to see > facts there? This is not for Joe to decide, it's for the person who receives the patch to decide. You could start with the ones for which the code actually compiles, using the standard make file and no special options, and a recent version of gcc. julia
>> I got an other impression. The probability for disagreements is increasing >> in relation to the number of contributors to which I show change possibilities. > > No. You should learn from the previous submissions what concerns people > have and address them up front. The concerns can vary as contributors and changes are different. How would you like to “address” the data structure for a default allocation failure report finally? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
>>> Many people do not know that a generic kmalloc does a >>> dump_stack() on OOM. >> >> This is another interesting information, isn't it? >> >> It is expected that the function “devm_kzalloc” has got a similar property. > > > You don't have to expect this. Go look at the definition of devm_kzalloc > and see whether it has the property or not. I find that the corresponding documentation of these programming interfaces is incomplete for a desired format which could be different than C source code. https://elixir.free-electrons.com/linux/v4.15-rc1/source/include/linux/device.h#L657 https://elixir.free-electrons.com/linux/v4.15-rc1/source/drivers/base/devres.c#L763 https://www.kernel.org/doc/html/latest/driver-api/basics.html#c.devm_kmalloc Can the Coccinelle software help more to determine desired function properties? >> For which hardware and software combinations would you like to see >> facts there? > > This is not for Joe to decide, This view is fine in principle. > it's for the person who receives the patch to decide. I am curious on further comments from these contributors. > You could start with the ones for which the code actually compiles, > using the standard make file and no special options, and a > recent version of gcc. The variation space could become too big to handle for me (alone). How will this aspect evolve further? Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> How many times have I told you to include the reason for your patches > in your proposed commit message? Will it be useful to look again at the involved circumstances? > Too often. Did I answer any concerns partly? > Many people do not know that a generic kmalloc does a dump_stack() on OOM. Do you see a need to represent such information better? Is it expected that the function “devm_kzalloc” has got a similar property? > That information should be part of the commit message. How do you think about to share it also from any reference documentation in a clearer way? Do we stumble on a target conflict in this case? I am generally trying to improve the software situation to some degree. I prefer then to work with safe information sources. Unfortunately, I might have not reached a desired confidence level here for a more detailed commit message. I assume that software development efforts could increase in significant ways if something should be improved further in a direction I hope. But this could mean that time frames will grow for corresponding clarifications. * Does such a situation block progress on the deletion of other remaining questionable error messages? * Would you like to increase the software development attention anyhow? By the way: It seems that my update suggestion for the directory “omapfb/dss” could be superseded by the patch “omapfb: dss: Do not duplicate features data” from Ladislav Michl. https://patchwork.kernel.org/patch/10082027/ https://lkml.kernel.org/r/<20171129123308.GA26578@lenoch> Regards, Markus -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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: