From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from ipmail07.adl2.internode.on.net ([150.101.137.131]:27036 "EHLO
	ipmail07.adl2.internode.on.net" rhost-flags-OK-OK-OK-OK)
	by vger.kernel.org with ESMTP id S1751819AbcBUVer (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Sun, 21 Feb 2016 16:34:47 -0500
Date: Mon, 22 Feb 2016 08:34:19 +1100
From: Dave Chinner <david@fromorbit.com>
To: Waiman Long <Waiman.Long@hpe.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>, Jan Kara <jack@suse.com>,
	Jeff Layton <jlayton@poochiereds.net>,
	"J. Bruce Fields" <bfields@fieldses.org>,
	Tejun Heo <tj@kernel.org>,
	Christoph Lameter <cl@linux-foundation.org>,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Andi Kleen <andi@firstfloor.org>,
	Dave Chinner <dchinner@redhat.com>,
	Scott J Norton <scott.norton@hp.com>,
	Douglas Hatch <doug.hatch@hp.com>
Subject: Re: [PATCH v2 3/3] vfs: Use per-cpu list for superblock's inode list
Message-ID: <20160221213419.GB25832@dastard>
References: <1455916245-32707-1-git-send-email-Waiman.Long@hpe.com>
 <1455916245-32707-4-git-send-email-Waiman.Long@hpe.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1455916245-32707-4-git-send-email-Waiman.Long@hpe.com>
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Fri, Feb 19, 2016 at 04:10:45PM -0500, Waiman Long wrote:
> +/*
> + * Superblock's inode list iterator function and arguments macros
> + */
> +#define SB_INODES_ITER_FUNC(name, lock, struct_fields)			\
> +	struct name ## _arg {						\
> +		struct_fields;						\
> +	};								\
> +	static int name ## _iter(struct pcpu_list_node *_node,		\
> +				 struct pcpu_list_node **_pnext,	\
> +				 spinlock_t *lock, void *_arg)
> +
> +#define SB_INODES_ITER_ARGS(name, i, a)					\
> +	struct inode *i = container_of(_node, struct inode, i_sb_list);	\
> +	struct name ## _arg *a = (struct name ## _arg *)_arg
> +
> +#define SB_INODES_ITER_ARGS_SAFE(name, i, n, a)				\
> +	struct inode *i = container_of(_node, struct inode, i_sb_list);	\
> +	struct inode *n = container_of(*_pnext, struct inode, i_sb_list);\
> +	struct name ## _arg *a = (struct name ## _arg *)_arg
> +
> +#define SB_INODES_ITER_SET_PCPU_LIST_NEXT(n)				\
> +	{ *_pnext = &(n)->i_sb_list; }
> +
> +#define SB_INODES_ITER_CALL(name, sb)					\
> +	pcpu_list_iterate(sb->s_inodes, false, NULL, name ## _iter, &arg)
> +
> +#define SB_INODES_ITER_CALL_SAFE(name, sb, phead)			\
> +	pcpu_list_iterate(sb->s_inodes, true, phead, name ## _iter, &arg)
> +

No, just no.

Ungreppable, breaks cscope, obfuscates everything, shouts a lot,
code using the API looks completely broken (e.g. semi-colons in
"function declarations"), and it reminds me of the worst of the
worst unmaintainable code in an exceedingly buggy and undebuggable
proprietary filesystem I've previously had the "joy" of working
with.

Just fix the bug in the previous version; it's so much cleaner than
this .... mess.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com