Thread (20 messages) 20 messages, 3 authors, 2019-02-12

Re: [PATCH 2/5] genirq/affinity: allow driver to setup managed IRQ's affinity

From: Thomas Gleixner <hidden>
Date: 2019-02-11 22:38:22
Also in: linux-nvme, linux-pci, lkml

Ming,

On Mon, 11 Feb 2019, Bjorn Helgaas wrote:
On Mon, Feb 11, 2019 at 11:54:00AM +0800, Ming Lei wrote:
quoted
On Sun, Feb 10, 2019 at 05:30:41PM +0100, Thomas Gleixner wrote:
quoted
On Fri, 25 Jan 2019, Ming Lei wrote:
quoted
This patch introduces callback of .setup_affinity into 'struct
irq_affinity', so that:
Please see Documentation/process/submitting-patches.rst. Search for 'This
patch' ....
Sorry for that, because I am not a native English speaker and it looks a bit
difficult for me to understand the subtle difference.
Sorry I was a bit terse.
I think Thomas is saying that instead of "This patch introduces
callback ...", you could say "Introduce callback of ...".

The changelog is *part* of the patch, so the context is obvious and
there's no need to include the words "This patch".

I make the same changes to patches I receive.  In fact, I would go
even further and say "Add callback .setup_affinity() ..." because "add"
means the same as "introduce" but is shorter and simpler.
Thanks for the explanation, Bjorn!

There is another point here. It's not only the 'This patch introduces ...'
part. It's also good practice to structure the changelog so it provides
context and reasoning first and then tells what the change is, e.g.:

   The current handling of multiple interrupt sets in the core interrupt
   affinity logic, requires the driver to do .......  This is necessary
   because ....

   This handling should be in the core code, but the core implementation
   has no way to recompute the interrupt sets for a given number of
   vectors.

   Add an optional callback to struct affd, which lets the driver recompute
   the interrupt set before the interrupt affinity spreading takes place.

The first paragraph provides context, i.e. the status quo, The second
paragraph provides reasoning what is missing and the last one tells what's
done to solve it.

That's pretty much the same for all changelogs, even if you fix a bug. Just
in the bug case, the second paragraph describes the details of the bug and
the possible consequences.

You really need to look at the changelog as a stand alone information
source. That's important when you look at a commit as an outsider or even
if you look at your own patch 6 month down the road when you already paged
out all the details.

Hope that clarifies it.

Thanks,

	tglx
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help