From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754205AbcA0KzD (ORCPT ); Wed, 27 Jan 2016 05:55:03 -0500 Received: from www.linutronix.de ([62.245.132.108]:42447 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752171AbcA0Ky6 (ORCPT ); Wed, 27 Jan 2016 05:54:58 -0500 Date: Wed, 27 Jan 2016 11:53:50 +0100 (CET) From: Thomas Gleixner To: Jan Beulich cc: mingo@elte.hu, linux-kernel@vger.kernel.org, hpa@zytor.com Subject: Re: [PATCH] x86/mm: avoid premature success when changing page attributes In-Reply-To: <56A8AD9E02000078000CB7CA@prv-mh.provo.novell.com> Message-ID: References: <56A6615E02000078000CAC96@prv-mh.provo.novell.com> <56A8AD9E02000078000CB7CA@prv-mh.provo.novell.com> User-Agent: Alpine 2.11 (DEB 23 2013-08-11) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII X-Linutronix-Spam-Score: -1.0 X-Linutronix-Spam-Level: - X-Linutronix-Spam-Status: No , -1.0 points, 5.0 required, ALL_TRUSTED=-1,SHORTCIRCUIT=-0.0001,URIBL_BLOCKED=0.001 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 27 Jan 2016, Jan Beulich wrote: > >>> On 27.01.16 at 11:05, 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