lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20251108010153.GF1932966@nvidia.com>
Date: Fri, 7 Nov 2025 21:01:53 -0400
From: Jason Gunthorpe <jgg@...dia.com>
To: Nicolin Chen <nicolinc@...dia.com>
Cc: will@...nel.org, jean-philippe@...aro.org, robin.murphy@....com,
	joro@...tes.org, balbirs@...dia.com, miko.lenczewski@....com,
	peterz@...radead.org, kevin.tian@...el.com, praan@...gle.com,
	linux-arm-kernel@...ts.infradead.org, iommu@...ts.linux.dev,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 3/7] iommu/arm-smmu-v3: Introduce a per-domain
 arm_smmu_invs array

On Fri, Nov 07, 2025 at 12:23:56PM -0800, Nicolin Chen wrote:
> On Fri, Nov 07, 2025 at 03:41:58PM -0400, Jason Gunthorpe wrote:
> > On Mon, Oct 27, 2025 at 11:54:17AM -0700, Nicolin Chen wrote:
> > > +struct arm_smmu_invs *arm_smmu_invs_merge(struct arm_smmu_invs *invs,
> > > +					  struct arm_smmu_invs *to_merge)
> > > +{
> > > +	struct arm_smmu_invs *new_invs;
> > > +	struct arm_smmu_inv *new;
> > > +	size_t num_trashes = 0;
> > > +	size_t num_adds = 0;
> > > +	size_t i, j;
> > > +
> > > +	for (i = j = 0; i != invs->num_invs || j != to_merge->num_invs;) {
> > > +		int cmp = arm_smmu_invs_cmp(invs, i, to_merge, j);
> > > +
> > > +		/* Skip any unwanted trash entry */
> > > +		if (cmp < 0 && !refcount_read(&invs->inv[i].users)) {
> > 
> > Do we need cmp < 0 here and in all these other similar ifs? Can't we
> > just fully ignore trash entries no matter how they cmopare to the
> > other list?
>  
> The index "i" might overflow in case of cmp > 1. So, if we don't
> check cmp, we'd need to check "i != invs->num_invs" instead.
> 
> > If cmp ==0 and we do num_trash++ then the next iteration will see j
> > ass cmp > 1 so it will do num_adds++ and the two will cancel out.
> 
> Yea, cmp == 0 should work with an additional iteration.
> 
> Perhaps we can do this:
> 
> +		int cmp;
> +
> +		/* Skip any trash entry */
> +		if (i != invs->num_invs && !refcount_read(&invs->inv[i].users)) {
> +			num_trashes++;
> +			i++;
> +			continue;
> +		}
> +
> +		cmp = arm_smmu_invs_cmp(invs, i, to_merge, j);
> 
> And I will fix the other two places too.

Mm, now I see why you did it OK, I do think the above is clearer..

Jason

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ