Re: [PATCH V2 linux-next] fs: add BLOCKSIZE(inode)

[Andrew Morton]

On Fri,  9 Dec 2016 20:38:33 +0100 Fabian Frederick <fabf@xxxxxxxxx> 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)
{
	...
}