On Fri, 15 Nov 2013 10:29:56 +0800 majianpeng <majianpeng@xxxxxxxxx> wrote:
> >> Yes.How about those code:
> >> spin_lock_irq(&conf->resync_lock);
> >> retry_check:
> >> if (need_to_wait_for_sync(conf, bio)) {
> >> conf->nr_waiting++;
> >> /* Wait for the barrier to drop.
> >> * However if there are already pending
> >> * requests (preventing the barrier from
> >> * rising completely), and the
> >> * pre-process bio queue isn't empty,
> >> * then don't wait, as we need to empty
> >> * that queue to get the nr_pending
> >> * count down.
> >> */
> >> wait_event_lock_irq(conf->wait_barrier,
> >> !conf->array_frozen &&
> >> (!conf->barrier ||
> >> (conf->nr_pending &&
> >> current->bio_list &&
> >> !bio_list_empty(current->bio_list))),
> >> conf->resync_lock);
> >> conf->nr_waiting--;
> >> goto retry_check;
> >> >
> >
> >It would look a lot better if you made it a while loop:
> >
> > while (need_to_wait....) {
> > nr_waiting++
> > wait_event.....
> > nr_waiting--
> > if (bio_data_dir.....
> >
> For this, i think it don;t need while() or recheck.
> The code should:
>
> if (nedd_to_wait...) {
> nr_waiting++
> wait_event.....
> nr_waiting--
> }
> if (bio && bio_data_dir(bio) == WRITE) {
> do;
> }
>
> I think again need_to_wait...(),it must return false. So we don't recheck this.
> Or am I missing something?
>
An 'if' is fine. I just didn't like the goto.
If you can send me the updated patch in a day or 2 I should be able to get it
into the current merge window.
thanks,
NeilBrown
Attachment:
signature.asc
Description: PGP signature
