From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jens Axboe Subject: Re: [PATCH 1/2] [SCSI] Print only a single message "rejecting I/O to device being removed" Date: Mon, 6 Nov 2006 22:00:24 +0100 Message-ID: <20061106210023.GJ19471@kernel.dk> References: <20061105113628.GU13555@kernel.dk> <267953.40180.qm@web31808.mail.mud.yahoo.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Received: from brick.kernel.dk ([62.242.22.158]:34839 "EHLO kernel.dk") by vger.kernel.org with ESMTP id S1752499AbWKFU6Q (ORCPT ); Mon, 6 Nov 2006 15:58:16 -0500 Content-Disposition: inline In-Reply-To: <267953.40180.qm@web31808.mail.mud.yahoo.com> Sender: linux-scsi-owner@vger.kernel.org List-Id: linux-scsi@vger.kernel.org To: Luben Tuikov Cc: linux-scsi , James.Bottomley@SteelEye.com On Sun, Nov 05 2006, Luben Tuikov wrote: > --- Jens Axboe wrote: > > On Sat, Nov 04 2006, Luben Tuikov wrote: > > > --- Jens Axboe wrote: > > > > On Fri, Nov 03 2006, Luben Tuikov wrote: > > > > > ... at device removal. > > > > > > > > > > Signed-off-by: Luben Tuikov > > > > > --- > > > > > drivers/scsi/scsi_lib.c | 10 +++++++--- > > > > > drivers/scsi/scsi_scan.c | 1 + > > > > > include/scsi/scsi_device.h | 3 +++ > > > > > 3 files changed, 11 insertions(+), 3 deletions(-) > > > > > > > > > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > > > > > index 7b0f9a3..f0de7ca 100644 > > > > > --- a/drivers/scsi/scsi_lib.c > > > > > +++ b/drivers/scsi/scsi_lib.c > > > > > @@ -1302,9 +1302,13 @@ static int scsi_prep_fn(struct request_q > > > > > if(specials_only == SDEV_QUIESCE || > > > > > specials_only == SDEV_BLOCK) > > > > > goto defer; > > > > > - > > > > > - sdev_printk(KERN_ERR, sdev, > > > > > - "rejecting I/O to device being removed\n"); > > > > > + > > > > > + if (sdev->num_rej_messages > 0) { > > > > > + sdev->num_rej_messages--; > > > > > + sdev_printk(KERN_ERR, sdev, > > > > > + "rejecting I/O to device " > > > > > + "being removed\n"); > > > > > + } > > > > > > > > How about using some variant of printk_ratelimit() instead? > > > > > > Jens, I didn't think to use such a heavy-weight as printk_ratelimit() > > > (grabbing irq spinlocks et al), since the device struct would be > > > "freed" shortly. FWIW, one message is more than enough. I'm being > > > liberal with 5, but would gladly revert it back to the original > > > of one message. > > > > It's a printk, it doesn't matter how heavy weight it is. Just seems > > silly to re-invent some sort of printk limiter, when one already exists. > > As with anything in life -- beauty is in the eye of the beholder. > Having said that, I'll take a patch using printk_ratelimint(), too. > > The bottom line was that I didn't want over 2 million messages > of sorts to be printed within very short period of time and > soft-locking all 4 CPUs. > > > > Feel free to resubmit using printk_ratelimit(). > > > > Sorry no plans to do that, I'll let James decide what he wants to take > > or not. > > You seem to know what is better for upsream Linux, i.e. using > printk_ratelimit() vs. printk() on a device which will be freed. > > Why not submit your own version of the same thing "doing it the right > way"? "my" version would be really simple - either just use if (printk_ratelimit()) or add a scsi_printk_ratelimit() with personalized settings. I do agree that we have an issue there, we have the potential to generate oodles of these offline or reject messages and currently nothing limiting them. So, again, I'll ask James' (this time cc'ed) opinion on what he thinks. Perhaps he agrees that we should go the ratelimit route, or perhaps he likes your version. If the former, I'll be happy to code that up (should be a 1 minute job). -- Jens Axboe