linux-kernel - Re: [Patch 2/2] cciss: supercedes add shutdown support (replaces reboot notifier)

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]

Message-ID: <20070223214433.GB26058@beardog.cca.cpqcorp.net>
Date:	Fri, 23 Feb 2007 15:44:33 -0600
From:	"Mike Miller (OS Dev)" <mikem@...rdog.cca.cpqcorp.net>
To:	Greg KH <gregkh@...e.de>
Cc:	akpm@...l.org, jens.axboe@...cle.com, linux-kernel@...r.kernel.org,
	linux-scsi@...r.kernel.org
Subject: Re: [Patch 2/2] cciss: supercedes add shutdown support (replaces reboot notifier)

On Fri, Feb 23, 2007 at 01:32:36PM -0800, Greg KH wrote:
> On Fri, Feb 23, 2007 at 02:42:39PM -0600, Mike Miller (OS Dev) wrote:
> > Patch 2/2
> > 
> > This patch supercedes yesterdays cciss-shutdown patch. The primary difference is
> > removing __devexit from cciss_remove_one. Instead of create another function I'd
> > rather use the code that was intended to perform the cleanup and cache flush. I've
> > tested as a loadable module and statically linked without error.
> > Please consider this for inclusion.
> > 
> > Signed-off-by: Mike Miller <mike.miller@...com>
> > ------------------------------------------------------------------------------------------
> > diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
> > index 9d84ab3..b16f48c 100644
> > --- a/drivers/block/cciss.c
> > +++ b/drivers/block/cciss.c
> > @@ -3404,7 +3404,7 @@ #endif
> >  	return -1;
> >  }
> >  
> > -static void __devexit cciss_remove_one(struct pci_dev *pdev)
> > +static void cciss_remove_one(struct pci_dev *pdev)
> >  {
> >  	ctlr_info_t *tmp_ptr;
> >  	int i, j;
> > @@ -3428,9 +3428,10 @@ static void __devexit cciss_remove_one(s
> >  	memset(flush_buf, 0, 4);
> >  	return_code = sendcmd(CCISS_CACHE_FLUSH, i, flush_buf, 4, 0, 0, 0, NULL,
> >  			      TYPE_CMD);
> > -	if (return_code != IO_OK) {
> > -		printk(KERN_WARNING "Error Flushing cache on controller %d\n",
> > -		       i);
> > +	if (return_code == IO_OK) {
> > +		printk(KERN_WARNING "Completed flushing cache on controller %d\n", i);
> 
> Why do you want the world to know this on every shutdown?
> 
> At the least, use dev_warn() to show the proper device and driver that
> this is happening to, but does it really need to go to the warning log?
> It sounds like a normal operation to me.  How about dev_dbg() instead?
> 
> thanks,
> 
> greg k-h

OK, here's a version that only prints if a problem occurred. That's the way the code
was originally written.

-- mikem
------------------------------------------------------------------------------------------
diff --git a/drivers/block/cciss.c b/drivers/block/cciss.c
index 9d84ab3..c50c09b 100644
--- a/drivers/block/cciss.c
+++ b/drivers/block/cciss.c
@@ -3404,7 +3404,7 @@ #endif
 	return -1;
 }
 
-static void __devexit cciss_remove_one(struct pci_dev *pdev)
+static void cciss_remove_one(struct pci_dev *pdev)
 {
 	ctlr_info_t *tmp_ptr;
 	int i, j;
@@ -3429,8 +3429,7 @@ static void __devexit cciss_remove_one(s
 	return_code = sendcmd(CCISS_CACHE_FLUSH, i, flush_buf, 4, 0, 0, 0, NULL,
 			      TYPE_CMD);
 	if (return_code != IO_OK) {
-		printk(KERN_WARNING "Error Flushing cache on controller %d\n",
-		       i);
+		printk(KERN_WARNING "Error flushing cache on controller %d\n", i);
 	}
 	free_irq(hba[i]->intr[2], hba[i]);
 
@@ -3481,6 +3480,7 @@ static struct pci_driver cciss_pci_drive
 	.probe = cciss_init_one,
 	.remove = __devexit_p(cciss_remove_one),
 	.id_table = cciss_pci_device_id,	/* id_table */
+	.shutdown = cciss_remove_one,
 };
 
 /*
-
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/