* [PATCH] staging: lustre: osc: Fix sparse warning about osc_init @ 2015-02-02 13:36 Andreas Ruprecht 2015-02-02 14:16 ` Al Viro 0 siblings, 1 reply; 19+ messages in thread From: Andreas Ruprecht @ 2015-02-02 13:36 UTC (permalink / raw) To: Oleg Drokin Cc: Andreas Dilger, Greg Kroah-Hartman, HPDD-discuss, devel, linux-kernel, Andreas Ruprecht When running sparse on the osc/ subdirectory, it shows the following warning: andreas@workbox:~/linux-next$ make C=1 M=drivers/staging/lustre/lustre/osc/ [...] drivers/staging/lustre/lustre/osc/osc_request.c:3335:12: warning: symbol 'osc_init' was not declared. Should it be static? [...] As this is the module init function, it can (and should) be declared static. Signed-off-by: Andreas Ruprecht <rupran@einserver.de> --- drivers/staging/lustre/lustre/osc/osc_request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c index b9450b9..0adfa70 100644 --- a/drivers/staging/lustre/lustre/osc/osc_request.c +++ b/drivers/staging/lustre/lustre/osc/osc_request.c @@ -3332,7 +3332,7 @@ extern struct lu_kmem_descr osc_caches[]; extern spinlock_t osc_ast_guard; extern struct lock_class_key osc_ast_guard_class; -int __init osc_init(void) +static int __init osc_init(void) { struct lprocfs_static_vars lvars = { NULL }; int rc; -- 1.9.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH] staging: lustre: osc: Fix sparse warning about osc_init 2015-02-02 13:36 [PATCH] staging: lustre: osc: Fix sparse warning about osc_init Andreas Ruprecht @ 2015-02-02 14:16 ` Al Viro 2015-02-02 19:13 ` Andreas Ruprecht 2015-02-02 19:24 ` [PATCH] staging: lustre: osc: Make osc_init() static Andreas Ruprecht 0 siblings, 2 replies; 19+ messages in thread From: Al Viro @ 2015-02-02 14:16 UTC (permalink / raw) To: Andreas Ruprecht Cc: Oleg Drokin, Andreas Dilger, Greg Kroah-Hartman, HPDD-discuss, devel, linux-kernel On Mon, Feb 02, 2015 at 02:36:43PM +0100, Andreas Ruprecht wrote: > When running sparse on the osc/ subdirectory, it shows the > following warning: > > andreas@workbox:~/linux-next$ make C=1 M=drivers/staging/lustre/lustre/osc/ > [...] > drivers/staging/lustre/lustre/osc/osc_request.c:3335:12: warning: > symbol 'osc_init' was not declared. Should it be static? > [...] > > As this is the module init function, it can (and should) be declared > static. > > Signed-off-by: Andreas Ruprecht <rupran@einserver.de> For pity sake, folks, could you at least try to produce less meaningless commit summaries? "Fix sparse warning" says nothing whatsoever about what's getting done. The role of sparse in that is simple - it has provided a tip to look at that declaration. That's all; it might as well had been offered by a guy puking at the next stall in the loo after big party. And no, sparse alone is not sufficient to prove that it could be made static - it might be used somewhere else with explicit extern, its declaration might've been in a header that didn't get included here, or under slightly incorrect ifdefs, etc. Such things need to be investigated manually, not that it was hard to do. OK, in this case the tip had panned out; it can be made static (quick grep shows that) and it's better off that way - less namespace pollution, makes life easier when doing analysis of that code, etc. _That_ is the point of this patch, not making the holy oracle shut the bleeding fuck up. If you want to credit sparse (or that guy puking in the next stall, or whatever had drawn your attention to the function in question) - great, do so inside the commit message. _AFTER_ having described what the patch does ("make osc_init() static" or equivalent thereof), please. Ideally - with something like "it's never used outside of osc_request.c" after summary line (strictly speaking, something that serves as module_init might very well be called elsewhere in the module explicitly; not common, but possible). ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] staging: lustre: osc: Fix sparse warning about osc_init 2015-02-02 14:16 ` Al Viro @ 2015-02-02 19:13 ` Andreas Ruprecht 2015-02-05 16:30 ` use of opaque subject lines Al Viro 2015-02-02 19:24 ` [PATCH] staging: lustre: osc: Make osc_init() static Andreas Ruprecht 1 sibling, 1 reply; 19+ messages in thread From: Andreas Ruprecht @ 2015-02-02 19:13 UTC (permalink / raw) To: Al Viro Cc: Oleg Drokin, Andreas Dilger, Greg Kroah-Hartman, HPDD-discuss, devel, linux-kernel On 02.02.2015 15:16, Al Viro wrote: > On Mon, Feb 02, 2015 at 02:36:43PM +0100, Andreas Ruprecht wrote: >> When running sparse on the osc/ subdirectory, it shows the >> following warning: >> >> andreas@workbox:~/linux-next$ make C=1 M=drivers/staging/lustre/lustre/osc/ >> [...] >> drivers/staging/lustre/lustre/osc/osc_request.c:3335:12: warning: >> symbol 'osc_init' was not declared. Should it be static? >> [...] >> >> As this is the module init function, it can (and should) be declared >> static. >> >> Signed-off-by: Andreas Ruprecht <rupran@einserver.de> > > For pity sake, folks, could you at least try to produce less meaningless > commit summaries? "Fix sparse warning" says nothing whatsoever about > what's getting done. The role of sparse in that is simple - it has > provided a tip to look at that declaration. That's all; it might as well > had been offered by a guy puking at the next stall in the loo after big > party. And no, sparse alone is not sufficient to prove that it could be > made static - it might be used somewhere else with explicit extern, its > declaration might've been in a header that didn't get included here, or > under slightly incorrect ifdefs, etc. Such things need to be investigated > manually, not that it was hard to do. OK, in this case the tip had panned > out; it can be made static (quick grep shows that) and it's better off > that way - less namespace pollution, makes life easier when doing analysis > of that code, etc. > > _That_ is the point of this patch, not making the holy oracle shut the bleeding > fuck up. If you want to credit sparse (or that guy puking in the next stall, > or whatever had drawn your attention to the function in question) - great, > do so inside the commit message. _AFTER_ having described what the patch > does ("make osc_init() static" or equivalent thereof), please. Ideally - > with something like "it's never used outside of osc_request.c" after summary > line (strictly speaking, something that serves as module_init might very well > be called elsewhere in the module explicitly; not common, but possible). > ... and a very nice day to you, too, sir! On a serious note: I do understand what you're getting at, I don't take that personally (and I will send a v2 addressing the things above), but honestly, this kind of answer might just be a real turn-off for other people trying to get into kernel development... I don't want to start a whole new 'attitude in the kernel community' discussion, but I can't just let this go like that, sorry. Regards, Andreas ^ permalink raw reply [flat|nested] 19+ messages in thread
* use of opaque subject lines 2015-02-02 19:13 ` Andreas Ruprecht @ 2015-02-05 16:30 ` Al Viro 2015-02-05 16:57 ` Lad, Prabhakar 2015-02-05 17:08 ` Joe Perches 0 siblings, 2 replies; 19+ messages in thread From: Al Viro @ 2015-02-05 16:30 UTC (permalink / raw) To: Andreas Ruprecht Cc: Oleg Drokin, Andreas Dilger, Greg Kroah-Hartman, HPDD-discuss, devel, linux-kernel, Lad, Prabhakar On Mon, Feb 02, 2015 at 08:13:10PM +0100, Andreas Ruprecht wrote: > On a serious note: I do understand what you're getting at, I don't take > that personally (and I will send a v2 addressing the things above), but > honestly, this kind of answer might just be a real turn-off for other > people trying to get into kernel development... > > I don't want to start a whole new 'attitude in the kernel community' > discussion, but I can't just let this go like that, sorry. Just during the last 12 hours or so, I've seen the following l-k traffic: Subject: [PATCH] usb: host/sl811-hcd: fix sparse warning Subject: [PATCH] usb: gadget: function/f_sourcesink: fix sparse warning Subject: [PATCH] tty: vt/vt: fix sparse warning Subject: [PATCH] scsi: fix sparse warnings Subject: [PATCH] bfa: bfa_core: fix sparse warning Subject: [PATCH] scsi: fix sparse warning Subject: [PATCH] xen/acpi-processor: fix sparse warning Subject: [PATCH] scsi: initio: fix sparse warnings Subject: [PATCH] scsi: dc395x: fix sparse warning Subject: [PATCH] scsi: eata: fix sparse warning Subject: [PATCH] scsi: qla1280: fix sparse warnings Subject: [PATCH] scsi: ips: fix sparse warnings Subject: [PATCH] fbdev: via/via_clock: fix sparse warning Subject: [PATCH] usb: gadget: fix sparse warnings Subject: [PATCH] usb: gadget: fix sparse warnings Subject: [PATCH] usb: gadget: function/uvc_v4l2.c: fix sparse warnings Subject: [PATCH] xen-netback: fix sparse warning Subject: [PATCH] thermal: int340x: fix sparse warning Subject: [PATCH] vxge: fix sparse warning Subject: Re: [PATCH] xen-netback: fix sparse warning Subject: [PATCH] ixgbe: fix sparse warnings Subject: [PATCH] samsung-laptop: fix sparse warning Subject: [PATCH] x86: thinkpad_acpi.c: fix sparse warning Subject: [PATCH] Sony-laptop: fix sparse warning one of those is an ACK from maintainer, the rest are (AFAICS) all "make something static", all with rather poor commit messages and subjects. Not threaded, either (that would've somewhat mitigated one of the problems). Bad attitude? I'd say that _this_ is one. l-k is high-traffic list; postings like that mean extra strain on those who are trying at least to skim through it and they are not easy on those who are trying to read the commit logs. And it's trivial to mitigate - choose less opaque subjects, for starters. Because these are about as opaque as it gets: essentially, it's "sparse had drawn my attention to $FILE; I've fixed that problem by making it STFU". Readers would like more information to filter upon? Tough luck, open the damn mail and look into it. One after another, after another, to the tune of dozens per night. All with the commit messages pretty much of the form "$TOOL has produced $MESSAGE; with diff below it doesn't". It's physically impossible to read through all l-k traffic; the rate is too high for that. Is making it possible to do minimal triage too much to ask for? I'm not blaming anyone for going after the low-hanging fruit like that to get some experience on simple stuff, but as it is, it reinforces seriously bad habits while creating considerable PITA for list readers. And yes, we'd all done our share of lousy commit messages, but this pretty much teaches newbies to do that all the time. Sigh... Folks, please * Use descriptive subject lines at least somewhere in a thread. * Describe what the thing does, besides "makes $TOOL to STFU". * Remember, you don't fix warnings - you deal with the problem (if any) in the code those warnings had pointed to. And if there isn't a problem (i.e. the warning was a false positive) you might be annotating the code to tell the dumb tool what's going on there. Which is a different genre, and needs different mindset when reviewing (it's easy to obfuscate a _real_ problem away - away from the $TOOL's ability to spot it, that is). * Remember that said tools are nowhere near strong AI; the same warning can come from very different underlying problems. These warnigns are heuristics - such-and-such looking code has high chances of needing attention. Give at least some indication what the problem _is_ - e.g. the sparse warnings that has spawned this flood might come from 1) function really is used only in the file it's defined in, never intended to be used elsewhere, ought to be static to avoid namespace pollution 2) function is used elsewhere, and its uses elsewhere have explicit externs right at the point of use. Dangerous, since there's no practical way for compiler (gcc, sparse, lint, whatever) to verify that there's no type mismatch. Ought to have extern placed in some header included both by the file where it's defined and by all files where it's used, with externs at the point of use removed. Need to be careful about the choice of header and location in it. 3) function _is_ declared in a common header, but it's either not included by the file where it actually lives, *or* is included only on some architectures and/or configs. Ought to figure out which one it is, and either add an include or (needs _much_ more care) make some of the includes in the chain of indirect includes unconditional. 4) function is declared in a common header, and it is included, but declaration is under the too tight set of ifdefs. Might need to move it around, but that also requires some care - less than what you need when messing with conditional indirect includes, but also a non-trivial amount. Note that "I've made it static and result builds, so it must be (1)" is incorrect - the code elsewhere actually using it might have been ifdefed out on your config and architecture, simply not included into your build, or optimized away (again, on your config and architecture). "This identifier is never mentioned elsewhere" is better (albeit still not fool-proof - creative use of ## in macros might end up with something like FOO_FUNC(add3) hiding a use of silly_prefix_add3_you_wont_find_me_with_grep, and yes, things like that do happen). * When mass-submitting, use threading. It's much easier on maillist readers. * When using threading, at least try to keep the patches in one thread related to each other. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: use of opaque subject lines 2015-02-05 16:30 ` use of opaque subject lines Al Viro @ 2015-02-05 16:57 ` Lad, Prabhakar 2015-02-05 17:57 ` Greg Kroah-Hartman 2015-02-05 17:08 ` Joe Perches 1 sibling, 1 reply; 19+ messages in thread From: Lad, Prabhakar @ 2015-02-05 16:57 UTC (permalink / raw) To: Al Viro Cc: Andreas Ruprecht, Oleg Drokin, Andreas Dilger, Greg Kroah-Hartman, HPDD-discuss, OSUOSL Drivers, LKML On Thu, Feb 5, 2015 at 4:30 PM, Al Viro <viro@zeniv.linux.org.uk> wrote: > On Mon, Feb 02, 2015 at 08:13:10PM +0100, Andreas Ruprecht wrote: > >> On a serious note: I do understand what you're getting at, I don't take >> that personally (and I will send a v2 addressing the things above), but >> honestly, this kind of answer might just be a real turn-off for other >> people trying to get into kernel development... >> >> I don't want to start a whole new 'attitude in the kernel community' >> discussion, but I can't just let this go like that, sorry. > > Just during the last 12 hours or so, I've seen the following l-k traffic: > > Subject: [PATCH] usb: host/sl811-hcd: fix sparse warning > Subject: [PATCH] usb: gadget: function/f_sourcesink: fix sparse warning > Subject: [PATCH] tty: vt/vt: fix sparse warning > Subject: [PATCH] scsi: fix sparse warnings > Subject: [PATCH] bfa: bfa_core: fix sparse warning > Subject: [PATCH] scsi: fix sparse warning > Subject: [PATCH] xen/acpi-processor: fix sparse warning > Subject: [PATCH] scsi: initio: fix sparse warnings > Subject: [PATCH] scsi: dc395x: fix sparse warning > Subject: [PATCH] scsi: eata: fix sparse warning > Subject: [PATCH] scsi: qla1280: fix sparse warnings > Subject: [PATCH] scsi: ips: fix sparse warnings > Subject: [PATCH] fbdev: via/via_clock: fix sparse warning > Subject: [PATCH] usb: gadget: fix sparse warnings > Subject: [PATCH] usb: gadget: fix sparse warnings > Subject: [PATCH] usb: gadget: function/uvc_v4l2.c: fix sparse warnings > Subject: [PATCH] xen-netback: fix sparse warning > Subject: [PATCH] thermal: int340x: fix sparse warning > Subject: [PATCH] vxge: fix sparse warning > Subject: Re: [PATCH] xen-netback: fix sparse warning > Subject: [PATCH] ixgbe: fix sparse warnings > Subject: [PATCH] samsung-laptop: fix sparse warning > Subject: [PATCH] x86: thinkpad_acpi.c: fix sparse warning > Subject: [PATCH] Sony-laptop: fix sparse warning > all right I have stopped the script to send any more patches fixing sparse warnings ! Cheers, --Prabhakar Lad ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: use of opaque subject lines 2015-02-05 16:57 ` Lad, Prabhakar @ 2015-02-05 17:57 ` Greg Kroah-Hartman 2015-02-05 18:22 ` Lad, Prabhakar 2015-02-05 19:32 ` Paul Bolle 0 siblings, 2 replies; 19+ messages in thread From: Greg Kroah-Hartman @ 2015-02-05 17:57 UTC (permalink / raw) To: Lad, Prabhakar Cc: Al Viro, OSUOSL Drivers, Andreas Dilger, LKML, Oleg Drokin, HPDD-discuss, Andreas Ruprecht On Thu, Feb 05, 2015 at 04:57:09PM +0000, Lad, Prabhakar wrote: > On Thu, Feb 5, 2015 at 4:30 PM, Al Viro <viro@zeniv.linux.org.uk> wrote: > > On Mon, Feb 02, 2015 at 08:13:10PM +0100, Andreas Ruprecht wrote: > > > >> On a serious note: I do understand what you're getting at, I don't take > >> that personally (and I will send a v2 addressing the things above), but > >> honestly, this kind of answer might just be a real turn-off for other > >> people trying to get into kernel development... > >> > >> I don't want to start a whole new 'attitude in the kernel community' > >> discussion, but I can't just let this go like that, sorry. > > > > Just during the last 12 hours or so, I've seen the following l-k traffic: > > > > Subject: [PATCH] usb: host/sl811-hcd: fix sparse warning > > Subject: [PATCH] usb: gadget: function/f_sourcesink: fix sparse warning > > Subject: [PATCH] tty: vt/vt: fix sparse warning > > Subject: [PATCH] scsi: fix sparse warnings > > Subject: [PATCH] bfa: bfa_core: fix sparse warning > > Subject: [PATCH] scsi: fix sparse warning > > Subject: [PATCH] xen/acpi-processor: fix sparse warning > > Subject: [PATCH] scsi: initio: fix sparse warnings > > Subject: [PATCH] scsi: dc395x: fix sparse warning > > Subject: [PATCH] scsi: eata: fix sparse warning > > Subject: [PATCH] scsi: qla1280: fix sparse warnings > > Subject: [PATCH] scsi: ips: fix sparse warnings > > Subject: [PATCH] fbdev: via/via_clock: fix sparse warning > > Subject: [PATCH] usb: gadget: fix sparse warnings > > Subject: [PATCH] usb: gadget: fix sparse warnings > > Subject: [PATCH] usb: gadget: function/uvc_v4l2.c: fix sparse warnings > > Subject: [PATCH] xen-netback: fix sparse warning > > Subject: [PATCH] thermal: int340x: fix sparse warning > > Subject: [PATCH] vxge: fix sparse warning > > Subject: Re: [PATCH] xen-netback: fix sparse warning > > Subject: [PATCH] ixgbe: fix sparse warnings > > Subject: [PATCH] samsung-laptop: fix sparse warning > > Subject: [PATCH] x86: thinkpad_acpi.c: fix sparse warning > > Subject: [PATCH] Sony-laptop: fix sparse warning > > > all right I have stopped the script to send any more patches fixing > sparse warnings ! That's not the point at all {sigh} The point is, if you are going to do fixes, also provide a valid subject line too. Think of the people on the receiving end of your patch, they are the most valuable and limited resource our community has right now. You want to make it as _easy_ as possible for them to accept your contribution. If you don't provide enough information, or drown them in redundancy, or crappy patches, they will just get frustrated and drop them all on the floor. And _NEVER_ have automated scripts create patches and send them out. I only know of ONE person/bot that gets away with this, and you are not that person, sorry. It it not a script on the receiving end of your output, so don't use a script to create a mess for them to dig through. I want these types of fixes, but make it easy for us to accept them, not hard, like Al is pointing out in very vivid detail. To respond to his heartfelt plea and detailed instructions with a "fine, I'll just go away!" is disrespectful. greg k-h ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: use of opaque subject lines 2015-02-05 17:57 ` Greg Kroah-Hartman @ 2015-02-05 18:22 ` Lad, Prabhakar 2015-02-05 19:32 ` Paul Bolle 1 sibling, 0 replies; 19+ messages in thread From: Lad, Prabhakar @ 2015-02-05 18:22 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Al Viro, OSUOSL Drivers, Andreas Dilger, LKML, Oleg Drokin, hpdd-discuss, Andreas Ruprecht On Thu, Feb 5, 2015 at 5:57 PM, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > On Thu, Feb 05, 2015 at 04:57:09PM +0000, Lad, Prabhakar wrote: >> On Thu, Feb 5, 2015 at 4:30 PM, Al Viro <viro@zeniv.linux.org.uk> wrote: >> > On Mon, Feb 02, 2015 at 08:13:10PM +0100, Andreas Ruprecht wrote: >> > >> >> On a serious note: I do understand what you're getting at, I don't take >> >> that personally (and I will send a v2 addressing the things above), but >> >> honestly, this kind of answer might just be a real turn-off for other >> >> people trying to get into kernel development... >> >> >> >> I don't want to start a whole new 'attitude in the kernel community' >> >> discussion, but I can't just let this go like that, sorry. >> > >> > Just during the last 12 hours or so, I've seen the following l-k traffic: >> > >> > Subject: [PATCH] usb: host/sl811-hcd: fix sparse warning >> > Subject: [PATCH] usb: gadget: function/f_sourcesink: fix sparse warning >> > Subject: [PATCH] tty: vt/vt: fix sparse warning >> > Subject: [PATCH] scsi: fix sparse warnings >> > Subject: [PATCH] bfa: bfa_core: fix sparse warning >> > Subject: [PATCH] scsi: fix sparse warning >> > Subject: [PATCH] xen/acpi-processor: fix sparse warning >> > Subject: [PATCH] scsi: initio: fix sparse warnings >> > Subject: [PATCH] scsi: dc395x: fix sparse warning >> > Subject: [PATCH] scsi: eata: fix sparse warning >> > Subject: [PATCH] scsi: qla1280: fix sparse warnings >> > Subject: [PATCH] scsi: ips: fix sparse warnings >> > Subject: [PATCH] fbdev: via/via_clock: fix sparse warning >> > Subject: [PATCH] usb: gadget: fix sparse warnings >> > Subject: [PATCH] usb: gadget: fix sparse warnings >> > Subject: [PATCH] usb: gadget: function/uvc_v4l2.c: fix sparse warnings >> > Subject: [PATCH] xen-netback: fix sparse warning >> > Subject: [PATCH] thermal: int340x: fix sparse warning >> > Subject: [PATCH] vxge: fix sparse warning >> > Subject: Re: [PATCH] xen-netback: fix sparse warning >> > Subject: [PATCH] ixgbe: fix sparse warnings >> > Subject: [PATCH] samsung-laptop: fix sparse warning >> > Subject: [PATCH] x86: thinkpad_acpi.c: fix sparse warning >> > Subject: [PATCH] Sony-laptop: fix sparse warning >> > >> all right I have stopped the script to send any more patches fixing >> sparse warnings ! > > That's not the point at all {sigh} > > The point is, if you are going to do fixes, also provide a valid subject > line too. Think of the people on the receiving end of your patch, they > are the most valuable and limited resource our community has right now. > You want to make it as _easy_ as possible for them to accept your > contribution. If you don't provide enough information, or drown them in > redundancy, or crappy patches, they will just get frustrated and drop > them all on the floor. > > And _NEVER_ have automated scripts create patches and send them out. I > only know of ONE person/bot that gets away with this, and you are not > that person, sorry. It it not a script on the receiving end of your > output, so don't use a script to create a mess for them to dig through. > > I want these types of fixes, but make it easy for us to accept them, not > hard, like Al is pointing out in very vivid detail. To respond to his > heartfelt plea and detailed instructions with a "fine, I'll just go > away!" is disrespectful. > Sorry for that. I agree a proper a description is needed ideally, but all these days prior to my patches, the subject line was 'fix sparse warnings' for such patches, that’s the reason I picked it for my script. I understand people get annoyed seeing so many continuous patches with same subject line, ill make my script a bit smarter to have detailed explanation now on. Just a side note I verify the patch created by the script and if its OK then only I post it to ML. Regards, --Prabhakar Lad ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: use of opaque subject lines 2015-02-05 17:57 ` Greg Kroah-Hartman 2015-02-05 18:22 ` Lad, Prabhakar @ 2015-02-05 19:32 ` Paul Bolle 2015-02-05 20:06 ` Greg Kroah-Hartman 1 sibling, 1 reply; 19+ messages in thread From: Paul Bolle @ 2015-02-05 19:32 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Valentin Rothberg, Lad, Prabhakar, Al Viro, OSUOSL Drivers, Andreas Dilger, LKML, Oleg Drokin, HPDD-discuss, Andreas Ruprecht On Thu, 2015-02-05 at 09:57 -0800, Greg Kroah-Hartman wrote: > And _NEVER_ have automated scripts create patches and send them out. I > only know of ONE person/bot that gets away with this, and you are not > that person, sorry. It it not a script on the receiving end of your > output, so don't use a script to create a mess for them to dig through. Mind if I ask which person/bot you have in mind? People are trying to bot away the kconfig related messages I send out - which I think is a very good idea - and I'd like to know what level that bot should reach before people can get out of its way. (I happen to think that the stuff I currently do is best done with a human filter applied.) Thanks, Paul Bolle ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: use of opaque subject lines 2015-02-05 19:32 ` Paul Bolle @ 2015-02-05 20:06 ` Greg Kroah-Hartman 2015-02-05 20:53 ` Joe Perches 0 siblings, 1 reply; 19+ messages in thread From: Greg Kroah-Hartman @ 2015-02-05 20:06 UTC (permalink / raw) To: Paul Bolle Cc: Valentin Rothberg, Lad, Prabhakar, Al Viro, OSUOSL Drivers, Andreas Dilger, LKML, Oleg Drokin, HPDD-discuss, Andreas Ruprecht On Thu, Feb 05, 2015 at 08:32:13PM +0100, Paul Bolle wrote: > On Thu, 2015-02-05 at 09:57 -0800, Greg Kroah-Hartman wrote: > > And _NEVER_ have automated scripts create patches and send them out. I > > only know of ONE person/bot that gets away with this, and you are not > > that person, sorry. It it not a script on the receiving end of your > > output, so don't use a script to create a mess for them to dig through. > > Mind if I ask which person/bot you have in mind? If you don't know that there is a bot doing patches, then that proves it is so good that people do not realize it :) > People are trying to > bot away the kconfig related messages I send out - which I think is a > very good idea - and I'd like to know what level that bot should reach > before people can get out of its way. > > (I happen to think that the stuff I currently do is best done with a > human filter applied.) Please stick with a human filter, that's the best thing possible. thanks, greg k-h ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: use of opaque subject lines 2015-02-05 20:06 ` Greg Kroah-Hartman @ 2015-02-05 20:53 ` Joe Perches 0 siblings, 0 replies; 19+ messages in thread From: Joe Perches @ 2015-02-05 20:53 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Paul Bolle, Valentin Rothberg, Lad, Prabhakar, Al Viro, OSUOSL Drivers, Andreas Dilger, LKML, Oleg Drokin, HPDD-discuss, Andreas Ruprecht On Thu, 2015-02-05 at 12:06 -0800, Greg Kroah-Hartman wrote: > On Thu, Feb 05, 2015 at 08:32:13PM +0100, Paul Bolle wrote: > > On Thu, 2015-02-05 at 09:57 -0800, Greg Kroah-Hartman wrote: > > > And _NEVER_ have automated scripts create patches and send them out. I > > > only know of ONE person/bot that gets away with this, and you are not > > > that person, sorry. It it not a script on the receiving end of your > > > output, so don't use a script to create a mess for them to dig through. > > > > Mind if I ask which person/bot you have in mind? > > If you don't know that there is a bot doing patches, then that proves it > is so good that people do not realize it :) Hmm. That proves bots should be used to create patches. ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: use of opaque subject lines 2015-02-05 16:30 ` use of opaque subject lines Al Viro 2015-02-05 16:57 ` Lad, Prabhakar @ 2015-02-05 17:08 ` Joe Perches 2015-02-05 17:25 ` Dan Carpenter 2015-02-05 18:03 ` Greg Kroah-Hartman 1 sibling, 2 replies; 19+ messages in thread From: Joe Perches @ 2015-02-05 17:08 UTC (permalink / raw) To: Al Viro Cc: Andreas Ruprecht, Oleg Drokin, Andreas Dilger, Greg Kroah-Hartman, HPDD-discuss, devel, linux-kernel, Lad, Prabhakar On Thu, 2015-02-05 at 16:30 +0000, Al Viro wrote: > On Mon, Feb 02, 2015 at 08:13:10PM +0100, Andreas Ruprecht wrote: > > > On a serious note: I do understand what you're getting at, I don't take > > that personally (and I will send a v2 addressing the things above), but > > honestly, this kind of answer might just be a real turn-off for other > > people trying to get into kernel development... > > > > I don't want to start a whole new 'attitude in the kernel community' > > discussion, but I can't just let this go like that, sorry. Maybe YA checkpatch warning when patch subjects include either "checkpatch" or "sparse" would help? Something like: --- scripts/checkpatch.pl | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 3642b0d..b6bed59 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -2170,6 +2170,13 @@ sub process { } } +# Check email subject for poor style + if ($in_header_lines && + $line =~ /^Subject:.*\b(?:checkpatch|sparse)\b[^:]/i) { + WARN("EMAIL_SUBJECT", + "A patch subject line should describe the change not the tool that found it\n" . $herecurr); + } + # Check for old stable address if ($line =~ /^\s*cc:\s*.*<?\bstable\@kernel\.org\b>?.*$/i) { ERROR("STABLE_ADDRESS", ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: use of opaque subject lines 2015-02-05 17:08 ` Joe Perches @ 2015-02-05 17:25 ` Dan Carpenter 2015-02-05 17:31 ` Joe Perches 2015-02-05 18:03 ` Greg Kroah-Hartman 1 sibling, 1 reply; 19+ messages in thread From: Dan Carpenter @ 2015-02-05 17:25 UTC (permalink / raw) To: Joe Perches Cc: Al Viro, devel, Andreas Dilger, Greg Kroah-Hartman, linux-kernel, Oleg Drokin, Lad, Prabhakar, HPDD-discuss, Andreas Ruprecht Acked-by: Dan Carpenter <dan.carpenter@oracle.com> regards, dan carpenter ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: use of opaque subject lines 2015-02-05 17:25 ` Dan Carpenter @ 2015-02-05 17:31 ` Joe Perches 2015-02-05 17:32 ` Joe Perches 0 siblings, 1 reply; 19+ messages in thread From: Joe Perches @ 2015-02-05 17:31 UTC (permalink / raw) To: Dan Carpenter Cc: Al Viro, devel, Andreas Dilger, Greg Kroah-Hartman, linux-kernel, Oleg Drokin, Lad, Prabhakar, HPDD-discuss, Andreas Ruprecht On Thu, 2015-02-05 at 20:25 +0300, Dan Carpenter wrote: > Acked-by: Dan Carpenter <dan.carpenter@oracle.com> Maybe I should add sparse too... ;) ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: use of opaque subject lines 2015-02-05 17:31 ` Joe Perches @ 2015-02-05 17:32 ` Joe Perches 2015-02-05 17:35 ` Dan Carpenter 0 siblings, 1 reply; 19+ messages in thread From: Joe Perches @ 2015-02-05 17:32 UTC (permalink / raw) To: Dan Carpenter Cc: Al Viro, devel, Andreas Dilger, Greg Kroah-Hartman, linux-kernel, Oleg Drokin, Lad, Prabhakar, HPDD-discuss, Andreas Ruprecht On Thu, 2015-02-05 at 09:31 -0800, Joe Perches wrote: > On Thu, 2015-02-05 at 20:25 +0300, Dan Carpenter wrote: > > Acked-by: Dan Carpenter <dan.carpenter@oracle.com> > > Maybe I should add sparse too... ;) drat, of course I meant smatch... ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: use of opaque subject lines 2015-02-05 17:32 ` Joe Perches @ 2015-02-05 17:35 ` Dan Carpenter 2015-02-05 18:01 ` Joe Perches 0 siblings, 1 reply; 19+ messages in thread From: Dan Carpenter @ 2015-02-05 17:35 UTC (permalink / raw) To: Joe Perches Cc: Al Viro, devel, Andreas Dilger, Greg Kroah-Hartman, linux-kernel, Oleg Drokin, Lad, Prabhakar, HPDD-discuss, Andreas Ruprecht On Thu, Feb 05, 2015 at 09:32:55AM -0800, Joe Perches wrote: > On Thu, 2015-02-05 at 09:31 -0800, Joe Perches wrote: > > On Thu, 2015-02-05 at 20:25 +0300, Dan Carpenter wrote: > > > Acked-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > Maybe I should add sparse too... ;) > > drat, of course I meant smatch... > No need. Smatch users are awesome at writing changelogs. regards, dan carpenter ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: use of opaque subject lines 2015-02-05 17:35 ` Dan Carpenter @ 2015-02-05 18:01 ` Joe Perches 2015-02-05 18:26 ` Dan Carpenter 0 siblings, 1 reply; 19+ messages in thread From: Joe Perches @ 2015-02-05 18:01 UTC (permalink / raw) To: Dan Carpenter Cc: Al Viro, devel, Andreas Dilger, Greg Kroah-Hartman, linux-kernel, Oleg Drokin, Lad, Prabhakar, HPDD-discuss, Andreas Ruprecht On Thu, 2015-02-05 at 20:35 +0300, Dan Carpenter wrote: > On Thu, Feb 05, 2015 at 09:32:55AM -0800, Joe Perches wrote: > > On Thu, 2015-02-05 at 09:31 -0800, Joe Perches wrote: > > > On Thu, 2015-02-05 at 20:25 +0300, Dan Carpenter wrote: > > > > Acked-by: Dan Carpenter <dan.carpenter@oracle.com> > > > Maybe I should add sparse too... ;) > > drat, of course I meant smatch... > No need. Smatch users are awesome at writing changelogs. :) good point, kinda... $ git log --format=oneline | grep -wi smatch | wc -l 141 $ git log --pretty=oneline | grep -wi sparse | wc -l 2383 $ git log --pretty=oneline | grep -Pi "\bcheckpatch\b[^:]" | wc -l 1637 ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: use of opaque subject lines 2015-02-05 18:01 ` Joe Perches @ 2015-02-05 18:26 ` Dan Carpenter 0 siblings, 0 replies; 19+ messages in thread From: Dan Carpenter @ 2015-02-05 18:26 UTC (permalink / raw) To: Joe Perches Cc: devel, Andreas Dilger, Greg Kroah-Hartman, linux-kernel, Oleg Drokin, Lad, Prabhakar, Al Viro, HPDD-discuss, Andreas Ruprecht On Thu, Feb 05, 2015 at 10:01:39AM -0800, Joe Perches wrote: > On Thu, 2015-02-05 at 20:35 +0300, Dan Carpenter wrote: > > On Thu, Feb 05, 2015 at 09:32:55AM -0800, Joe Perches wrote: > > > On Thu, 2015-02-05 at 09:31 -0800, Joe Perches wrote: > > > > On Thu, 2015-02-05 at 20:25 +0300, Dan Carpenter wrote: > > > > > Acked-by: Dan Carpenter <dan.carpenter@oracle.com> > > > > Maybe I should add sparse too... ;) > > > drat, of course I meant smatch... > > No need. Smatch users are awesome at writing changelogs. > > :) good point, kinda... > > $ git log --format=oneline | grep -wi smatch | wc -l > 141 Please add smatch to the list of tools. regards, dan carpenter ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: use of opaque subject lines 2015-02-05 17:08 ` Joe Perches 2015-02-05 17:25 ` Dan Carpenter @ 2015-02-05 18:03 ` Greg Kroah-Hartman 1 sibling, 0 replies; 19+ messages in thread From: Greg Kroah-Hartman @ 2015-02-05 18:03 UTC (permalink / raw) To: Joe Perches Cc: Al Viro, Andreas Ruprecht, Oleg Drokin, Andreas Dilger, HPDD-discuss, devel, linux-kernel, Lad, Prabhakar On Thu, Feb 05, 2015 at 09:08:59AM -0800, Joe Perches wrote: > On Thu, 2015-02-05 at 16:30 +0000, Al Viro wrote: > > On Mon, Feb 02, 2015 at 08:13:10PM +0100, Andreas Ruprecht wrote: > > > > > On a serious note: I do understand what you're getting at, I don't take > > > that personally (and I will send a v2 addressing the things above), but > > > honestly, this kind of answer might just be a real turn-off for other > > > people trying to get into kernel development... > > > > > > I don't want to start a whole new 'attitude in the kernel community' > > > discussion, but I can't just let this go like that, sorry. > > Maybe YA checkpatch warning when patch subjects > include either "checkpatch" or "sparse" would help? > > Something like: > --- > scripts/checkpatch.pl | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl > index 3642b0d..b6bed59 100755 > --- a/scripts/checkpatch.pl > +++ b/scripts/checkpatch.pl > @@ -2170,6 +2170,13 @@ sub process { > } > } > > +# Check email subject for poor style > + if ($in_header_lines && > + $line =~ /^Subject:.*\b(?:checkpatch|sparse)\b[^:]/i) { > + WARN("EMAIL_SUBJECT", > + "A patch subject line should describe the change not the tool that found it\n" . $herecurr); > + } > + > # Check for old stable address > if ($line =~ /^\s*cc:\s*.*<?\bstable\@kernel\.org\b>?.*$/i) { > ERROR("STABLE_ADDRESS", > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH] staging: lustre: osc: Make osc_init() static 2015-02-02 14:16 ` Al Viro 2015-02-02 19:13 ` Andreas Ruprecht @ 2015-02-02 19:24 ` Andreas Ruprecht 1 sibling, 0 replies; 19+ messages in thread From: Andreas Ruprecht @ 2015-02-02 19:24 UTC (permalink / raw) To: Oleg Drokin Cc: Andreas Dilger, Greg Kroah-Hartman, HPDD-discuss, devel, linux-kernel, viro, Andreas Ruprecht osc_init() is marked as the module_init function in osc_request.c and is never used anywhere else. Hence, it can (and should) be declared static. sparse also complained about this with the following warning, which is fixed by this patch. andreas@workbox:~/linux-next$ make C=1 M=drivers/staging/lustre/lustre/osc/ [...] drivers/staging/lustre/lustre/osc/osc_request.c:3335:12: warning: symbol 'osc_init' was not declared. Should it be static? [...] Signed-off-by: Andreas Ruprecht <rupran@einserver.de> --- drivers/staging/lustre/lustre/osc/osc_request.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/staging/lustre/lustre/osc/osc_request.c b/drivers/staging/lustre/lustre/osc/osc_request.c index b9450b9..0adfa70 100644 --- a/drivers/staging/lustre/lustre/osc/osc_request.c +++ b/drivers/staging/lustre/lustre/osc/osc_request.c @@ -3332,7 +3332,7 @@ extern struct lu_kmem_descr osc_caches[]; extern spinlock_t osc_ast_guard; extern struct lock_class_key osc_ast_guard_class; -int __init osc_init(void) +static int __init osc_init(void) { struct lprocfs_static_vars lvars = { NULL }; int rc; -- 1.9.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
end of thread, other threads:[~2015-02-05 20:54 UTC | newest] Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-02-02 13:36 [PATCH] staging: lustre: osc: Fix sparse warning about osc_init Andreas Ruprecht 2015-02-02 14:16 ` Al Viro 2015-02-02 19:13 ` Andreas Ruprecht 2015-02-05 16:30 ` use of opaque subject lines Al Viro 2015-02-05 16:57 ` Lad, Prabhakar 2015-02-05 17:57 ` Greg Kroah-Hartman 2015-02-05 18:22 ` Lad, Prabhakar 2015-02-05 19:32 ` Paul Bolle 2015-02-05 20:06 ` Greg Kroah-Hartman 2015-02-05 20:53 ` Joe Perches 2015-02-05 17:08 ` Joe Perches 2015-02-05 17:25 ` Dan Carpenter 2015-02-05 17:31 ` Joe Perches 2015-02-05 17:32 ` Joe Perches 2015-02-05 17:35 ` Dan Carpenter 2015-02-05 18:01 ` Joe Perches 2015-02-05 18:26 ` Dan Carpenter 2015-02-05 18:03 ` Greg Kroah-Hartman 2015-02-02 19:24 ` [PATCH] staging: lustre: osc: Make osc_init() static Andreas Ruprecht
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.