From: Yuri Tikhonov <[email protected]>
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 <[email protected]>
Signed-off-by: Hugh Dickins <[email protected]>
---
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))
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 <[email protected]>
---
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 */
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 <[email protected]>
---
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.
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 <[email protected]>
> ---
> 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.
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 <[email protected]>
> > ---
> > 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
On Mon, 6 Apr 2009 21:56:13 +0100 (BST)
Hugh Dickins <[email protected]> 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.
On Mon, 6 Apr 2009 22:01:15 +0100 (BST)
Hugh Dickins <[email protected]> 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 <[email protected]>
> ---
> 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?
On Thu, 9 Apr 2009, Andrew Morton wrote:
> On Mon, 6 Apr 2009 22:01:15 +0100 (BST)
> Hugh Dickins <[email protected]> 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 <[email protected]>
> > ---
> > 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
On Thu, 9 Apr 2009, Andrew Morton wrote:
> On Mon, 6 Apr 2009 21:56:13 +0100 (BST)
> Hugh Dickins <[email protected]> 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
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 <[email protected]>
>
Acked-by: Ilya Yanok <[email protected]>
> ---
> 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.