Quoting Paul Dickson (paul@xxxxxxxxxxxxxxxxx):
> 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.
I dunno, I thought the goto versoin was cleaner and easier to tell that
the right locks are getting unlocked. The worst part in the second
version is the break in the middle!
-serge
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
