Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Subject: Re: [PATCH] block: Remove redundant assignments
To:     Michal Orzel <michalorzel.eng@gmail.com>,
        Jens Axboe <axboe@kernel.dk>,
        "Richard Russon (FlatCap)" <ldm@flatcap.org>,
        Nathan Chancellor <nathan@kernel.org>,
        Nick Desaulniers <ndesaulniers@google.com>
Cc:     linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-ntfs-dev@lists.sourceforge.net, llvm@lists.linux.dev
References: <20220409101933.207157-1-michalorzel.eng@gmail.com>
 <20220409101933.207157-2-michalorzel.eng@gmail.com>
 <a56d1184-d399-d5f8-765f-5a4f35dacd5e@redhat.com>
 <446a3e88-4c14-300d-862a-1279274c3760@gmail.com>
From:   Tom Rix <trix@redhat.com>
Message-ID: <c2c40f00-777c-f19a-eb5f-6e978ddd6bcf@redhat.com>
Date:   Sat, 9 Apr 2022 09:03:40 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.10.1
MIME-Version: 1.0
In-Reply-To: <446a3e88-4c14-300d-862a-1279274c3760@gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 8bit
Content-Language: en-US
Precedence: bulk


On 4/9/22 6:20 AM, Michal Orzel wrote:
> Hi Tom,
>
>> On 4/9/22 3:19 AM, Michal Orzel wrote:
>>> Get rid of redundant assignments which end up in values not being
>>> read either because they are overwritten or the function ends.
>> This log is the same as your last patch.
>>
>> Instead of a general statement on deadstores, a more specific
>>
>> analysis of the setting being removed would be helpful.
>>
>> This will mean splitting the patch to match the analysis.
>>
>> Tom
>>
> I think that this explanation in a commit is enough for such a trivial patch.
> It informs that we are fixing clang-tidy findings related to deadstores
> in a block subsystem. What analysis would you want to see?
> Something like "Remove deadstore assignment ret = -EINVAL  in a function bio_map_user_iov" ?
> I think that it will create too much of not needed overhead.

The compiler should remove these already.  All of the static analyzers 
produce false positives, if you do not provide analysis the reviewer has 
to assume you have not done any and has to do the false positive 
analysis themselves. This burdens the reviewer for something of small 
value that can be dismissed as churn.

Tom

>
> Cheers,
> Michal
>