mike.miller@xxxxxx wrote:
>
> Patch 3 of 8
> This patch removes a couple of functions dealing with configuration and
> replaces them with new functions. This implementation fixes some bugs
> associated with the ACUXE. It also allows a logical volume to be removed
> from the middle without deleting all volumes behind it.
> If a user has 5 logical volumes and decides he wants to reconfigure volume
> number 3, he can now do that without removing volumes 4 & 5 first. This
> code has been tested in our labs against all application software.
> Please consider this for inclusion.
>
> ...
>
> +static void cciss_update_drive_info(int ctlr, int drv_index)
> + {
> + ctlr_info_t *h = hba[ctlr];
> + struct gendisk *disk;
> + ReadCapdata_struct *size_buff = NULL;
> + InquiryData_struct *inq_buff = NULL;
> + unsigned int block_size;
> + unsigned int total_size;
> + unsigned long flags = 0;
> + int ret = 0;
> +
> + /* if the disk already exists then deregister it before proceeding*/
> + if (h->drv[drv_index].raid_level != -1){
> + spin_lock_irqsave(CCISS_LOCK(h->ctlr), flags);
> + h->drv[drv_index].busy_configuring = 1;
> + spin_unlock_irqrestore(CCISS_LOCK(h->ctlr), flags);
I always get worried when I see a spinlock around a simple assignment like
this - it often doesn't make a lot of sense and can indicate that
something's wrong.
> ...
> +
> + /* Get information about the disk and modify the driver sturcture */
> + size_buff = kmalloc(sizeof( ReadCapdata_struct), GFP_KERNEL);
> + if (size_buff == NULL)
> + goto mem_msg;
> + inq_buff = kmalloc(sizeof( InquiryData_struct), GFP_KERNEL);
> + if (inq_buff == NULL)
> + goto mem_msg;
Indenting went wrong.
urgh, maybe it just looks wrong because someone's being inserting spaces
instead of tabs.
> + ld_buff = kmalloc(sizeof(ReportLunData_struct), GFP_KERNEL);
> + if (ld_buff == NULL)
> + goto mem_msg;
> + memset(ld_buff, 0, sizeof(ReportLunData_struct));
We have kzalloc() now.
> /* make sure logical volume is NOT is use */
> - if( drv->usage_count > 1) {
> - spin_unlock_irqrestore(CCISS_LOCK(ctlr), flags);
> + if(clear_all || (h->gendisk[0] == disk)) {
> + if (drv->usage_count > 1)
> return -EBUSY;
Again, it looks wrong but is probably OK due to broken tab key.
Do you want the code to go in like this or do you want to clean it up?
-
: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
