Re: [patch-4.16] arm/smmuv1,v2: Protect smmu master list with a lock

From: Ian Jackson <iwj@xenproject.org>
To: Julien Grall <julien@xen.org>
Cc: Michal Orzel <michal.orzel@arm.com>,
	Stefano Stabellini <sstabellini@kernel.org>,
	Ian Jackson <iwj@xenproject.org>,
	Julien Grall <julien.grall.oss@gmail.com>,
	xen-devel <xen-devel@lists.xenproject.org>,
	Volodymyr Babchuk <Volodymyr_Babchuk@epam.com>,
	Bertrand Marquis <bertrand.marquis@arm.com>
Subject: Re: [patch-4.16] arm/smmuv1,v2: Protect smmu master list with a lock
Date: Thu, 4 Nov 2021 17:52:24 +0000	[thread overview]
Message-ID: <24964.7640.116612.519384@mariner.uk.xensource.com> (raw)
In-Reply-To: <8ff2dc1a-b640-ec2a-810a-c135f0399130@xen.org>

Julien Grall writes ("Re: [patch-4.16] arm/smmuv1,v2: Protect smmu master list with a lock"):
> On 04/11/2021 09:18, Michal Orzel wrote:
> > On 01.11.2021 21:51, Stefano Stabellini wrote:
> >> On Mon, 1 Nov 2021, Ian Jackson wrote:
> >>> It sounds like this is a possible latent bug, or at least a bad state
> >>> of the code that might lead to the introduction of bad bugs later.

^ this is the upside.

> >>> Can you set out the downside for me too ?  What are the risks ?  How
> >>> are the affected code paths used in 4.16 ?
> >>>
> >>> A good way to think about this is: if taking this patch for 4.16
> >>> causes problems, what would that look like ?
> >>
> >> The patch affects the SMMU code paths that are currently in-use for
> >> non-PCI devices which are currently supported. A bug in this patch could
> >> cause a failure to setup the SMMU for one or more devices. I would
> >> imagine that it would manifest probably as either an error or an hang
> >> (given that it is adding spin locks) early at boot when the SMMU is
> >> configured.
> >>
> >> The validation of this patch would mostly happen by review: it is the
> >> kind of patch that changes some "return -1" into "goto err".

^ this is the downside.

> > In order not to leave this patch high and dry:

Michal, you are right that we should not just stall this.

> My main objection is on the process. We should not merge patch that 
> doesn't fix a real issue at this stage of this release.

I agree with Julien.  I wouldn't characterise this as a process
objection.  I think it is a practical objection.  As I understand it
the patch can only harm the experience of users of 4.16.  The release
process is primarily aimed at making sure 4.16 meets the needs of
users.

So:

Release-Nacked-by: Ian Jackson <iwj@xenproject.org>

I would be very sympathetic to code comment patches which document the
limitations/restrictions so as to make the future bugs less likely.

> ... Ian can we get your release-acked-by?

You can have my decision.  I hope this is helpful.

Thanks,
Ian.