From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ian Campbell Subject: Re: [PATCH v8 3/8] xen/arm: return int *_dcache_va_range Date: Tue, 21 Oct 2014 10:07:01 +0100 Message-ID: <1413882421.23337.5.camel@citrix.com> References: <1413798499-28314-3-git-send-email-stefano.stabellini@eu.citrix.com> <1413817046.13796.21.camel@citrix.com> <5445452F02000078000405BD@mail.emea.novell.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta4.messagelabs.com ([85.158.143.247]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XgVPG-0001UR-Ir for xen-devel@lists.xenproject.org; Tue, 21 Oct 2014 09:07:06 +0000 In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: julien.grall@citrix.com, xen-devel , Jan Beulich List-Id: xen-devel@lists.xenproject.org On Mon, 2014-10-20 at 17:34 +0100, Stefano Stabellini wrote: > On Mon, 20 Oct 2014, Jan Beulich wrote: > > >>> On 20.10.14 at 16:57, wrote: > > >> --- a/xen/arch/arm/mm.c > > >> +++ b/xen/arch/arm/mm.c > > >> @@ -388,7 +388,7 @@ void flush_page_to_ram(unsigned long mfn) > > >> { > > >> void *v = map_domain_page(mfn); > > >> > > >> - clean_and_invalidate_dcache_va_range(v, PAGE_SIZE); > > >> + ASSERT(clean_and_invalidate_dcache_va_range(v, PAGE_SIZE) == 0); > > > > > > Just this one assert? What about all the other callers of > > > *_dcache_va_range? > > There are many many callers under xen/arch/arm, should I really add > ASSERT everywhere? I dunno, but you should certainly be consistent, i.e. everywhere or nowhere. > Maybe I could just add an ASSERT on the return value within > clean_dcache_va_range and clean_and_invalidate_dcache_va_range? I don't think an ASSERT() is necessary, but a comment before the return 0 like: /* * These cannot fail and many callers from within the ARM * architecture code do not check the result. */ That's assuming there's no possibility of us needing to propagate failure in the future. Since we are talking about a h/w instruction with no possibility of an error return I think we can assert (no pun intended) that this won't happen. > Or should I introduce wrappers with the ASSERT? I don't think so. Ian.