| Message ID | 20190523214939.30277-1-jpittman@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: print offending values when cloned rq limits are exceeded | expand |
I think it will be useful to print the information along with the error. Do we want to address the checkpatch warnings ? WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ... #20: FILE: block/blk-core.c:1202: + printk(KERN_ERR "%s: over max size limit. (%u > %u)\n", WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then dev_err(dev, ... then pr_err(... to printk(KERN_ERR ... #31: FILE: block/blk-core.c:1216: + printk(KERN_ERR "%s: over max segments limit. (%hu > %hu)\n", In either case, Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> . On 5/23/19 2:49 PM, John Pittman wrote: > While troubleshooting issues where cloned request limits have been > exceeded, it is often beneficial to know the actual values that > have been breached. Print these values, assisting in ease of > identification of root cause of the breach. > > Signed-off-by: John Pittman <jpittman@redhat.com> > --- > block/blk-core.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/block/blk-core.c b/block/blk-core.c > index 419d600e6637..af62150bb1ba 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -1199,7 +1199,9 @@ static int blk_cloned_rq_check_limits(struct request_queue *q, > struct request *rq) > { > if (blk_rq_sectors(rq) > blk_queue_get_max_sectors(q, req_op(rq))) { > - printk(KERN_ERR "%s: over max size limit.\n", __func__); > + printk(KERN_ERR "%s: over max size limit. (%u > %u)\n", > + __func__, blk_rq_sectors(rq), > + blk_queue_get_max_sectors(q, req_op(rq))); > return -EIO; > } > > @@ -1211,7 +1213,8 @@ static int blk_cloned_rq_check_limits(struct request_queue *q, > */ > blk_recalc_rq_segments(rq); > if (rq->nr_phys_segments > queue_max_segments(q)) { > - printk(KERN_ERR "%s: over max segments limit.\n", __func__); > + printk(KERN_ERR "%s: over max segments limit. (%hu > %hu)\n", > + __func__, rq->nr_phys_segments, queue_max_segments(q)); > return -EIO; > } > >
On Fri, May 24, 2019 at 5:50 AM John Pittman <jpittman@redhat.com> wrote: > > While troubleshooting issues where cloned request limits have been > exceeded, it is often beneficial to know the actual values that > have been breached. Print these values, assisting in ease of > identification of root cause of the breach. > > Signed-off-by: John Pittman <jpittman@redhat.com> > --- > block/blk-core.c | 7 +++++-- > 1 file changed, 5 insertions(+), 2 deletions(-) > > diff --git a/block/blk-core.c b/block/blk-core.c > index 419d600e6637..af62150bb1ba 100644 > --- a/block/blk-core.c > +++ b/block/blk-core.c > @@ -1199,7 +1199,9 @@ static int blk_cloned_rq_check_limits(struct request_queue *q, > struct request *rq) > { > if (blk_rq_sectors(rq) > blk_queue_get_max_sectors(q, req_op(rq))) { > - printk(KERN_ERR "%s: over max size limit.\n", __func__); > + printk(KERN_ERR "%s: over max size limit. (%u > %u)\n", > + __func__, blk_rq_sectors(rq), > + blk_queue_get_max_sectors(q, req_op(rq))); > return -EIO; > } > > @@ -1211,7 +1213,8 @@ static int blk_cloned_rq_check_limits(struct request_queue *q, > */ > blk_recalc_rq_segments(rq); > if (rq->nr_phys_segments > queue_max_segments(q)) { > - printk(KERN_ERR "%s: over max segments limit.\n", __func__); > + printk(KERN_ERR "%s: over max segments limit. (%hu > %hu)\n", > + __func__, rq->nr_phys_segments, queue_max_segments(q)); > return -EIO; > } Reviewed-by: Ming Lei <ming.lei@redhat.com> BTW, do you still see such kind of warning since v5.1? If yes, could you share something more? Thanks, Ming Lei
Hi Ming; thanks for reviewing. I've not seen anything in 5.x. Currently, we have a RHEL case for kernel 3.x where we've hit this error, but we've yet to identify root cause. As part of debugging, the techs involved are having to grab the values via systemtap (which the end-user is resisting). Please feel free to contact me off-list if you want the details on the case. If the patch is approved here, I was planning to see if you would pull it into the RHEL stream for 4.x so we would not have to mess w/ systemtap next time. Generally, in support, when we see this error it's from an admin or script setting bad stacking values. On Thu, May 23, 2019 at 9:58 PM Ming Lei <tom.leiming@gmail.com> wrote: > > On Fri, May 24, 2019 at 5:50 AM John Pittman <jpittman@redhat.com> wrote: > > > > While troubleshooting issues where cloned request limits have been > > exceeded, it is often beneficial to know the actual values that > > have been breached. Print these values, assisting in ease of > > identification of root cause of the breach. > > > > Signed-off-by: John Pittman <jpittman@redhat.com> > > --- > > block/blk-core.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/block/blk-core.c b/block/blk-core.c > > index 419d600e6637..af62150bb1ba 100644 > > --- a/block/blk-core.c > > +++ b/block/blk-core.c > > @@ -1199,7 +1199,9 @@ static int blk_cloned_rq_check_limits(struct request_queue *q, > > struct request *rq) > > { > > if (blk_rq_sectors(rq) > blk_queue_get_max_sectors(q, req_op(rq))) { > > - printk(KERN_ERR "%s: over max size limit.\n", __func__); > > + printk(KERN_ERR "%s: over max size limit. (%u > %u)\n", > > + __func__, blk_rq_sectors(rq), > > + blk_queue_get_max_sectors(q, req_op(rq))); > > return -EIO; > > } > > > > @@ -1211,7 +1213,8 @@ static int blk_cloned_rq_check_limits(struct request_queue *q, > > */ > > blk_recalc_rq_segments(rq); > > if (rq->nr_phys_segments > queue_max_segments(q)) { > > - printk(KERN_ERR "%s: over max segments limit.\n", __func__); > > + printk(KERN_ERR "%s: over max segments limit. (%hu > %hu)\n", > > + __func__, rq->nr_phys_segments, queue_max_segments(q)); > > return -EIO; > > } > > Reviewed-by: Ming Lei <ming.lei@redhat.com> > > BTW, do you still see such kind of warning since v5.1? If yes, could > you share something > more? > > Thanks, > Ming Lei
Thanks Chaitanya for the review. I was not sure what Jens would think about the checkpatch warning, so I left it as it was so he could decide. I tried to model the value output after that old 'bio too big' error. Thanks again. On Thu, May 23, 2019 at 9:17 PM Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com> wrote: > > I think it will be useful to print the information along with the error. > > Do we want to address the checkpatch warnings ? > > WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then > dev_err(dev, ... then pr_err(... to printk(KERN_ERR ... > #20: FILE: block/blk-core.c:1202: > + printk(KERN_ERR "%s: over max size limit. (%u > %u)\n", > > WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then > dev_err(dev, ... then pr_err(... to printk(KERN_ERR ... > #31: FILE: block/blk-core.c:1216: > + printk(KERN_ERR "%s: over max segments limit. (%hu > %hu)\n", > > In either case, > > Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> . > > On 5/23/19 2:49 PM, John Pittman wrote: > > While troubleshooting issues where cloned request limits have been > > exceeded, it is often beneficial to know the actual values that > > have been breached. Print these values, assisting in ease of > > identification of root cause of the breach. > > > > Signed-off-by: John Pittman <jpittman@redhat.com> > > --- > > block/blk-core.c | 7 +++++-- > > 1 file changed, 5 insertions(+), 2 deletions(-) > > > > diff --git a/block/blk-core.c b/block/blk-core.c > > index 419d600e6637..af62150bb1ba 100644 > > --- a/block/blk-core.c > > +++ b/block/blk-core.c > > @@ -1199,7 +1199,9 @@ static int blk_cloned_rq_check_limits(struct request_queue *q, > > struct request *rq) > > { > > if (blk_rq_sectors(rq) > blk_queue_get_max_sectors(q, req_op(rq))) { > > - printk(KERN_ERR "%s: over max size limit.\n", __func__); > > + printk(KERN_ERR "%s: over max size limit. (%u > %u)\n", > > + __func__, blk_rq_sectors(rq), > > + blk_queue_get_max_sectors(q, req_op(rq))); > > return -EIO; > > } > > > > @@ -1211,7 +1213,8 @@ static int blk_cloned_rq_check_limits(struct request_queue *q, > > */ > > blk_recalc_rq_segments(rq); > > if (rq->nr_phys_segments > queue_max_segments(q)) { > > - printk(KERN_ERR "%s: over max segments limit.\n", __func__); > > + printk(KERN_ERR "%s: over max segments limit. (%hu > %hu)\n", > > + __func__, rq->nr_phys_segments, queue_max_segments(q)); > > return -EIO; > > } > > > > >
Let's leave it to Jens. On 5/24/19 5:19 AM, John Pittman wrote: > Thanks Chaitanya for the review. I was not sure what Jens would think > about the checkpatch warning, so I left it as it was so he could > decide. I tried to model the value output after that old 'bio too > big' error. Thanks again. > > On Thu, May 23, 2019 at 9:17 PM Chaitanya Kulkarni > <Chaitanya.Kulkarni@wdc.com> wrote: >> >> I think it will be useful to print the information along with the error. >> >> Do we want to address the checkpatch warnings ? >> >> WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then >> dev_err(dev, ... then pr_err(... to printk(KERN_ERR ... >> #20: FILE: block/blk-core.c:1202: >> + printk(KERN_ERR "%s: over max size limit. (%u > %u)\n", >> >> WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then >> dev_err(dev, ... then pr_err(... to printk(KERN_ERR ... >> #31: FILE: block/blk-core.c:1216: >> + printk(KERN_ERR "%s: over max segments limit. (%hu > %hu)\n", >> >> In either case, >> >> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> . >> >> On 5/23/19 2:49 PM, John Pittman wrote: >>> While troubleshooting issues where cloned request limits have been >>> exceeded, it is often beneficial to know the actual values that >>> have been breached. Print these values, assisting in ease of >>> identification of root cause of the breach. >>> >>> Signed-off-by: John Pittman <jpittman@redhat.com> >>> --- >>> block/blk-core.c | 7 +++++-- >>> 1 file changed, 5 insertions(+), 2 deletions(-) >>> >>> diff --git a/block/blk-core.c b/block/blk-core.c >>> index 419d600e6637..af62150bb1ba 100644 >>> --- a/block/blk-core.c >>> +++ b/block/blk-core.c >>> @@ -1199,7 +1199,9 @@ static int blk_cloned_rq_check_limits(struct request_queue *q, >>> struct request *rq) >>> { >>> if (blk_rq_sectors(rq) > blk_queue_get_max_sectors(q, req_op(rq))) { >>> - printk(KERN_ERR "%s: over max size limit.\n", __func__); >>> + printk(KERN_ERR "%s: over max size limit. (%u > %u)\n", >>> + __func__, blk_rq_sectors(rq), >>> + blk_queue_get_max_sectors(q, req_op(rq))); >>> return -EIO; >>> } >>> >>> @@ -1211,7 +1213,8 @@ static int blk_cloned_rq_check_limits(struct request_queue *q, >>> */ >>> blk_recalc_rq_segments(rq); >>> if (rq->nr_phys_segments > queue_max_segments(q)) { >>> - printk(KERN_ERR "%s: over max segments limit.\n", __func__); >>> + printk(KERN_ERR "%s: over max segments limit. (%hu > %hu)\n", >>> + __func__, rq->nr_phys_segments, queue_max_segments(q)); >>> return -EIO; >>> } >>> >>> >> >
Hi Jens. Does this patch seem reasonable? On Fri, May 24, 2019 at 12:51 PM Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com> wrote: > > Let's leave it to Jens. > > On 5/24/19 5:19 AM, John Pittman wrote: > > Thanks Chaitanya for the review. I was not sure what Jens would think > > about the checkpatch warning, so I left it as it was so he could > > decide. I tried to model the value output after that old 'bio too > > big' error. Thanks again. > > > > On Thu, May 23, 2019 at 9:17 PM Chaitanya Kulkarni > > <Chaitanya.Kulkarni@wdc.com> wrote: > >> > >> I think it will be useful to print the information along with the error. > >> > >> Do we want to address the checkpatch warnings ? > >> > >> WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then > >> dev_err(dev, ... then pr_err(... to printk(KERN_ERR ... > >> #20: FILE: block/blk-core.c:1202: > >> + printk(KERN_ERR "%s: over max size limit. (%u > %u)\n", > >> > >> WARNING: Prefer [subsystem eg: netdev]_err([subsystem]dev, ... then > >> dev_err(dev, ... then pr_err(... to printk(KERN_ERR ... > >> #31: FILE: block/blk-core.c:1216: > >> + printk(KERN_ERR "%s: over max segments limit. (%hu > %hu)\n", > >> > >> In either case, > >> > >> Reviewed-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> . > >> > >> On 5/23/19 2:49 PM, John Pittman wrote: > >>> While troubleshooting issues where cloned request limits have been > >>> exceeded, it is often beneficial to know the actual values that > >>> have been breached. Print these values, assisting in ease of > >>> identification of root cause of the breach. > >>> > >>> Signed-off-by: John Pittman <jpittman@redhat.com> > >>> --- > >>> block/blk-core.c | 7 +++++-- > >>> 1 file changed, 5 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/block/blk-core.c b/block/blk-core.c > >>> index 419d600e6637..af62150bb1ba 100644 > >>> --- a/block/blk-core.c > >>> +++ b/block/blk-core.c > >>> @@ -1199,7 +1199,9 @@ static int blk_cloned_rq_check_limits(struct request_queue *q, > >>> struct request *rq) > >>> { > >>> if (blk_rq_sectors(rq) > blk_queue_get_max_sectors(q, req_op(rq))) { > >>> - printk(KERN_ERR "%s: over max size limit.\n", __func__); > >>> + printk(KERN_ERR "%s: over max size limit. (%u > %u)\n", > >>> + __func__, blk_rq_sectors(rq), > >>> + blk_queue_get_max_sectors(q, req_op(rq))); > >>> return -EIO; > >>> } > >>> > >>> @@ -1211,7 +1213,8 @@ static int blk_cloned_rq_check_limits(struct request_queue *q, > >>> */ > >>> blk_recalc_rq_segments(rq); > >>> if (rq->nr_phys_segments > queue_max_segments(q)) { > >>> - printk(KERN_ERR "%s: over max segments limit.\n", __func__); > >>> + printk(KERN_ERR "%s: over max segments limit. (%hu > %hu)\n", > >>> + __func__, rq->nr_phys_segments, queue_max_segments(q)); > >>> return -EIO; > >>> } > >>> > >>> > >> > > >
On 5/31/19 11:34 AM, John Pittman wrote:
> Hi Jens. Does this patch seem reasonable?
Looks good to me, applied.
diff --git a/block/blk-core.c b/block/blk-core.c index 419d600e6637..af62150bb1ba 100644 --- a/block/blk-core.c +++ b/block/blk-core.c @@ -1199,7 +1199,9 @@ static int blk_cloned_rq_check_limits(struct request_queue *q, struct request *rq) { if (blk_rq_sectors(rq) > blk_queue_get_max_sectors(q, req_op(rq))) { - printk(KERN_ERR "%s: over max size limit.\n", __func__); + printk(KERN_ERR "%s: over max size limit. (%u > %u)\n", + __func__, blk_rq_sectors(rq), + blk_queue_get_max_sectors(q, req_op(rq))); return -EIO; } @@ -1211,7 +1213,8 @@ static int blk_cloned_rq_check_limits(struct request_queue *q, */ blk_recalc_rq_segments(rq); if (rq->nr_phys_segments > queue_max_segments(q)) { - printk(KERN_ERR "%s: over max segments limit.\n", __func__); + printk(KERN_ERR "%s: over max segments limit. (%hu > %hu)\n", + __func__, rq->nr_phys_segments, queue_max_segments(q)); return -EIO; }
While troubleshooting issues where cloned request limits have been exceeded, it is often beneficial to know the actual values that have been breached. Print these values, assisting in ease of identification of root cause of the breach. Signed-off-by: John Pittman <jpittman@redhat.com> --- block/blk-core.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)