On Thu, 2005-09-01 at 18:46 +0800, David Teigland wrote:
> Hi, this is the latest set of gfs patches, it includes some minor munging
> since the previous set. Andrew, could this be added to -mm? there's not
> much in the way of pending changes.
>
> http://redhat.com/~teigland/gfs2/20050901/gfs2-full.patch
> http://redhat.com/~teigland/gfs2/20050901/broken-out/
+static inline void glock_put(struct gfs2_glock *gl)
+{
+ if (atomic_read(&gl->gl_count) == 1)
+ gfs2_glock_schedule_for_reclaim(gl);
+ gfs2_assert(gl->gl_sbd, atomic_read(&gl->gl_count) > 0,);
+ atomic_dec(&gl->gl_count);
+}
this code has a race
what is gfs2_assert() about anyway? please just use BUG_ON directly everywhere
+static inline int queue_empty(struct gfs2_glock *gl, struct list_head *head)
+{
+ int empty;
+ spin_lock(&gl->gl_spin);
+ empty = list_empty(head);
+ spin_unlock(&gl->gl_spin);
+ return empty;
+}
that looks like a racey interface to me... if so.. why bother locking at all?
+void gfs2_glock_hold(struct gfs2_glock *gl)
+{
+ glock_hold(gl);
+}
eh why?
+struct gfs2_holder *gfs2_holder_get(struct gfs2_glock *gl, unsigned int state,
+ int flags, int gfp_flags)
+{
+ struct gfs2_holder *gh;
+
+ gh = kmalloc(sizeof(struct gfs2_holder), GFP_KERNEL | gfp_flags);
this looks odd. Either you take flags or you don't.. this looks really half
arsed and thus is really surprising
to all callers
static int gi_skeleton(struct gfs2_inode *ip, struct gfs2_ioctl *gi,
+ gi_filler_t filler)
+{
+ unsigned int size = gfs2_tune_get(ip->i_sbd, gt_lockdump_size);
+ char *buf;
+ unsigned int count = 0;
+ int error;
+
+ if (size > gi->gi_size)
+ size = gi->gi_size;
+
+ buf = kmalloc(size, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ error = filler(ip, gi, buf, size, &count);
+ if (error)
+ goto out;
+
+ if (copy_to_user(gi->gi_data, buf, count + 1))
+ error = -EFAULT;
where does count get a sensible value?
+static unsigned int handle_roll(atomic_t *a)
+{
+ int x = atomic_read(a);
+ if (x < 0) {
+ atomic_set(a, 0);
+ return 0;
+ }
+ return (unsigned int)x;
+}
this is just plain scary.
you'll have to post the rest of your patches if you want anyone to look at
them...
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
