Re: [RFC][PATCH 5/14] Introduce union stack

Paul Dickson
2007-05-19T10:33:51Z

On Mon, 14 May 2007 13:23:06 -0700, Badari Pulavarty wrote:

> > + while (fs) {
> > + locked = union_trylock(fs->root);
> > + if (!locked)
> > + goto loop1;
> > + locked = union_trylock(fs->altroot);
> > + if (!locked)
> > + goto loop2;
> > + locked = union_trylock(fs->pwd);
> > + if (!locked)
> > + goto loop3;
> > + break;
> > + loop3:
> > + union_unlock(fs->altroot);
> > + loop2:
> > + union_unlock(fs->root);
> > + loop1:
> > + read_unlock(&fs->lock);
> > + UM_DEBUG_LOCK("Failed to get all semaphores in fs_struct!\n");
> > + cpu_relax();
> > + read_lock(&fs->lock);
> > + continue;
>
> Nit.. why "continue" ?
>
> > + }
> > + BUG_ON(!fs);

How about getting rid of the gotos:

while (fs) {
locked = union_trylock(fs->root);
if (locked) {
locked = union_trylock(fs->altroot);
if (locked) {
locked = union_trylock(fs->pwd);
if (locked)
break;
else {
union_unlock(fs->altroot);
union_unlock(fs->root);
}
else
union_unlock(fs->root);
}
}
read_unlock(&fs->lock);
UM_DEBUG_LOCK("Failed to get all semaphores in fs_struct!\n");
cpu_relax();
read_lock(&fs->lock);
}
BUG_ON(!fs);

It's the same number of lines. Shorter if you get rid of the "locked"
variable.

-Paul