* Re: [PATCH] lib/scatterlist: Fix memory leak in sgl_alloc_order() [not found] <e69e9865-a599-5bd9-95b1-7d57c7e2e90c@web.de> @ 2020-09-20 19:35 ` Douglas Gilbert [not found] ` <d8eb3d0e-52e2-1dab-ac02-774fdbd9c18c@web.de> 2020-09-21 8:37 ` Dan Carpenter 0 siblings, 2 replies; 3+ messages in thread From: Douglas Gilbert @ 2020-09-20 19:35 UTC (permalink / raw) To: Markus Elfring, linux-block Cc: linux-kernel, kernel-janitors, Bart Van Assche, Jens Axboe On 2020-09-20 1:09 p.m., Markus Elfring wrote: >> Noticed that when sgl_alloc_order() failed with order > 0 that >> free memory on my machine shrank. That function shouldn't call >> sgl_free() on its error path since that is only correct when >> order=0 . > > * Would an imperative wording become helpful for the change description? No passive tense there. Or do you mean usage like: "Go to hell" or "Fix memory leak in ..."? I studied French and Latin at school; at a guess, my mother tongue got its grammar from the former. My mother taught English grammar and the term "imperative wording" rings no bells in my grammatical education. Google agrees with me. Please define: "imperative wording". > * How do you think about to add the tag “Fixes” to the commit message?r In the workflow I'm used to, others (closer to LT) make that decision. Why waste my time? > * Will an other patch subject be more appropriate? Twas testing a 6 GB allocation with said function on my 8 GB laptop. It failed and free told me 5 GB had disappeared (and 'cat /sys/kernel/debug/kmemleak' told me _nothing_). Umm, it is potentially a HUGE f@#$ing memory LEAK! Best to call a spade a spade. Doug Gilbert ^ permalink raw reply [flat|nested] 3+ messages in thread
[parent not found: <d8eb3d0e-52e2-1dab-ac02-774fdbd9c18c@web.de>]
* Re: [PATCH] lib/scatterlist: Fix memory leak in sgl_alloc_order() [not found] ` <d8eb3d0e-52e2-1dab-ac02-774fdbd9c18c@web.de> @ 2020-09-21 0:57 ` Douglas Gilbert 0 siblings, 0 replies; 3+ messages in thread From: Douglas Gilbert @ 2020-09-21 0:57 UTC (permalink / raw) To: Markus Elfring, linux-block Cc: linux-kernel, kernel-janitors, Bart Van Assche, Jens Axboe [-- Warning: decoded text below may be mangled, UTF-8 assumed --] [-- Attachment #1: Type: text/plain; charset="windows-1254", Size: 2486 bytes --] On 2020-09-20 4:11 p.m., Markus Elfring wrote: >>>> Noticed that when sgl_alloc_order() failed with order > 0 that >>>> free memory on my machine shrank. That function shouldn't call >>>> sgl_free() on its error path since that is only correct when >>>> order=0 . >>> >>> * Would an imperative wording become helpful for the change description? > … >> … and the term "imperative wording" rings no >> bells in my grammatical education. … > > I suggest to take another look at the published Linux development documentation. > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id½cf11de8f776152c82d2197b255c2d04603f976#n151 > > >>> * How do you think about to add the tag “Fixes†to the commit message?r >> >> In the workflow I'm used to, others (closer to LT) make that decision. >> Why waste my time? > > I find another bit of guidance relevant. > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id½cf11de8f776152c82d2197b255c2d04603f976#n183 > > >>> * Will an other patch subject be more appropriate? >> >> Twas testing a 6 GB allocation with said function on my 8 GB laptop. >> It failed and free told me 5 GB had disappeared … > … > > Have we got any different expectations for the canonical patch subject line? > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id½cf11de8f776152c82d2197b255c2d04603f976#n684 > > I am curious how the software will evolve further also according to your > system test experiences. Sorry, I didn't come down in the last shower, it's not my first bug fix. Try consulting 'git log' and look for my name or the MAINTAINERS file. The culprits are usually happy as was the case with this patch. It's ack-ed and I would be very surprised if Jens Axboe doesn't accept it. It is an obvious flaw. Fix it and move on. Alternatively supply your own patch that ticks all the above boxes. If you want to talk about something substantial, then why do we have a function named sgl_free() that only works properly if, for example, the sgl_alloc_order() function creating the sgl used order=0 ? IMO sgl_free() should be removed or renamed. Doug Gilbert BTW The "imperative mood" stuff in that document is nonsense, at least in English. Wikipedia maps that term back to "the imperative" as in "Get thee to a nunnery" and "Et tu, Brute". ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] lib/scatterlist: Fix memory leak in sgl_alloc_order() 2020-09-20 19:35 ` [PATCH] lib/scatterlist: Fix memory leak in sgl_alloc_order() Douglas Gilbert [not found] ` <d8eb3d0e-52e2-1dab-ac02-774fdbd9c18c@web.de> @ 2020-09-21 8:37 ` Dan Carpenter 1 sibling, 0 replies; 3+ messages in thread From: Dan Carpenter @ 2020-09-21 8:37 UTC (permalink / raw) To: Douglas Gilbert Cc: Markus Elfring, linux-block, linux-kernel, kernel-janitors, Bart Van Assche, Jens Axboe We asked Markus over and over to stop sending these emails but he refused so eventually he had to be banned from vger. Unless you're directly mentioned in the CC list then you can't see his emails. regards, dan carpenter ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-21 8:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <e69e9865-a599-5bd9-95b1-7d57c7e2e90c@web.de>
2020-09-20 19:35 ` [PATCH] lib/scatterlist: Fix memory leak in sgl_alloc_order() Douglas Gilbert
[not found] ` <d8eb3d0e-52e2-1dab-ac02-774fdbd9c18c@web.de>
2020-09-21 0:57 ` Douglas Gilbert
2020-09-21 8:37 ` Dan Carpenter
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).