Re: [PATCH v6 05/24] erofs: add inode operations

From: Gao Xiang
Date: Mon Sep 02 2019 - 09:56:16 EST


Hi David,

On Mon, Sep 02, 2019 at 03:43:29PM +0200, David Sterba wrote:
> On Sun, Sep 01, 2019 at 05:34:00PM +0800, Gao Xiang wrote:
> > > > +static int read_inode(struct inode *inode, void *data)
> > > > +{
> > > > + struct erofs_vnode *vi = EROFS_V(inode);
> > > > + struct erofs_inode_v1 *v1 = data;
> > > > + const unsigned int advise = le16_to_cpu(v1->i_advise);
> > > > + erofs_blk_t nblks = 0;
> > > > +
> > > > + vi->datamode = __inode_data_mapping(advise);
> > >
> > > What is the deal with these magic underscores here and various
> > > other similar helpers?
> >
> > Fixed in
> > https://lore.kernel.org/linux-fsdevel/20190901055130.30572-17-hsiangkao@xxxxxxx/
> >
> > underscores means 'internal' in my thought, it seems somewhat
> > some common practice of Linux kernel, or some recent discussions
> > about it?... I didn't notice these discussions...
>
> I know about a few valid uses of the underscores:
>
> * pattern where the __underscored version does not do locking, while the other
> does
> * similarly for atomic and non-atomic version
> * macro that needs to manipulate the argument name (like glue some
> prefix, so the macro does not have underscores and is supposed to be
> used instead of the function with underscores that needs the full name
> of a variable/constant/..
> * underscore function takes a few more parameters to further tune the
> behaviour, but most users are fine with the defaults and that is
> provided as a function without underscores
> * in case you have just one function of the kind, don't use the underscores
>
> I can lookup examples if you're interested or if the brief description
> is not sufficient. The list covers what I've seen and used, but the list
> may be incomplete.

Thanks, I learn a lot from the above. [thumb]

Thanks,
Gao Xiang