[PATCH 1/3] shmem: fix division by zero

[PATCH 1/3] shmem: fix division by zero

[Hugh Dickins]

From: Yuri Tikhonov <yur@emcraft.com>

Fix a division by zero which we have in shmem_truncate_range() and
shmem_unuse_inode() when using big PAGE_SIZE values (e.g. 256kB on ppc44x).

With 256kB PAGE_SIZE, the ENTRIES_PER_PAGEPAGE constant becomes too large
(0x1.0000.0000) on a 32-bit kernel, so this patch just changes its type
from 'unsigned long' to 'unsigned long long'.

Hugh: reverted its unsigned long longs in shmem_truncate_range() and
shmem_getpage(): the pagecache index cannot be more than an unsigned long,
so the divisions by zero occurred in unreached code.  It's a pity we need
any ULL arithmetic here, but I found no pretty way to avoid it.

Signed-off-by: Yuri Tikhonov <yur@emcraft.com>
Signed-off-by: Hugh Dickins <hugh@veritas.com>
---

 mm/shmem.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- 2.6.29-git13/mm/shmem.c	2009-04-06 11:48:55.000000000 +0100
+++ 2.6.29-git13+/mm/shmem.c	2009-04-06 15:33:09.000000000 +0100
@@ -66,7 +66,7 @@ static struct vfsmount *shm_mnt;
 #include <asm/pgtable.h>
 
 #define ENTRIES_PER_PAGE (PAGE_CACHE_SIZE/sizeof(unsigned long))
-#define ENTRIES_PER_PAGEPAGE (ENTRIES_PER_PAGE*ENTRIES_PER_PAGE)
+#define ENTRIES_PER_PAGEPAGE ((unsigned long long)ENTRIES_PER_PAGE*ENTRIES_PER_PAGE)
 #define BLOCKS_PER_PAGE  (PAGE_CACHE_SIZE/512)
 
 #define SHMEM_MAX_INDEX  (SHMEM_NR_DIRECT + (ENTRIES_PER_PAGEPAGE/2) * (ENTRIES_PER_PAGE+1))

[PATCH 2/3] shmem: respect MAX_LFS_FILESIZE

[Hugh Dickins]

SHMEM_MAX_BYTES was derived from the maximum size of its triple-indirect
swap vector, forgetting to take the MAX_LFS_FILESIZE limit into account.
Never mind 256kB pages, even 8kB pages on 32-bit kernels allowed files
to grow slightly bigger than that supposed maximum.

Fix this by using the min of both (at build time not run time).  And it
happens that this calculation is good as far as 8MB pages on 32-bit or
16MB pages on 64-bit: though SHMSWP_MAX_INDEX gets truncated before that,
it's truncated to such large numbers that we don't need to care.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
---
Question: couldn't the 32-bit kernel's MAX_LFS_FILESIZE be almost doubled?
It limits the pagecache index to a signed long, but we use an unsigned long.

 mm/shmem.c |   23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

--- 2.6.29-git13+/mm/shmem.c	2009-04-06 15:33:09.000000000 +0100
+++ linux/mm/shmem.c	2009-04-06 18:18:47.000000000 +0100
@@ -65,13 +65,28 @@ static struct vfsmount *shm_mnt;
 #include <asm/div64.h>
 #include <asm/pgtable.h>
 
+/*
+ * The maximum size of a shmem/tmpfs file is limited by the maximum size of
+ * its triple-indirect swap vector - see illustration at shmem_swp_entry().
+ *
+ * With 4kB page size, maximum file size is just over 2TB on a 32-bit kernel,
+ * but one eighth of that on a 64-bit kernel.  With 8kB page size, maximum
+ * file size is just over 4TB on a 64-bit kernel, but 16TB on a 32-bit kernel,
+ * MAX_LFS_FILESIZE being then more restrictive than swap vector layout.
+ *
+ * We use / and * instead of shifts in the definitions below, so that the swap
+ * vector can be tested with small even values (e.g. 20) for ENTRIES_PER_PAGE.
+ */
 #define ENTRIES_PER_PAGE (PAGE_CACHE_SIZE/sizeof(unsigned long))
 #define ENTRIES_PER_PAGEPAGE ((unsigned long long)ENTRIES_PER_PAGE*ENTRIES_PER_PAGE)
-#define BLOCKS_PER_PAGE  (PAGE_CACHE_SIZE/512)
 
-#define SHMEM_MAX_INDEX  (SHMEM_NR_DIRECT + (ENTRIES_PER_PAGEPAGE/2) * (ENTRIES_PER_PAGE+1))
-#define SHMEM_MAX_BYTES  ((unsigned long long)SHMEM_MAX_INDEX << PAGE_CACHE_SHIFT)
+#define SHMSWP_MAX_INDEX (SHMEM_NR_DIRECT + (ENTRIES_PER_PAGEPAGE/2) * (ENTRIES_PER_PAGE+1))
+#define SHMSWP_MAX_BYTES (SHMSWP_MAX_INDEX << PAGE_CACHE_SHIFT)
+
+#define SHMEM_MAX_BYTES  min(SHMSWP_MAX_BYTES, (unsigned long long)MAX_LFS_FILESIZE)
+#define SHMEM_MAX_INDEX  ((unsigned long)((SHMEM_MAX_BYTES+1) >> PAGE_CACHE_SHIFT))
 
+#define BLOCKS_PER_PAGE  (PAGE_CACHE_SIZE/512)
 #define VM_ACCT(size)    (PAGE_CACHE_ALIGN(size) >> PAGE_SHIFT)
 
 /* info->flags needs VM_flags to handle pagein/truncate races efficiently */
@@ -2581,7 +2596,7 @@ int shmem_unuse(swp_entry_t entry, struc
 #define shmem_get_inode(sb, mode, dev, flags)	ramfs_get_inode(sb, mode, dev)
 #define shmem_acct_size(flags, size)		0
 #define shmem_unacct_size(flags, size)		do {} while (0)
-#define SHMEM_MAX_BYTES				LLONG_MAX
+#define SHMEM_MAX_BYTES				MAX_LFS_FILESIZE
 
 #endif /* CONFIG_SHMEM */
 

Re: [PATCH 2/3] shmem: respect MAX_LFS_FILESIZE

[Andrew Morton]

On Mon, 6 Apr 2009 21:56:13 +0100 (BST)
Hugh Dickins <hugh@veritas.com> wrote:

> Question: couldn't the 32-bit kernel's MAX_LFS_FILESIZE be almost doubled?
> It limits the pagecache index to a signed long, but we use an unsigned long.

I expect it would be OK, yes.  The only failure mode I can think of is
if someone is using signed long as a pagecache index and I'd be pretty
surprised if we've made that mistake anywhere.  The potential for goofs
is higher down in filesystems, but they shouldn't be using pagecache
indices much at all.

Of course it does invite people to write applications which then fail
on older kernels, but such is life.

Re: [PATCH 2/3] shmem: respect MAX_LFS_FILESIZE

[Hugh Dickins]

On Thu, 9 Apr 2009, Andrew Morton wrote:
> On Mon, 6 Apr 2009 21:56:13 +0100 (BST)
> Hugh Dickins <hugh@veritas.com> wrote:
> 
> > Question: couldn't the 32-bit kernel's MAX_LFS_FILESIZE be almost doubled?
> > It limits the pagecache index to a signed long, but we use an unsigned long.
> 
> I expect it would be OK, yes.  The only failure mode I can think of is
> if someone is using signed long as a pagecache index and I'd be pretty
> surprised if we've made that mistake anywhere.  The potential for goofs
> is higher down in filesystems, but they shouldn't be using pagecache
> indices much at all.
> 
> Of course it does invite people to write applications which then fail
> on older kernels, but such is life.

Hmm, that's a very good point, and I doubt Ned Kelly can have the
last word on it.  Good filesystems go to a great deal of trouble over
the compatibility issues of new features: it would be rather sad to
blow that all away with a careless doubling of MAX_LFS_FILESIZE.

Or I'm talking nonsense: we already have this issue, when using
a 32-bit kernel to look at big files created with a 64-bit kernel.

But even so, I think I'll leave this change to someone braver.

Hugh

[PATCH 3/3] powerpc: allow 256kB pages with SHMEM

[Hugh Dickins]

Now that shmem's divisions by zero and SHMEM_MAX_BYTES are fixed,
let powerpc 256kB pages coexist with CONFIG_SHMEM again.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
---
Added linuxppc-dev and some other Cc's for this 3/3: sorry
if you didn't see 1/3 and 2/3, they were just in mm/shmem.c.

 arch/powerpc/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- 2.6.29-git13/arch/powerpc/Kconfig	2009-04-06 11:47:57.000000000 +0100
+++ linux/arch/powerpc/Kconfig	2009-04-06 18:18:47.000000000 +0100
@@ -462,7 +462,7 @@ config PPC_64K_PAGES
 
 config PPC_256K_PAGES
 	bool "256k page size" if 44x
-	depends on !STDBINUTILS && (!SHMEM || BROKEN)
+	depends on !STDBINUTILS
 	help
 	  Make the page size 256k.
 

Re: [PATCH 3/3] powerpc: allow 256kB pages with SHMEM

[Paul Mackerras]

Hugh Dickins writes:

> Now that shmem's divisions by zero and SHMEM_MAX_BYTES are fixed,
> let powerpc 256kB pages coexist with CONFIG_SHMEM again.
> 
> Signed-off-by: Hugh Dickins <hugh@veritas.com>
> ---
> Added linuxppc-dev and some other Cc's for this 3/3: sorry
> if you didn't see 1/3 and 2/3, they were just in mm/shmem.c.

Looks OK - what path are you using to get the series upstream?
(I.e. should I be putting 3/3 in the powerpc tree or are you going to
push it along with the others?)

Paul.

Re: [PATCH 3/3] powerpc: allow 256kB pages with SHMEM

[Hugh Dickins]

On Tue, 7 Apr 2009, Paul Mackerras wrote:
> Hugh Dickins writes:
> 
> > Now that shmem's divisions by zero and SHMEM_MAX_BYTES are fixed,
> > let powerpc 256kB pages coexist with CONFIG_SHMEM again.
> > 
> > Signed-off-by: Hugh Dickins <hugh@veritas.com>
> > ---
> > Added linuxppc-dev and some other Cc's for this 3/3: sorry
> > if you didn't see 1/3 and 2/3, they were just in mm/shmem.c.
> 
> Looks OK - what path are you using to get the series upstream?
> (I.e. should I be putting 3/3 in the powerpc tree or are you going to
> push it along with the others?)

If it's OK with you (and you're saying it is), I think the best
would be for it to sit alongside 1/3 and 2/3 in Andrew's tree,
hopefully get an Ack from Yuri (I don't have any powerpc 256kB
pages myself to test it fully), and then go on into 2.6.30.

Hugh

Re: [PATCH 3/3] powerpc: allow 256kB pages with SHMEM

[Andrew Morton]

On Mon, 6 Apr 2009 22:01:15 +0100 (BST)
Hugh Dickins <hugh@veritas.com> wrote:

> Now that shmem's divisions by zero and SHMEM_MAX_BYTES are fixed,
> let powerpc 256kB pages coexist with CONFIG_SHMEM again.
> 
> Signed-off-by: Hugh Dickins <hugh@veritas.com>
> ---
> Added linuxppc-dev and some other Cc's for this 3/3: sorry
> if you didn't see 1/3 and 2/3, they were just in mm/shmem.c.

Do we think these patches should be in 2.6.30?

Re: [PATCH 3/3] powerpc: allow 256kB pages with SHMEM

[Hugh Dickins]

On Thu, 9 Apr 2009, Andrew Morton wrote:
> On Mon, 6 Apr 2009 22:01:15 +0100 (BST)
> Hugh Dickins <hugh@veritas.com> wrote:
> 
> > Now that shmem's divisions by zero and SHMEM_MAX_BYTES are fixed,
> > let powerpc 256kB pages coexist with CONFIG_SHMEM again.
> > 
> > Signed-off-by: Hugh Dickins <hugh@veritas.com>
> > ---
> > Added linuxppc-dev and some other Cc's for this 3/3: sorry
> > if you didn't see 1/3 and 2/3, they were just in mm/shmem.c.
> 
> Do we think these patches should be in 2.6.30?

I think so - 2.6.30 will have the rest of Yuri's 256kB page support.
But it would be nice to have an Ack from Yuri before sending these
on through.  2/3 is a bugfix justified even without 256kB pages -
I should have inverted the ordering.

Hugh

Re: [PATCH 3/3] powerpc: allow 256kB pages with SHMEM

[Ilya Yanok]

Hi Hugh,

Hugh Dickins wrote:
> Now that shmem's divisions by zero and SHMEM_MAX_BYTES are fixed,
> let powerpc 256kB pages coexist with CONFIG_SHMEM again.
>
> Signed-off-by: Hugh Dickins <hugh@veritas.com>
>   

Acked-by: Ilya Yanok <yanok@emcraft.com>

> ---
> Added linuxppc-dev and some other Cc's for this 3/3: sorry
> if you didn't see 1/3 and 2/3, they were just in mm/shmem.c.
>
>  arch/powerpc/Kconfig |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- 2.6.29-git13/arch/powerpc/Kconfig	2009-04-06 11:47:57.000000000 +0100
> +++ linux/arch/powerpc/Kconfig	2009-04-06 18:18:47.000000000 +0100
> @@ -462,7 +462,7 @@ config PPC_64K_PAGES
>  
>  config PPC_256K_PAGES
>  	bool "256k page size" if 44x
> -	depends on !STDBINUTILS && (!SHMEM || BROKEN)
> +	depends on !STDBINUTILS
>  	help
>  	  Make the page size 256k.
>  
>   

Regards, Ilya.