* [PATCH] scsi_lib: rate-limit the error message from failing commands @ 2012-03-05 20:42 Yi Zou 2012-03-06 15:40 ` Tomas Henzl 0 siblings, 1 reply; 5+ messages in thread From: Yi Zou @ 2012-03-05 20:42 UTC (permalink / raw) To: linux-scsi; +Cc: devel When performing a cable pull test w/ active stress I/O using fio over a dual port Intel 82599 FCoE CNA, w/ 256LUNs on one port and about 32LUNs on the other, it is observed that the system becomes not usable due to scsi-ml being busy printing the error messages for all the failing commands. I don't believe this problem is specific to FCoE and these commands are anyway failing due to link being down (DID_NO_CONNECT), just rate-limit the messages here to solve this issue. Signed-off-by: Yi Zou <yi.zou@intel.com> Cc: www.Open-FCoE.org <devel@open-fcoe.org> --- drivers/scsi/scsi_lib.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c index b2c95db..e2128d9 100644 --- a/drivers/scsi/scsi_lib.c +++ b/drivers/scsi/scsi_lib.c @@ -934,7 +934,8 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) case ACTION_FAIL: /* Give up and fail the remainder of the request */ scsi_release_buffers(cmd); - if (!(req->cmd_flags & REQ_QUIET)) { + if (!(req->cmd_flags & REQ_QUIET) && + printk_ratelimit()) { if (description) scmd_printk(KERN_INFO, cmd, "%s\n", description); ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi_lib: rate-limit the error message from failing commands 2012-03-05 20:42 [PATCH] scsi_lib: rate-limit the error message from failing commands Yi Zou @ 2012-03-06 15:40 ` Tomas Henzl 2012-03-06 17:10 ` Zou, Yi 0 siblings, 1 reply; 5+ messages in thread From: Tomas Henzl @ 2012-03-06 15:40 UTC (permalink / raw) To: Yi Zou; +Cc: linux-scsi, devel On 03/05/2012 09:42 PM, Yi Zou wrote: > When performing a cable pull test w/ active stress I/O using fio over > a dual port Intel 82599 FCoE CNA, w/ 256LUNs on one port and about 32LUNs > on the other, it is observed that the system becomes not usable due to > scsi-ml being busy printing the error messages for all the failing commands. > I don't believe this problem is specific to FCoE and these commands are > anyway failing due to link being down (DID_NO_CONNECT), just rate-limit > the messages here to solve this issue. > > Signed-off-by: Yi Zou <yi.zou@intel.com> > Cc: www.Open-FCoE.org <devel@open-fcoe.org> Hi Zou, you shouldn't use printk_ratelimit . checkpatch says: WARNING: Prefer printk_ratelimited or pr_<level>_ratelimited to printk_ratelimit Tomas > --- > > drivers/scsi/scsi_lib.c | 3 ++- > 1 files changed, 2 insertions(+), 1 deletions(-) > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > index b2c95db..e2128d9 100644 > --- a/drivers/scsi/scsi_lib.c > +++ b/drivers/scsi/scsi_lib.c > @@ -934,7 +934,8 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) > case ACTION_FAIL: > /* Give up and fail the remainder of the request */ > scsi_release_buffers(cmd); > - if (!(req->cmd_flags & REQ_QUIET)) { > + if (!(req->cmd_flags & REQ_QUIET) && > + printk_ratelimit()) { > if (description) > scmd_printk(KERN_INFO, cmd, "%s\n", > description); > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] scsi_lib: rate-limit the error message from failing commands 2012-03-06 15:40 ` Tomas Henzl @ 2012-03-06 17:10 ` Zou, Yi 2012-03-07 13:17 ` Tomas Henzl 0 siblings, 1 reply; 5+ messages in thread From: Zou, Yi @ 2012-03-06 17:10 UTC (permalink / raw) To: Tomas Henzl; +Cc: linux-scsi@vger.kernel.org, devel@open-fcoe.org > > On 03/05/2012 09:42 PM, Yi Zou wrote: > > When performing a cable pull test w/ active stress I/O using fio over > > a dual port Intel 82599 FCoE CNA, w/ 256LUNs on one port and about > 32LUNs > > on the other, it is observed that the system becomes not usable due to > > scsi-ml being busy printing the error messages for all the failing > commands. > > I don't believe this problem is specific to FCoE and these commands are > > anyway failing due to link being down (DID_NO_CONNECT), just rate-limit > > the messages here to solve this issue. > > > > Signed-off-by: Yi Zou <yi.zou@intel.com> > > Cc: www.Open-FCoE.org <devel@open-fcoe.org> > Hi Zou, > you shouldn't use printk_ratelimit . > checkpatch says: WARNING: Prefer printk_ratelimited or > pr_<level>_ratelimited to printk_ratelimit > Tomas Yes, I saw that when I did the checkpatch, but pratek_ratelimit() actually serves the purpose better, as it guards the block of the several scsi related prints that follow, and this works well particularly for this bug. For the worst case, the shared ratelimit surpresses everything here, blk i/o error report would still be able pick up printing something. If it is desired, which I doubt, I could change all scmd_printk, scsi_print_xxxx calls to use printk_ratelimited(), seems an overkill for this to me. I am open for a better solution. yi > > > --- > > > > drivers/scsi/scsi_lib.c | 3 ++- > > 1 files changed, 2 insertions(+), 1 deletions(-) > > > > diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > > index b2c95db..e2128d9 100644 > > --- a/drivers/scsi/scsi_lib.c > > +++ b/drivers/scsi/scsi_lib.c > > @@ -934,7 +934,8 @@ void scsi_io_completion(struct scsi_cmnd *cmd, > unsigned int good_bytes) > > case ACTION_FAIL: > > /* Give up and fail the remainder of the request */ > > scsi_release_buffers(cmd); > > - if (!(req->cmd_flags & REQ_QUIET)) { > > + if (!(req->cmd_flags & REQ_QUIET) && > > + printk_ratelimit()) { > > if (description) > > scmd_printk(KERN_INFO, cmd, "%s\n", > > description); > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-scsi" > in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] scsi_lib: rate-limit the error message from failing commands 2012-03-06 17:10 ` Zou, Yi @ 2012-03-07 13:17 ` Tomas Henzl 2012-03-07 14:55 ` Zou, Yi 0 siblings, 1 reply; 5+ messages in thread From: Tomas Henzl @ 2012-03-07 13:17 UTC (permalink / raw) To: Zou, Yi; +Cc: linux-scsi@vger.kernel.org, devel@open-fcoe.org On 03/06/2012 06:10 PM, Zou, Yi wrote: >> On 03/05/2012 09:42 PM, Yi Zou wrote: >>> When performing a cable pull test w/ active stress I/O using fio over >>> a dual port Intel 82599 FCoE CNA, w/ 256LUNs on one port and about >> 32LUNs >>> on the other, it is observed that the system becomes not usable due to >>> scsi-ml being busy printing the error messages for all the failing >> commands. >>> I don't believe this problem is specific to FCoE and these commands are >>> anyway failing due to link being down (DID_NO_CONNECT), just rate-limit >>> the messages here to solve this issue. >>> >>> Signed-off-by: Yi Zou <yi.zou@intel.com> >>> Cc: www.Open-FCoE.org <devel@open-fcoe.org> >> Hi Zou, >> you shouldn't use printk_ratelimit . >> checkpatch says: WARNING: Prefer printk_ratelimited or >> pr_<level>_ratelimited to printk_ratelimit >> Tomas > Yes, I saw that when I did the checkpatch, but pratek_ratelimit() actually > serves the purpose better, as it guards the block of the several scsi related > prints that follow, and this works well particularly for this bug. For the worst > case, the shared ratelimit surpresses everything here, blk i/o error report > would still be able pick up printing something. > > If it is desired, which I doubt, I could change all scmd_printk, scsi_print_xxxx > calls to use printk_ratelimited(), seems an overkill for this to me. I am open > for a better solution. For a whole block of commands you can use __ratelimit, so something like this were possible: @@ -20,6 +20,7 @@ #include <linux/delay.h> #include <linux/hardirq.h> #include <linux/scatterlist.h> +#include <linux/ratelimit.h> #include <scsi/scsi.h> #include <scsi/scsi_cmnd.h> @@ -745,6 +746,8 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) enum {ACTION_FAIL, ACTION_REPREP, ACTION_RETRY, ACTION_DELAYED_RETRY} action; char *description = NULL; + static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL, + DEFAULT_RATELIMIT_BURST); if (result) { sense_valid = scsi_command_normalize_sense(cmd, &sshdr); @@ -935,7 +938,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, unsigned int good_bytes) case ACTION_FAIL: /* Give up and fail the remainder of the request */ scsi_release_buffers(cmd); - if (!(req->cmd_flags & REQ_QUIET)) { + if (!(req->cmd_flags & REQ_QUIET) && __ratelimit(&rs)) { if (description) scmd_printk(KERN_INFO, cmd, "%s\n", description);yi >>> --- >>> >>> drivers/scsi/scsi_lib.c | 3 ++- >>> 1 files changed, 2 insertions(+), 1 deletions(-) >>> >>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c >>> index b2c95db..e2128d9 100644 >>> --- a/drivers/scsi/scsi_lib.c >>> +++ b/drivers/scsi/scsi_lib.c >>> @@ -934,7 +934,8 @@ void scsi_io_completion(struct scsi_cmnd *cmd, >> unsigned int good_bytes) >>> case ACTION_FAIL: >>> /* Give up and fail the remainder of the request */ >>> scsi_release_buffers(cmd); >>> - if (!(req->cmd_flags & REQ_QUIET)) { >>> + if (!(req->cmd_flags & REQ_QUIET) && >>> + printk_ratelimit()) { >>> if (description) >>> scmd_printk(KERN_INFO, cmd, "%s\n", >>> description); >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" >> in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH] scsi_lib: rate-limit the error message from failing commands 2012-03-07 13:17 ` Tomas Henzl @ 2012-03-07 14:55 ` Zou, Yi 0 siblings, 0 replies; 5+ messages in thread From: Zou, Yi @ 2012-03-07 14:55 UTC (permalink / raw) To: Tomas Henzl; +Cc: linux-scsi@vger.kernel.org, devel@open-fcoe.org > > On 03/06/2012 06:10 PM, Zou, Yi wrote: > >> On 03/05/2012 09:42 PM, Yi Zou wrote: > >>> When performing a cable pull test w/ active stress I/O using fio over > >>> a dual port Intel 82599 FCoE CNA, w/ 256LUNs on one port and about > >> 32LUNs > >>> on the other, it is observed that the system becomes not usable due > to > >>> scsi-ml being busy printing the error messages for all the failing > >> commands. > >>> I don't believe this problem is specific to FCoE and these commands > are > >>> anyway failing due to link being down (DID_NO_CONNECT), just rate- > limit > >>> the messages here to solve this issue. > >>> > >>> Signed-off-by: Yi Zou <yi.zou@intel.com> > >>> Cc: www.Open-FCoE.org <devel@open-fcoe.org> > >> Hi Zou, > >> you shouldn't use printk_ratelimit . > >> checkpatch says: WARNING: Prefer printk_ratelimited or > >> pr_<level>_ratelimited to printk_ratelimit > >> Tomas > > Yes, I saw that when I did the checkpatch, but pratek_ratelimit() > actually > > serves the purpose better, as it guards the block of the several scsi > related > > prints that follow, and this works well particularly for this bug. For > the worst > > case, the shared ratelimit surpresses everything here, blk i/o error > report > > would still be able pick up printing something. > > > > If it is desired, which I doubt, I could change all scmd_printk, > scsi_print_xxxx > > calls to use printk_ratelimited(), seems an overkill for this to me. I > am open > > for a better solution. > For a whole block of commands you can use __ratelimit, so something like > this > were possible: > > @@ -20,6 +20,7 @@ > #include <linux/delay.h> > #include <linux/hardirq.h> > #include <linux/scatterlist.h> > +#include <linux/ratelimit.h> > > #include <scsi/scsi.h> > #include <scsi/scsi_cmnd.h> > @@ -745,6 +746,8 @@ void scsi_io_completion(struct scsi_cmnd *cmd, > unsigned int good_bytes) > enum {ACTION_FAIL, ACTION_REPREP, ACTION_RETRY, > ACTION_DELAYED_RETRY} action; > char *description = NULL; > + static DEFINE_RATELIMIT_STATE(rs, DEFAULT_RATELIMIT_INTERVAL, > + DEFAULT_RATELIMIT_BURST); > > if (result) { > sense_valid = scsi_command_normalize_sense(cmd, &sshdr); > @@ -935,7 +938,7 @@ void scsi_io_completion(struct scsi_cmnd *cmd, > unsigned int good_bytes) > case ACTION_FAIL: > /* Give up and fail the remainder of the request */ > scsi_release_buffers(cmd); > - if (!(req->cmd_flags & REQ_QUIET)) { > + if (!(req->cmd_flags & REQ_QUIET) && __ratelimit(&rs)) { > if (description) > scmd_printk(KERN_INFO, cmd, "%s\n", > description);yi > This looks better, I will resend the patch, which will be pretty much the same what you have here. thanks. yi > > >>> --- > >>> > >>> drivers/scsi/scsi_lib.c | 3 ++- > >>> 1 files changed, 2 insertions(+), 1 deletions(-) > >>> > >>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c > >>> index b2c95db..e2128d9 100644 > >>> --- a/drivers/scsi/scsi_lib.c > >>> +++ b/drivers/scsi/scsi_lib.c > >>> @@ -934,7 +934,8 @@ void scsi_io_completion(struct scsi_cmnd *cmd, > >> unsigned int good_bytes) > >>> case ACTION_FAIL: > >>> /* Give up and fail the remainder of the request */ > >>> scsi_release_buffers(cmd); > >>> - if (!(req->cmd_flags & REQ_QUIET)) { > >>> + if (!(req->cmd_flags & REQ_QUIET) && > >>> + printk_ratelimit()) { > >>> if (description) > >>> scmd_printk(KERN_INFO, cmd, "%s\n", > >>> description); > >>> > >>> -- > >>> To unsubscribe from this list: send the line "unsubscribe linux-scsi" > >> in > >>> the body of a message to majordomo@vger.kernel.org > >>> More majordomo info at http://vger.kernel.org/majordomo-info.html > >> -- > >> To unsubscribe from this list: send the line "unsubscribe linux-scsi" > in > >> the body of a message to majordomo@vger.kernel.org > >> More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-03-07 14:55 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-03-05 20:42 [PATCH] scsi_lib: rate-limit the error message from failing commands Yi Zou 2012-03-06 15:40 ` Tomas Henzl 2012-03-06 17:10 ` Zou, Yi 2012-03-07 13:17 ` Tomas Henzl 2012-03-07 14:55 ` Zou, Yi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.