Date: Fri, 9 Dec 2016 12:03:27 -0800
From: Andrew Morton <akpm@linux-foundation.org>
To: Fabian Frederick <fabf@skynet.be>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
        linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH V2 linux-next] fs: add BLOCKSIZE(inode)
Message-Id: <20161209120327.1e75aaf0cc4c250d0a80f6b9@linux-foundation.org>
In-Reply-To: <1481312313-29680-1-git-send-email-fabf@skynet.be>
References: <1481312313-29680-1-git-send-email-fabf@skynet.be>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 987
Lines: 33

On Fri,  9 Dec 2016 20:38:33 +0100 Fabian Frederick <fabf@skynet.be> wrote:

> Add (1 << inode->i_blkbits) macro for more readability
> #define BLOCKSIZE(inode) (1 << inode->i_blkbits)
> 
> This patch also fixes multiple checkpatch warnings:
> WARNING: Prefer 'unsigned int' to bare use of 'unsigned'
> 
> ...
>
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -543,6 +543,7 @@ is_uncached_acl(struct posix_acl *acl)
>  #define IOP_LOOKUP	0x0002
>  #define IOP_NOFOLLOW	0x0004
>  #define IOP_XATTR	0x0008
> +#define BLOCKSIZE(inode) (1 << inode->i_blkbits)
>  
>  /*
>   * Keep mostly read-only and often accessed (especially for

The name "BLOCKSIZE" is too generic.  Why should inodes consume this
identifier kernel-wide rather than, say, struct bio?

The return type is `int', yes?  I guess a 32-bit quantity is OK, but I
don't see why it should be signed.

So I'd suggest something along the lines of

static inline unsigned i_blocksize(const struct inode *inode)
{
	...
}