linux-kernel - Re: [tip:x86/cache] x86/resctrl: Initialize a new resource group with default MBA values

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20190418072845.GA27160@zn.tnic>
Date:   Thu, 18 Apr 2019 09:28:46 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     Xiaochen Shen <xiaochen.shen@...el.com>
Cc:     tony.luck@...el.com, x86@...nel.org, mingo@...hat.com, bp@...e.de,
        reinette.chatre@...el.com, hpa@...or.com, tglx@...utronix.de,
        mingo@...nel.org, linux-kernel@...r.kernel.org,
        fenghua.yu@...el.com, linux-tip-commits@...r.kernel.org
Subject: Re: [tip:x86/cache] x86/resctrl: Initialize a new resource group
 with default MBA values

On Thu, Apr 18, 2019 at 03:03:35PM +0800, Xiaochen Shen wrote:
> In my opinion, this newline is unnecessary. Thank you.

See commit message:

>   [ bp: Add newlines between code blocks for better readability. ]

And I didn't add enough. That code is too crammed.

For example, the new __init_one_rdt_domain() could use some newlines
too, to separate code blocks for better readability. At least before
each comment so that one can visually distinguish code groups
better/faster.

Here the whole function pasted with newlines added where I think they
make sense. This way you have visual separation between the code blocks
and not one big fat clump of code which one has to painstakingly pick
apart.

static int __init_one_rdt_domain(struct rdt_domain *d, struct rdt_resource *r,
                                 u32 closid)
{
        struct rdt_resource *r_cdp = NULL;
        struct rdt_domain *d_cdp = NULL;
        u32 used_b = 0, unused_b = 0;
        unsigned long tmp_cbm;
        enum rdtgrp_mode mode;
        u32 peer_ctl, *ctrl;
        int i;

        rdt_cdp_peer_get(r, d, &r_cdp, &d_cdp);
        d->have_new_ctrl = false;
        d->new_ctrl = r->cache.shareable_bits;
        used_b = r->cache.shareable_bits;
        ctrl = d->ctrl_val;

        for (i = 0; i < closids_supported(); i++, ctrl++) {
                if (closid_allocated(i) && i != closid) {
                        mode = rdtgroup_mode_by_closid(i);
                        if (mode == RDT_MODE_PSEUDO_LOCKSETUP)
                                break;

                        /*
                         * If CDP is active include peer domain's
                         * usage to ensure there is no overlap
                         * with an exclusive group.
                         */
                        if (d_cdp)
                                peer_ctl = d_cdp->ctrl_val[i];
                        else
                                peer_ctl = 0;

                        used_b |= *ctrl | peer_ctl;
                        if (mode == RDT_MODE_SHAREABLE)
                                d->new_ctrl |= *ctrl | peer_ctl;
                }
        }

        if (d->plr && d->plr->cbm > 0)
                used_b |= d->plr->cbm;

        unused_b = used_b ^ (BIT_MASK(r->cache.cbm_len) - 1);
        unused_b &= BIT_MASK(r->cache.cbm_len) - 1;
        d->new_ctrl |= unused_b;

        /*
         * Force the initial CBM to be valid, user can
         * modify the CBM based on system availability.
         */
        cbm_ensure_valid(&d->new_ctrl, r);

        /*
         * Assign the u32 CBM to an unsigned long to ensure that
         * bitmap_weight() does not access out-of-bound memory.
         */
        tmp_cbm = d->new_ctrl;

        if (bitmap_weight(&tmp_cbm, r->cache.cbm_len) < r->cache.min_cbm_bits) {
                rdt_last_cmd_printf("No space on %s:%d\n", r->name, d->id);
                return -ENOSPC;
        }
        d->have_new_ctrl = true;

        return 0;
}

-- 
Regards/Gruss,
    Boris.

Good mailing practices for 400: avoid top-posting and trim the reply.