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

David Sterba
2019-09-02T13:43:13Z

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@aol.com/
>
> 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.