lkml.org 
[lkml]   [2016]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: + x86-add-support-for-pud-sized-transparent-hugepages-checkpatch-fixes.patch added to -mm tree
    Hello everyone,

    On Fri, Mar 04, 2016 at 03:30:18PM -0500, Matthew Wilcox wrote:
    > On Wed, Feb 03, 2016 at 08:48:35AM +0100, Ingo Molnar wrote:
    > > > @@ -111,8 +111,10 @@ static inline pud_t native_pudp_get_and_
    > > > #ifdef CONFIG_SMP
    > > > return native_make_pud(xchg(&pudp->pud, 0));
    > > > #else
    > > > - /* native_local_pudp_get_and_clear,
    > > > - but duplicated because of cyclic dependency */
    > > > + /*
    > > > + * native_local_pudp_get_and_clear, but duplicated because of cyclic
    > > > + * dependency
    > > > + */
    > > > pud_t ret = *pudp;
    > > > native_pud_clear(pudp);
    > > > return ret;
    > >
    > > When referring to functions in comments (or changelogs) please use () to make it
    > > clear on sight what is being referred to.
    > >
    > > Also, please try to construct proper English sentences with verbs and such!
    > >
    > > I.e. something like this would work for me:
    > >
    > > > + /*
    > > > + * This is a duplicate of native_local_pudp_get_and_clear(),
    > > > + * because we cannot use the original due to a cyclic header
    > > > + * file dependency:
    > > > + */
    > >
    > > (Assuming I managed to decode the shorthand form properly.)
    >
    > I have no idea what it means. This is copy-and-change of the pmd version,
    > which was originally commit db3eb96f4e6281b84dd33c8980dacc27f2efe177 by
    > Andrea.

    which I also copied from native_ptep_get_and_clear:

    tatic inline pte_t native_ptep_get_and_clear(pte_t *xp)
    {
    #ifdef CONFIG_SMP
    return native_make_pte(xchg(&xp->pte, 0));
    #else
    /* native_local_ptep_get_and_clear,
    but duplicated because of cyclic dependency */
    pte_t ret = *xp;
    native_pte_clear(NULL, 0, xp);
    return ret;
    #endif
    }

    static inline pmd_t native_pmdp_get_and_clear(pmd_t *xp)
    {
    #ifdef CONFIG_SMP
    return native_make_pmd(xchg(&xp->pmd, 0));
    #else
    /* native_local_pmdp_get_and_clear,
    but duplicated because of cyclic dependency */
    pmd_t ret = *xp;
    native_pmd_clear(xp);
    return ret;
    #endif
    }

    So if you intend to expand the comment in native_pmdp_get_and_clear
    that I added with my commit (from v2.6.38), I would suggest to also
    improve the comment in native_ptep_get_and_clear.

    I did only s/ptep/pmdp, the comment originated in commit
    4891645e764d2e181b834509a689fcd12e890c10 (from v2.6.25).

    The comment means native_local_pmdp_get_and_clear() couldn't be
    called, or the build would break because of preprocessor include order
    dependencies. I CC'ed Jeremy just in case, but I've no doubts about
    the comment myself.

    See also what native_local_pmdp_get_and_clear does..

    static inline pmd_t native_local_pmdp_get_and_clear(pmd_t *pmdp)
    {
    pmd_t res = *pmdp;

    native_pmd_clear(pmdp);
    return res;
    }

    It'd be sure fine to improve the comment, but a comment, even a short
    one, was in order. If a solution is found for the include ordering,
    one could call native_local_pmdp_get_and_clear there, so it was good
    to keep that in mind. Nothing special about the pmd-THP part, this
    build issue originated in the pte.

    In fact even before starting to fix the comment, I would recommend to
    try again to call native_local_pmdp_get_and_clear and
    native_local_ptep_get_and_clear to verify if it still breaks, just in
    case the include ordering got fixed by accident in the meanwhile (that
    was a comment in 2.6.25 when arch/x86/include/asm didn't even exist
    yet, it was still in include/asm-x86). If it would manage to build
    without the manual expansion, the comment could go and the duplication
    as well.

    Thanks,
    Andrea

    \
     
     \ /
      Last update: 2016-03-09 19:21    [W:2.145 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site