Re: [PATCH] one of $BIGNUM devfs races

Alexander Viro (viro@math.psu.edu)
Mon, 6 Aug 2001 20:34:03 -0400 (EDT)


On Mon, 6 Aug 2001, Richard Gooch wrote:

> This patch has the following ugly construct:
>
> > + /* Ensure table size is enough */
> > + while (fs_info.num_inodes >= fs_info.table_size) {
>
> Putting the allocation inside a while loop is horrible, and isn't a

Why, exactly? I can show you quite a few places where we do exactly that
(allocate and if somebody else had done it before us - free and repeat).
Pretty common for situations when we want low-contention spinlocks
to protect actual reassignment of buffer (in this case BKL acts as such).

> perfect solution anyway. I'm fixing this (and other races) with proper
> locking. If you went to the trouble to start patching, why at least
> didn't you do it cleanly with a lock?

Because it means adding a per-superblock lock for no good reason.

> Furthermore, the patch makes gratuitous formatting changes (which made
> it harder to see what it actually *changed*).

_Gratitious_? You want your style (which, BTW, flies in the face of
Documentation/CodingStyle) - you do it in some vaguely reasonable time.
Excuse me, but I might be inclined to follow your style half a year
ago. By now, IMO, you've lost any grounds for complaining. There is a
bunch of holes. Holes that need fixing. If you "have other priorities"
for that long - expect other folks to start fixing them without any
respect to your opinion on style.

/me is sorely tempted to say "screw it" and just do fork'n'rewrite...

PS: ObYourPropertyManager: karmic retribution?

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/