Re: [PATCH V2 02/20] sbitmap: introduce __sbitmap_for_each_set()

Ming Lei
2017-08-26T08:43:30Z

On Fri, Aug 25, 2017 at 09:36:26PM +0000, Bart Van Assche wrote:
> On Thu, 2017-08-24 at 11:57 +0800, Ming Lei wrote:
> > On Tue, Aug 22, 2017 at 06:28:54PM +0000, Bart Van Assche wrote:
> > > * Whether or not index >= sb->map_nr. I propose to start iterating from the
> > > start of @sb in this case.
> >
> > It has been checked at the end of the loop.
>
> That's not sufficient to avoid an out-of-bounds access if the start index is
> large. If __sbitmap_for_each_set() would accept values for the start index
> argument that result in index >= sb->map_nr then that will simplify code that
> accesses an sbitmap in a round-robin fashion.

Given the only user of this helper is blk_mq_dispatch_rq_from_ctx(), the
start index won't be out of bounds.

>
> > > }
> > >
> > > while (true) {
> > > struct sbitmap_word *word = &sb->map[i];
> > > unsigned int off;
> >
> > Looks you removed the check on 'word->word'.
>
> Yes, and I did that on purpose. If the start index refers to a word that is
> zero then the "if (word->word) continue;" code will cause the end-of-loop
> check to be skipped and hence will cause an infinite loop.

Got it, but it removes the optimization too, :-)

--
Ming