diff mbox series

[SRU,Z,1/1] nvme: update timeout module parameter type

Message ID 20171116053614.2136-2-daniel.axtens@canonical.com
State New
Headers show
Series LP#1729119: NVMe timeout parameter type change | expand

Commit Message

Daniel Axtens Nov. 16, 2017, 5:36 a.m. UTC 
From: Marc Olson <marcolso@amazon.com>

BugLink: https://bugs.launchpad.net/bugs/1729119

The underlying blk_mq_tag_set, and request timeout parameters support an
unsigned int. Extend the size of the nvme module parameters for io and
admin commands to match.

Signed-off-by: Marc Olson <marcolso@amazon.com>
Signed-off-by: Christoph Hellwig <hch@lst.de>
(cherry picked from commit 8ae4e4477d8f5cc7736816a0492f82934ca32ab7 linux-next)
Signed-off-by: Daniel Axtens <daniel.axtens@canonical.com>
---
 drivers/nvme/host/core.c | 8 ++++----
 drivers/nvme/host/nvme.h | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

Comments

Stefan Bader Nov. 17, 2017, 2:01 p.m. UTC | #1 
On 16.11.2017 06:36, Daniel Axtens wrote:
> From: Marc Olson <marcolso@amazon.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1729119
> 
> The underlying blk_mq_tag_set, and request timeout parameters support an
> unsigned int. Extend the size of the nvme module parameters for io and
> admin commands to match.
> 
> Signed-off-by: Marc Olson <marcolso@amazon.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> (cherry picked from commit 8ae4e4477d8f5cc7736816a0492f82934ca32ab7 linux-next)
> Signed-off-by: Daniel Axtens <daniel.axtens@canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>

> ---

As the Zesty and Artful change should be identical (both claim to be
cherry-picks, I would prefer to see it submitted only once ([SRU Z/A]). So it
needs only one pass of looking at and acking.

Thanks,
Stefan

>  drivers/nvme/host/core.c | 8 ++++----
>  drivers/nvme/host/nvme.h | 4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index c50e8cf92c8b..4b2b77f64033 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -35,13 +35,13 @@
>  
>  #define NVME_MINORS		(1U << MINORBITS)
>  
> -unsigned char admin_timeout = 60;
> -module_param(admin_timeout, byte, 0644);
> +unsigned int admin_timeout = 60;
> +module_param(admin_timeout, uint, 0644);
>  MODULE_PARM_DESC(admin_timeout, "timeout in seconds for admin commands");
>  EXPORT_SYMBOL_GPL(admin_timeout);
>  
> -unsigned char nvme_io_timeout = 30;
> -module_param_named(io_timeout, nvme_io_timeout, byte, 0644);
> +unsigned int nvme_io_timeout = 30;
> +module_param_named(io_timeout, nvme_io_timeout, uint, 0644);
>  MODULE_PARM_DESC(io_timeout, "timeout in seconds for I/O");
>  EXPORT_SYMBOL_GPL(nvme_io_timeout);
>  
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 7596ae072b5c..e3b3bb638fa3 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -30,10 +30,10 @@ enum {
>  	NVME_SC_CANCELLED		= -EINTR,
>  };
>  
> -extern unsigned char nvme_io_timeout;
> +extern unsigned int nvme_io_timeout;
>  #define NVME_IO_TIMEOUT	(nvme_io_timeout * HZ)
>  
> -extern unsigned char admin_timeout;
> +extern unsigned int admin_timeout;
>  #define ADMIN_TIMEOUT	(admin_timeout * HZ)
>  
>  extern unsigned char shutdown_timeout;
>
Daniel Axtens Nov. 17, 2017, 2:04 p.m. UTC | #2 
Hi Stefan,

I did consider that, but I found that although the cherry-pick succeeds in
both cases, the patch generated for Zesty wouldn't apply to Artful and vice
versa as the context had changed. Is that something the kernel team is
happy to sort out yourselves, or do you still want 2 patches in this case?

Regards,
Daniel

On Sat, Nov 18, 2017 at 1:01 AM, Stefan Bader <stefan.bader@canonical.com>
wrote:

> On 16.11.2017 06:36, Daniel Axtens wrote:
> > From: Marc Olson <marcolso@amazon.com>
> >
> > BugLink: https://bugs.launchpad.net/bugs/1729119
> >
> > The underlying blk_mq_tag_set, and request timeout parameters support an
> > unsigned int. Extend the size of the nvme module parameters for io and
> > admin commands to match.
> >
> > Signed-off-by: Marc Olson <marcolso@amazon.com>
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > (cherry picked from commit 8ae4e4477d8f5cc7736816a0492f82934ca32ab7
> linux-next)
> > Signed-off-by: Daniel Axtens <daniel.axtens@canonical.com>
> Acked-by: Stefan Bader <stefan.bader@canonical.com>
>
> > ---
>
> As the Zesty and Artful change should be identical (both claim to be
> cherry-picks, I would prefer to see it submitted only once ([SRU Z/A]). So
> it
> needs only one pass of looking at and acking.
>
> Thanks,
> Stefan
>
> >  drivers/nvme/host/core.c | 8 ++++----
> >  drivers/nvme/host/nvme.h | 4 ++--
> >  2 files changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> > index c50e8cf92c8b..4b2b77f64033 100644
> > --- a/drivers/nvme/host/core.c
> > +++ b/drivers/nvme/host/core.c
> > @@ -35,13 +35,13 @@
> >
> >  #define NVME_MINORS          (1U << MINORBITS)
> >
> > -unsigned char admin_timeout = 60;
> > -module_param(admin_timeout, byte, 0644);
> > +unsigned int admin_timeout = 60;
> > +module_param(admin_timeout, uint, 0644);
> >  MODULE_PARM_DESC(admin_timeout, "timeout in seconds for admin
> commands");
> >  EXPORT_SYMBOL_GPL(admin_timeout);
> >
> > -unsigned char nvme_io_timeout = 30;
> > -module_param_named(io_timeout, nvme_io_timeout, byte, 0644);
> > +unsigned int nvme_io_timeout = 30;
> > +module_param_named(io_timeout, nvme_io_timeout, uint, 0644);
> >  MODULE_PARM_DESC(io_timeout, "timeout in seconds for I/O");
> >  EXPORT_SYMBOL_GPL(nvme_io_timeout);
> >
> > diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> > index 7596ae072b5c..e3b3bb638fa3 100644
> > --- a/drivers/nvme/host/nvme.h
> > +++ b/drivers/nvme/host/nvme.h
> > @@ -30,10 +30,10 @@ enum {
> >       NVME_SC_CANCELLED               = -EINTR,
> >  };
> >
> > -extern unsigned char nvme_io_timeout;
> > +extern unsigned int nvme_io_timeout;
> >  #define NVME_IO_TIMEOUT      (nvme_io_timeout * HZ)
> >
> > -extern unsigned char admin_timeout;
> > +extern unsigned int admin_timeout;
> >  #define ADMIN_TIMEOUT        (admin_timeout * HZ)
> >
> >  extern unsigned char shutdown_timeout;
> >
>
>
>
<div dir="ltr">Hi Stefan,<div><br></div><div>I did consider that, but I found that although the cherry-pick succeeds in both cases, the patch generated for Zesty wouldn&#39;t apply to Artful and vice versa as the context had changed. Is that something the kernel team is happy to sort out yourselves, or do you still want 2 patches in this case?</div><div><br></div><div>Regards,</div><div>Daniel</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, Nov 18, 2017 at 1:01 AM, Stefan Bader <span dir="ltr">&lt;<a href="mailto:stefan.bader@canonical.com" target="_blank">stefan.bader@canonical.com</a>&gt;</span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 16.11.2017 06:36, Daniel Axtens wrote:<br>
&gt; From: Marc Olson &lt;<a href="mailto:marcolso@amazon.com">marcolso@amazon.com</a>&gt;<br>
&gt;<br>
&gt; BugLink: <a href="https://bugs.launchpad.net/bugs/1729119" rel="noreferrer" target="_blank">https://bugs.launchpad.net/<wbr>bugs/1729119</a><br>
&gt;<br>
&gt; The underlying blk_mq_tag_set, and request timeout parameters support an<br>
&gt; unsigned int. Extend the size of the nvme module parameters for io and<br>
&gt; admin commands to match.<br>
&gt;<br>
&gt; Signed-off-by: Marc Olson &lt;<a href="mailto:marcolso@amazon.com">marcolso@amazon.com</a>&gt;<br>
&gt; Signed-off-by: Christoph Hellwig &lt;<a href="mailto:hch@lst.de">hch@lst.de</a>&gt;<br>
&gt; (cherry picked from commit 8ae4e4477d8f5cc7736816a0492f82<wbr>934ca32ab7 linux-next)<br>
&gt; Signed-off-by: Daniel Axtens &lt;<a href="mailto:daniel.axtens@canonical.com">daniel.axtens@canonical.com</a>&gt;<br>
Acked-by: Stefan Bader &lt;<a href="mailto:stefan.bader@canonical.com">stefan.bader@canonical.com</a>&gt;<br>
<br>
&gt; ---<br>
<br>
As the Zesty and Artful change should be identical (both claim to be<br>
cherry-picks, I would prefer to see it submitted only once ([SRU Z/A]). So it<br>
needs only one pass of looking at and acking.<br>
<br>
Thanks,<br>
Stefan<br>
<br>
&gt;  drivers/nvme/host/core.c | 8 ++++----<br>
&gt;  drivers/nvme/host/nvme.h | 4 ++--<br>
&gt;  2 files changed, 6 insertions(+), 6 deletions(-)<br>
&gt;<br>
&gt; diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c<br>
&gt; index c50e8cf92c8b..4b2b77f64033 100644<br>
&gt; --- a/drivers/nvme/host/core.c<br>
&gt; +++ b/drivers/nvme/host/core.c<br>
&gt; @@ -35,13 +35,13 @@<br>
&gt;<br>
&gt;  #define NVME_MINORS          (1U &lt;&lt; MINORBITS)<br>
&gt;<br>
&gt; -unsigned char admin_timeout = 60;<br>
&gt; -module_param(admin_timeout, byte, 0644);<br>
&gt; +unsigned int admin_timeout = 60;<br>
&gt; +module_param(admin_timeout, uint, 0644);<br>
&gt;  MODULE_PARM_DESC(admin_<wbr>timeout, &quot;timeout in seconds for admin commands&quot;);<br>
&gt;  EXPORT_SYMBOL_GPL(admin_<wbr>timeout);<br>
&gt;<br>
&gt; -unsigned char nvme_io_timeout = 30;<br>
&gt; -module_param_named(io_<wbr>timeout, nvme_io_timeout, byte, 0644);<br>
&gt; +unsigned int nvme_io_timeout = 30;<br>
&gt; +module_param_named(io_<wbr>timeout, nvme_io_timeout, uint, 0644);<br>
&gt;  MODULE_PARM_DESC(io_timeout, &quot;timeout in seconds for I/O&quot;);<br>
&gt;  EXPORT_SYMBOL_GPL(nvme_io_<wbr>timeout);<br>
&gt;<br>
&gt; diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h<br>
&gt; index 7596ae072b5c..e3b3bb638fa3 100644<br>
&gt; --- a/drivers/nvme/host/nvme.h<br>
&gt; +++ b/drivers/nvme/host/nvme.h<br>
&gt; @@ -30,10 +30,10 @@ enum {<br>
&gt;       NVME_SC_CANCELLED               = -EINTR,<br>
&gt;  };<br>
&gt;<br>
&gt; -extern unsigned char nvme_io_timeout;<br>
&gt; +extern unsigned int nvme_io_timeout;<br>
&gt;  #define NVME_IO_TIMEOUT      (nvme_io_timeout * HZ)<br>
&gt;<br>
&gt; -extern unsigned char admin_timeout;<br>
&gt; +extern unsigned int admin_timeout;<br>
&gt;  #define ADMIN_TIMEOUT        (admin_timeout * HZ)<br>
&gt;<br>
&gt;  extern unsigned char shutdown_timeout;<br>
&gt;<br>
<br>
<br>
</blockquote></div><br></div>
Stefan Bader Nov. 17, 2017, 2:41 p.m. UTC | #3 
On 17.11.2017 15:04, Daniel Axtens wrote:
> Hi Stefan,
> 
> I did consider that, but I found that although the cherry-pick succeeds in both
> cases, the patch generated for Zesty wouldn't apply to Artful and vice versa as
> the context had changed. Is that something the kernel team is happy to sort out
> yourselves, or do you still want 2 patches in this case?

What I would send around is the patch generated by "git format-patch ..." out of
the upstream repo. If that does not apply somewhere, its no longer a
cherry-pick. There are cases (fuzz 1) which can be avoided by doing the git am
with -C2. Which then could be noted in the cover letter.

-Stefan

> 
> Regards,
> Daniel
> 
> On Sat, Nov 18, 2017 at 1:01 AM, Stefan Bader <stefan.bader@canonical.com
> <mailto:stefan.bader@canonical.com>> wrote:
> 
>     On 16.11.2017 06:36, Daniel Axtens wrote:
>     > From: Marc Olson <marcolso@amazon.com <mailto:marcolso@amazon.com>>
>     >
>     > BugLink: https://bugs.launchpad.net/bugs/1729119
>     <https://bugs.launchpad.net/bugs/1729119>
>     >
>     > The underlying blk_mq_tag_set, and request timeout parameters support an
>     > unsigned int. Extend the size of the nvme module parameters for io and
>     > admin commands to match.
>     >
>     > Signed-off-by: Marc Olson <marcolso@amazon.com <mailto:marcolso@amazon.com>>
>     > Signed-off-by: Christoph Hellwig <hch@lst.de <mailto:hch@lst.de>>
>     > (cherry picked from commit 8ae4e4477d8f5cc7736816a0492f82934ca32ab7
>     linux-next)
>     > Signed-off-by: Daniel Axtens <daniel.axtens@canonical.com
>     <mailto:daniel.axtens@canonical.com>>
>     Acked-by: Stefan Bader <stefan.bader@canonical.com
>     <mailto:stefan.bader@canonical.com>>
> 
>     > ---
> 
>     As the Zesty and Artful change should be identical (both claim to be
>     cherry-picks, I would prefer to see it submitted only once ([SRU Z/A]). So it
>     needs only one pass of looking at and acking.
> 
>     Thanks,
>     Stefan
> 
>     >  drivers/nvme/host/core.c | 8 ++++----
>     >  drivers/nvme/host/nvme.h | 4 ++--
>     >  2 files changed, 6 insertions(+), 6 deletions(-)
>     >
>     > diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
>     > index c50e8cf92c8b..4b2b77f64033 100644
>     > --- a/drivers/nvme/host/core.c
>     > +++ b/drivers/nvme/host/core.c
>     > @@ -35,13 +35,13 @@
>     >
>     >  #define NVME_MINORS          (1U << MINORBITS)
>     >
>     > -unsigned char admin_timeout = 60;
>     > -module_param(admin_timeout, byte, 0644);
>     > +unsigned int admin_timeout = 60;
>     > +module_param(admin_timeout, uint, 0644);
>     >  MODULE_PARM_DESC(admin_timeout, "timeout in seconds for admin commands");
>     >  EXPORT_SYMBOL_GPL(admin_timeout);
>     >
>     > -unsigned char nvme_io_timeout = 30;
>     > -module_param_named(io_timeout, nvme_io_timeout, byte, 0644);
>     > +unsigned int nvme_io_timeout = 30;
>     > +module_param_named(io_timeout, nvme_io_timeout, uint, 0644);
>     >  MODULE_PARM_DESC(io_timeout, "timeout in seconds for I/O");
>     >  EXPORT_SYMBOL_GPL(nvme_io_timeout);
>     >
>     > diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
>     > index 7596ae072b5c..e3b3bb638fa3 100644
>     > --- a/drivers/nvme/host/nvme.h
>     > +++ b/drivers/nvme/host/nvme.h
>     > @@ -30,10 +30,10 @@ enum {
>     >       NVME_SC_CANCELLED               = -EINTR,
>     >  };
>     >
>     > -extern unsigned char nvme_io_timeout;
>     > +extern unsigned int nvme_io_timeout;
>     >  #define NVME_IO_TIMEOUT      (nvme_io_timeout * HZ)
>     >
>     > -extern unsigned char admin_timeout;
>     > +extern unsigned int admin_timeout;
>     >  #define ADMIN_TIMEOUT        (admin_timeout * HZ)
>     >
>     >  extern unsigned char shutdown_timeout;
>     >
> 
> 
>
Colin Ian King Nov. 20, 2017, 11:36 a.m. UTC | #4 
On 16/11/17 05:36, Daniel Axtens wrote:
> From: Marc Olson <marcolso@amazon.com>
> 
> BugLink: https://bugs.launchpad.net/bugs/1729119
> 
> The underlying blk_mq_tag_set, and request timeout parameters support an
> unsigned int. Extend the size of the nvme module parameters for io and
> admin commands to match.
> 
> Signed-off-by: Marc Olson <marcolso@amazon.com>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> (cherry picked from commit 8ae4e4477d8f5cc7736816a0492f82934ca32ab7 linux-next)
> Signed-off-by: Daniel Axtens <daniel.axtens@canonical.com>
> ---
>  drivers/nvme/host/core.c | 8 ++++----
>  drivers/nvme/host/nvme.h | 4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index c50e8cf92c8b..4b2b77f64033 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -35,13 +35,13 @@
>  
>  #define NVME_MINORS		(1U << MINORBITS)
>  
> -unsigned char admin_timeout = 60;
> -module_param(admin_timeout, byte, 0644);
> +unsigned int admin_timeout = 60;
> +module_param(admin_timeout, uint, 0644);
>  MODULE_PARM_DESC(admin_timeout, "timeout in seconds for admin commands");
>  EXPORT_SYMBOL_GPL(admin_timeout);
>  
> -unsigned char nvme_io_timeout = 30;
> -module_param_named(io_timeout, nvme_io_timeout, byte, 0644);
> +unsigned int nvme_io_timeout = 30;
> +module_param_named(io_timeout, nvme_io_timeout, uint, 0644);
>  MODULE_PARM_DESC(io_timeout, "timeout in seconds for I/O");
>  EXPORT_SYMBOL_GPL(nvme_io_timeout);
>  
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 7596ae072b5c..e3b3bb638fa3 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -30,10 +30,10 @@ enum {
>  	NVME_SC_CANCELLED		= -EINTR,
>  };
>  
> -extern unsigned char nvme_io_timeout;
> +extern unsigned int nvme_io_timeout;
>  #define NVME_IO_TIMEOUT	(nvme_io_timeout * HZ)
>  
> -extern unsigned char admin_timeout;
> +extern unsigned int admin_timeout;
>  #define ADMIN_TIMEOUT	(admin_timeout * HZ)
>  
>  extern unsigned char shutdown_timeout;
> 

Acked-by: Colin Ian King <colin.king@canonical.com>
diff mbox series

Patch

diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
index c50e8cf92c8b..4b2b77f64033 100644
--- a/drivers/nvme/host/core.c
+++ b/drivers/nvme/host/core.c
@@ -35,13 +35,13 @@ 
 
 #define NVME_MINORS		(1U << MINORBITS)
 
-unsigned char admin_timeout = 60;
-module_param(admin_timeout, byte, 0644);
+unsigned int admin_timeout = 60;
+module_param(admin_timeout, uint, 0644);
 MODULE_PARM_DESC(admin_timeout, "timeout in seconds for admin commands");
 EXPORT_SYMBOL_GPL(admin_timeout);
 
-unsigned char nvme_io_timeout = 30;
-module_param_named(io_timeout, nvme_io_timeout, byte, 0644);
+unsigned int nvme_io_timeout = 30;
+module_param_named(io_timeout, nvme_io_timeout, uint, 0644);
 MODULE_PARM_DESC(io_timeout, "timeout in seconds for I/O");
 EXPORT_SYMBOL_GPL(nvme_io_timeout);
 
diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
index 7596ae072b5c..e3b3bb638fa3 100644
--- a/drivers/nvme/host/nvme.h
+++ b/drivers/nvme/host/nvme.h
@@ -30,10 +30,10 @@  enum {
 	NVME_SC_CANCELLED		= -EINTR,
 };
 
-extern unsigned char nvme_io_timeout;
+extern unsigned int nvme_io_timeout;
 #define NVME_IO_TIMEOUT	(nvme_io_timeout * HZ)
 
-extern unsigned char admin_timeout;
+extern unsigned int admin_timeout;
 #define ADMIN_TIMEOUT	(admin_timeout * HZ)
 
 extern unsigned char shutdown_timeout;