Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
Date:   Thu, 24 Feb 2022 00:30:04 +0200
From:   'Andy Shevchenko' <andriy.shevchenko@linux.intel.com>
To:     David Laight <David.Laight@aculab.com>
Cc:     Jason Gunthorpe <jgg@ziepe.ca>,
        "linux-rdma@vger.kernel.org" <linux-rdma@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Mike Marciniszyn <mike.marciniszyn@cornelisnetworks.com>,
        Dennis Dalessandro <dennis.dalessandro@cornelisnetworks.com>
Subject: Re: [PATCH v1 1/1] IB/hfi1: Don't cast parameter in bit operations
Message-ID: <Yha1bIYZpCWZIowl@smile.fi.intel.com>
References: <20220223185353.51370-1-andriy.shevchenko@linux.intel.com>
 <e39730af26cc4a4d944fa3205fa17b3c@AcuMS.aculab.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <e39730af26cc4a4d944fa3205fa17b3c@AcuMS.aculab.com>
Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
Precedence: bulk

On Wed, Feb 23, 2022 at 09:44:32PM +0000, David Laight wrote:
> From: Andy Shevchenko
> > Sent: 23 February 2022 18:54
> > 
> > While in this particular case it would not be a (critical) issue,
> > the pattern itself is bad and error prone in case somebody blindly
> > copies to their code.
> 
> It is horribly wrong on BE systems.

You mean the pattern? Yes, it has three issues regarding to endianess and
potential out of boundary access.

...

> > -	return handled;
> > +	return IRQ_RETVAL(!bitmap_empty(pending, CCE_NUM_INT_CSRS * 64));

> You really don't want to scan the bitmap again.

Either way it wastes cycles, the outcome depends on the actual distribution of
the interrupts across the bitmap. If it gathered closer to the beginning of the
bitmap, my code wins, otherwise the original ones.

> Actually, of the face of it, you could merge the two loops.
> Provided you clear the status bit before calling the relevant
> handler I expect it will all work.

True. I will consider that for v2.

-- 
With Best Regards,
Andy Shevchenko