linux-kernel - RE: [PATCH 7/9] scsi: megaraid_sas - Driver created the self work queue for OCR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 1 Dec 2010 08:55:00 -0700
From:	"Yang, Bo" <Bo.Yang@....com>
To:	Tomas Henzl <thenzl@...hat.com>
CC:	"'James.Bottomley@...senPartnership.com'" 
	<James.Bottomley@...senPartnership.com>,
	"'linux-scsi@...r.kernel.org'" <linux-scsi@...r.kernel.org>,
	"'akpm@...l.org'" <akpm@...l.org>,
	"'linux-kernel@...r.kernel.org'" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 7/9] scsi: megaraid_sas - Driver created the self work
 queue for OCR

Tomas,

The megasas_ocr_wq only used by online controller reset.  Driver can either do return error or continue load by disabling the Online controller reset bit.  I will discuss internally and post back today for the changes.

Regards,

Bo Yang   

-----Original Message-----
From: Tomas Henzl [mailto:thenzl@...hat.com] 
Sent: Wednesday, December 01, 2010 7:19 AM
To: Yang, Bo
Cc: 'James.Bottomley@...senPartnership.com'; 'linux-scsi@...r.kernel.org'; 'akpm@...l.org'; 'linux-kernel@...r.kernel.org'
Subject: Re: [PATCH 7/9] scsi: megaraid_sas - Driver created the self work queue for OCR

On 11/30/2010 07:05 PM, Yang, Bo wrote:
> Tomas,
>
> In which case megasas_ocr_wq will hit NULL if create_workqueue create work queue success? If create_workqueue failed, OCR may have the problem.  But do you know if create_workqueue has the number limitation for creating work queues (how many work queue can it created? 10 or 20)?
>   
+	megasas_ocr_wq = create_workqueue("megasas_ocr");
+
+	if ( !megasas_ocr_wq )
+		printk(KERN_DEBUG "megasas: Failed to create workqueue.\n");

Then why do you check the return value if it can not fail?

You are allocating a lot of resources in the init function and when some of them is not 
available (which is unlikely) you return with an error value. Why don't you want that for 
create_workqueue too?

Is it safe enter the suspend mode with the workqueue still doing some job?
Shouldn't be the workqueue flushed in the suspend function?
 

> Thanks,
>
> Bo Yang
>
> -----Original Message-----
> From: Tomas Henzl [mailto:thenzl@...hat.com]
> Sent: Tuesday, November 30, 2010 11:11 AM 
> To: Yang, Bo
> Cc: 'James.Bottomley@...senPartnership.com'; 'linux-scsi@...r.kernel.org'; 'akpm@...l.org'; 'linux-kernel@...r.kernel.org'
> Subject: Re: [PATCH 7/9] scsi: megaraid_sas - Driver created the self work queue for OCR
>
> On 11/19/2010 06:37 PM, Yang, Bo wrote:
>   
>> Driver created the owner work queue (don't use global work queue). This queue will be used for online controller reset routine.
>>
>> Signed-off-by Bo Yang<bo.yang@....com>
>>
>> ---
>> drivers/scsi/megaraid/megaraid_sas.c |   13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>   
>> diff -rupN old/drivers/scsi/megaraid/megaraid_sas.c
>> new/drivers/scsi/megaraid/megaraid_sas.c ---
>> old/drivers/scsi/megaraid/megaraid_sas.c 2010-11-17 13:46:11.000000000
>> -0500 +++ new/drivers/scsi/megaraid/megaraid_sas.c 2010-11-17
>> 13:47:46.000000000 -0500 @@ -108,6 +108,7 @@ static struct
>> megasas_mgmt_info megasas_ static struct fasync_struct
>> *megasas_async_queue; static DEFINE_MUTEX(megasas_async_queue_mutex);
>> +static struct workqueue_struct *megasas_ocr_wq; static int
>> megasas_poll_wait_aen; static
>> DECLARE_WAIT_QUEUE_HEAD(megasas_poll_wait); static u32
>> support_poll_for_event; @@ -2435,7 +2436,7 @@
>> megasas_deplete_reply_queue(struct megas printk(KERN_NOTICE "megasas:
>> fwState=%x, stage:%d\n", fw_state, instance->adprecovery); -
>> schedule_work(&instance->work_init); + queue_work(megasas_ocr_wq,
>> &instance->work_init); return IRQ_HANDLED; } else { @@ -5157,6
>> +5158,11 @@ static int __init megasas_init(void) goto err_pcidrv; } +
>> megasas_ocr_wq = create_workqueue("megasas_ocr"); + + if (
>> !megasas_ocr_wq ) + printk(KERN_DEBUG "megasas: Failed to create
>> workqueue.\n"); + 
>>     
> What happens later, when you pass the null pointer to the queue_work ?
> I'm not sure this will work.
>
>   
>> rval = driver_create_file(&megasas_pci_driver.driver,
>> &driver_attr_version); if (rval) @@ -5228,6 +5234,11 @@ static void
>> __exit megasas_exit(void) &driver_attr_release_date);
>> driver_remove_file(&megasas_pci_driver.driver, &driver_attr_version);
>> + if (megasas_ocr_wq) { + destroy_workqueue(megasas_ocr_wq); +
>> megasas_ocr_wq = NULL; + } +
>> pci_unregister_driver(&megasas_pci_driver);
>> unregister_chrdev(megasas_mgmt_majorno, "megaraid_sas_ioctl"); } 
>>     
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/