Re: [PATCH] x86/mm: avoid premature success when changing page attributes

[Thomas Gleixner]

On Wed, 27 Jan 2016, Jan Beulich wrote:
> >>> On 27.01.16 at 11:05, <tglx@xxxxxxxxxxxxx> wrote:
> > On Mon, 25 Jan 2016, Jan Beulich wrote:
> > 
> > Sorry, that changelog does not make any sense.
> > 
> >> Since successful return from __cpa_process_fault() makes
> >> __change_page_attr() exit early (and successfully), its caller needs to
> > 
> > That has nothing to do with a successful return from __cpa_process_fault().
> 
> How does it not? When __change_page_attr() calls
> __cpa_process_fault() it doesn't even loot at its return value, but
> directly returns __cpa_process_fault()'s return value to its own
> caller. I.e. success of __cpa_process_fault() means success of
> __change_page_attr().

You wrote:

> >> Since successful return from __cpa_process_fault() makes
> >> __change_page_attr() exit early

This is wrong. Because it implies that a non successful return from
__cpa_process_fault() is not resulting in an early exit of
__change_page_attr().

> > __change_page_attr() always returns immediately after calling
> > __cpa_process_fault() no matter what the return code is.
> > 
> >> be instructed to continue its iteration by adjusting ->numpages.
> > 
> > And how is that instruction working?
> 
> I think some understanding of the internal working of cpa can
> be expected.

I very much know how it works as I wrote large parts of it. Still your
changelog makes no sense to me.

> The only adjustment I can see as being doable for me would be to
> invert the ordering of the description, starting with describing the
> failure scenario. That would nevertheless be with more or less the
> same wording, so I'm quite uncertain it would help (and hence be
> worth my time).

Documentation/SubmittingPatches:: 2) Describe your changes.

does apply to everyone including you. It's well worth the time because badly
written changelogs waste the time of reviewers, maintainers and people who
need to consult changelogs later on.

Thanks,

	tglx