Bug 188531 - Function mtip_block_initialize() does not set error code when the call to ida_pre_get() fails
Summary: Function mtip_block_initialize() does not set error code when the call to ida...
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Other (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: drivers_other
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-25 10:30 UTC by bianpan
Modified: 2017-05-11 09:21 UTC (History)
0 users

See Also:
Kernel Version: linux-4.9-rc6
Subsystem:
Regression: No
Bisected commit-id:


Attachments
A patch to fix the bug (1.39 KB, patch)
2017-05-11 09:21 UTC, bianpan
Details | Diff

Description bianpan 2016-11-25 10:30:12 UTC
The function mtip_block_initialize() defined in file drivers/block/mtip32xx/mtip32xx.c calls ida_pre_get() at line 3940. If the system is REALLY out of memory function ida_pre_get returns 0, otherwise 1. The comment of function mtip_block_initialize() says it should return 0 on success and an error code on failure. However, the return variable rc still keeps value 0 even when function ida_pre_get() fails. Though this error may occur rarely, I think it is better to assign a correct error code (e.g. -ENOMEM) to rc before the jump instruction at line 3941. Codes related to this bug are summarised as follows.

mtip_block_initialize @@ drivers/block/mtip32xx/mtip32xx.c
3904 /*
3905  * Block layer initialization function.
3906  *
3907  * This function is called once by the PCI layer for each P320
3908  * device that is connected to the system.
3909  *
3910  * @dd Pointer to the driver data structure.
3911  *
3912  * return value
3913  *  0 on success else an error code.
3914  */
3915 static int mtip_block_initialize(struct driver_data *dd)
3916 {
3917     int rv = 0, wait_for_rebuild = 0;

         ...
3925     if (mtip_hw_init(dd)) {
3926         rv = -EINVAL;
3927         goto protocol_init_error;
3928     }
         ...
3938     /* Generate the disk name, implemented same as in sd.c */
3939     do {
3940         if (!ida_pre_get(&rssd_index_ida, GFP_KERNEL))
                 // insert "rv = -ENOMEM" before the jump instruction?
3941             goto ida_get_error;   
3942 
3943         spin_lock(&rssd_index_lock);
3944         rv = ida_get_new(&rssd_index_ida, &index);
3945         spin_unlock(&rssd_index_lock);
3946     } while (rv == -EAGAIN);
         ...
4097 ida_get_error:
4098     put_disk(dd->disk);
4099 
4100 alloc_disk_error:
4101     mtip_hw_exit(dd); /* De-initialize the protocol layer. */
4102 
4103 protocol_init_error:
4104     return rv;
4105 }

Thanks very much!
Comment 1 bianpan 2017-05-11 09:21:16 UTC
Created attachment 256373 [details]
A patch to fix the bug

The patch has been merged into the lastest version. So I will close the bug.

Note You need to log in before you can comment on or make changes to this bug.