[dm-devel] dm: rename multipath path selector source files to have "dm-ps" prefix

[Mike Christie]

On 11/11/20 9:24 AM, Mike Snitzer wrote:
> On Wed, Nov 11 2020 at  6:45am -0500,
> Colin Ian King <colin.king at canonical.com> wrote:
> 
>> Hi,
>>
>> Static analysis on linux-next has detected an initialized variable issue
>> with the following recent commit:
>>
>> commit 28784347451fdbf4671ba97018f816041ba2306a
>> Author: Mike Snitzer <snitzer at redhat.com>
>> Date:   Tue Nov 10 13:41:53 2020 -0500
>>
>>      dm: rename multipath path selector source files to have "dm-ps" prefix
>>
>> The analysis is as follows:
>>
>>   43
>> static int ioa_add_path(struct path_selector *ps, struct dm_path *path,
>>   44                        int argc, char **argv, char **error)
>>   45 {
>>   46        struct selector *s = ps->context;
>>   47        struct path_info *pi = NULL;
>>     1. var_decl: Declaring variable cpu without initializer.
>>
>>   48        unsigned int cpu;
>>   49        int ret;
>>   50
>>     2. Condition argc != 1, taking false branch.
>>
>>   51        if (argc != 1) {
>>   52                *error = "io-affinity ps: invalid number of arguments";
>>   53                return -EINVAL;
>>   54        }
>>   55
>>
>>     Uninitialized scalar variable (UNINIT)
>>     3. uninit_use_in_call: Using uninitialized value cpu when calling
>> __cpu_to_node.
>>
>>   56        pi = kzalloc_node(sizeof(*pi), GFP_KERNEL, cpu_to_node(cpu));
>>   57        if (!pi) {
>>   58                *error = "io-affinity ps: Error allocating path context";
>>   59                return -ENOMEM;
>>   60        }
> 
> Valid report, but it focuses on a follow-on commit that is just noise.
> The commit "dm mpath: add IO affinity path selector" is what is in
> quesation, see:
> https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-5.11&id=c3d0a31e609e299836fa6ced28cb9ec06b408181__;!!GqivPVa7Brio!KJegykpnucM-IY4IWXzkUVvJeWoxyfPKk8SAwHrcy8iMldT01JwSwV-Jelxc3x0461yv$
> 
> Regardless, Mike Christie, Colin's report has identified a bug.
> 
> Please advise on how you'd like to fix ioa_add_path()'s allocation of
> 'struct path_info'.. pretty sure 'cpu' will default to 0 (on stack)
> despite no explicit initialization... so code "works" but does not
> do so with a specific cpu allocation affinity.
> 

I meant to drop the kzalloc_node and use kzalloc. I had experimented 
with allocating structs on specific nodes, but I didn't see much 
improvement.

Do you prefer I send a patch on top of what's in your branch now, or is 
that like a staging type of branch where you'll drop my original patch 
and want me to resubmit the original patch with the fix integrated?