Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1268191 > unrolled thread
| Started by | Matias Bjørling <m@bjorling.me> |
|---|---|
| First post | 2015-11-12 19:40 +0100 |
| Last post | 2015-11-12 20:10 +0100 |
| Articles | 5 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH] lightnvm: change max_phys_sect to ushort Matias Bjørling <m@bjorling.me> - 2015-11-12 19:40 +0100
Re: [PATCH] lightnvm: change max_phys_sect to ushort Geert Uytterhoeven <geert@linux-m68k.org> - 2015-11-12 19:50 +0100
Re: [PATCH] lightnvm: change max_phys_sect to ushort Matias Bjørling <m@bjorling.me> - 2015-11-12 20:00 +0100
Re: [PATCH] lightnvm: change max_phys_sect to ushort Matias Bjørling <m@bjorling.me> - 2015-11-12 20:10 +0100
Re: [PATCH] lightnvm: change max_phys_sect to ushort Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-12 20:10 +0100
| From | Matias Bjørling <m@bjorling.me> |
|---|---|
| Date | 2015-11-12 19:40 +0100 |
| Subject | [PATCH] lightnvm: change max_phys_sect to ushort |
| Message-ID | <qu4WT-42L-41@gated-at.bofh.it> |
The max_phys_sect variable is defined as a char. We do a boundary check
to maximally allow 256 physical page descriptors per command. As we are
not indexing from zero. This expression is always in false. Bump the
max_phys_sect to an unsigned short to support the range check.
Signed-off-by: Matias Bjørling <m@bjorling.me>
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
---
include/linux/lightnvm.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
index 69c9057..4b1cd3d 100644
--- a/include/linux/lightnvm.h
+++ b/include/linux/lightnvm.h
@@ -220,7 +220,7 @@ struct nvm_dev_ops {
nvm_dev_dma_alloc_fn *dev_dma_alloc;
nvm_dev_dma_free_fn *dev_dma_free;
- uint8_t max_phys_sect;
+ unsigned int max_phys_sect;
};
struct nvm_lun {
--
2.1.4
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [next] | [standalone]
| From | Geert Uytterhoeven <geert@linux-m68k.org> |
|---|---|
| Date | 2015-11-12 19:50 +0100 |
| Message-ID | <qu56y-46h-17@gated-at.bofh.it> |
| In reply to | #1268191 |
On Thu, Nov 12, 2015 at 7:35 PM, Matias Bjørling <m@bjorling.me> wrote:
> The max_phys_sect variable is defined as a char. We do a boundary check
> to maximally allow 256 physical page descriptors per command. As we are
> not indexing from zero. This expression is always in false. Bump the
> max_phys_sect to an unsigned short to support the range check.
unsigned int?
>
> Signed-off-by: Matias Bjørling <m@bjorling.me>
> Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> include/linux/lightnvm.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h
> index 69c9057..4b1cd3d 100644
> --- a/include/linux/lightnvm.h
> +++ b/include/linux/lightnvm.h
> @@ -220,7 +220,7 @@ struct nvm_dev_ops {
> nvm_dev_dma_alloc_fn *dev_dma_alloc;
> nvm_dev_dma_free_fn *dev_dma_free;
>
> - uint8_t max_phys_sect;
> + unsigned int max_phys_sect;
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Matias Bjørling <m@bjorling.me> |
|---|---|
| Date | 2015-11-12 20:00 +0100 |
| Message-ID | <qu5ge-49Z-5@gated-at.bofh.it> |
| In reply to | #1268199 |
On 11/12/2015 07:42 PM, Geert Uytterhoeven wrote: > On Thu, Nov 12, 2015 at 7:35 PM, Matias Bjørling <m@bjorling.me> wrote: >> The max_phys_sect variable is defined as a char. We do a boundary check >> to maximally allow 256 physical page descriptors per command. As we are >> not indexing from zero. This expression is always in false. Bump the >> max_phys_sect to an unsigned short to support the range check. > > unsigned int? Grah, I need to be more careful. I sent the wrong patch after I had fixed it to unsigned short. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Matias Bjørling <m@bjorling.me> |
|---|---|
| Date | 2015-11-12 20:10 +0100 |
| Message-ID | <qu5pU-4sN-11@gated-at.bofh.it> |
| In reply to | #1268204 |
On 11/12/2015 08:02 PM, Linus Torvalds wrote: > On Thu, Nov 12, 2015 at 10:57 AM, Matias Bjørling <m@bjorling.me> wrote: >> >> Grah, I need to be more careful. I sent the wrong patch after I had fixed it >> to unsigned short. > > Actually, I think "unsigned int" was better. > > You're not saving any space with "unsigned short" (the size of the > structure will be rounded up to the alignment of it anyway), and we > should generally strive to avoid 16-bit accesses unless there is some > real reason for them, because they are often slower than either "char" > or "int". Several architectures have weak support for 16-bit accesses > (eg alpha), and even on x86 you end up having operand size overrides > etc. > Thanks > So unless there is a clear *reason* to use "short" - just don't. > > Linus > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2015-11-12 20:10 +0100 |
| Message-ID | <qu5pU-4sN-7@gated-at.bofh.it> |
| In reply to | #1268204 |
On Thu, Nov 12, 2015 at 10:57 AM, Matias Bjørling <m@bjorling.me> wrote:
>
> Grah, I need to be more careful. I sent the wrong patch after I had fixed it
> to unsigned short.
Actually, I think "unsigned int" was better.
You're not saving any space with "unsigned short" (the size of the
structure will be rounded up to the alignment of it anyway), and we
should generally strive to avoid 16-bit accesses unless there is some
real reason for them, because they are often slower than either "char"
or "int". Several architectures have weak support for 16-bit accesses
(eg alpha), and even on x86 you end up having operand size overrides
etc.
So unless there is a clear *reason* to use "short" - just don't.
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web