Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <160f2a29-e935-143a-cdae-7b3e1f2797f9@xen0n.name>
Date:   Thu, 11 May 2023 18:07:29 +0800
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0)
 Gecko/20100101 Thunderbird/102.10.1
Subject: Re: [PATCH v2] LoongArch: Add jump-label implementation
Content-Language: en-US
To:     Peter Zijlstra <peterz@infradead.org>,
        Youling Tang <tangyouling@loongson.cn>
Cc:     Huacai Chen <chenhuacai@kernel.org>,
        Jonathan Corbet <corbet@lwn.net>,
        Josh Poimboeuf <jpoimboe@kernel.org>,
        Jason Baron <jbaron@akamai.com>,
        Zhangjin Wu <falcon@tinylab.org>, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org, loongarch@lists.linux.dev
References: <1683710206-23905-1-git-send-email-tangyouling@loongson.cn>
 <3b360532-6ad5-b22c-b02a-103be491be4c@xen0n.name>
 <72b6c81a-d4ee-575a-ff48-6be7e034ac96@loongson.cn>
 <20230511074359.GQ4253@hirez.programming.kicks-ass.net>
From:   WANG Xuerui <kernel@xen0n.name>
In-Reply-To: <20230511074359.GQ4253@hirez.programming.kicks-ass.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2023/5/11 15:43, Peter Zijlstra wrote:
> On Thu, May 11, 2023 at 09:33:37AM +0800, Youling Tang wrote:
> 
>>>> +void arch_jump_label_transform(struct jump_entry *entry,
>>>> +                   enum jump_label_type type)
>>>> +{
>>>> +    void *addr = (void *)jump_entry_code(entry);
>>>> +    u32 insn;
>>>> +
>>>> +    if (type == JUMP_LABEL_JMP)
>>>
>>> Please use a switch for dealing with enum-typed values.
>>
>> Because the current type only has JUMP_LABEL_NOP and JUMP_LABEL_JMP,
>> using if may be simpler than switch.
> 
> IIRC we used an enum with descriptive names instead of a boolean because
> true/false just doesn't tell you much.
> 
> The whole thing fundamentally is a boolean descision though, either
> you write a JMP or a NOP, jump-labels don't have more options.

Ah thanks for the background. My previous suggestion is just kinda 
generally applicable software engineering best practice, so if the 
actual enum is unlikely to get >2 variants then it should be fine to 
keep using "if". Youling, feel free to ignore the piece of comment, and 
sorry for not doing my archaeology beforehand. :)

-- 
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/