[PATCH 1/3] ext3/ext4: Factor out disk addressability check

[PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Patrick J. LoPresti]

As part of adding support for OCFS2 to mount huge volumes, we need to
check that the sector_t and page cache of the system are capable of
addressing the entire volume.

An identical check already appears in ext3 and ext4.  This patch moves
the addressability check into its own function in fs/libfs.c and
modifies ext3 and ext4 to invoke it.

Signed-off-by: Patrick LoPresti <lopresti@gmail.com>

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 471e1ff..6aff3f5 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2399,6 +2399,8 @@ extern ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
 
 extern int generic_file_fsync(struct file *, int);
 
+extern int generic_check_addressable(unsigned, u64);
+
 #ifdef CONFIG_MIGRATION
 extern int buffer_migrate_page(struct address_space *,
 				struct page *, struct page *);
diff --git a/fs/libfs.c b/fs/libfs.c
index dcaf972..b969648 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -955,6 +955,38 @@ int generic_file_fsync(struct file *file, int datasync)
 }
 EXPORT_SYMBOL(generic_file_fsync);
 
+/**
+ * generic_check_addressable - Check addressability of file system
+ * @blocksize_bits:	log of file system block size
+ * @num_blocks:		number of blocks in file system
+ *
+ * Determine whether a file system with @num_blocks blocks (and a
+ * block size of 2**@blocksize_bits) is addressable by the sector_t
+ * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
+ */
+int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
+{
+	u64 last_fs_block = num_blocks - 1;
+
+	BUG_ON(blocksize_bits < 9);
+	BUG_ON(blocksize_bits > PAGE_CACHE_SHIFT);
+
+	if (unlikely(num_blocks == 0))
+		return 0;
+
+	printk(KERN_INFO "HERE %u %lu %u %u", blocksize_bits, last_fs_block,
+	       sizeof(sector_t), sizeof(pgoff_t));
+
+	if ((last_fs_block >
+	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
+	    (last_fs_block >
+	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
+		return -EFBIG;
+	}
+	return 0;
+}
+EXPORT_SYMBOL(generic_check_addressable);
+
 /*
  * No-op implementation of ->fsync for in-memory filesystems.
  */
diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index 6c953bb..d0643db 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -1862,8 +1862,8 @@ static int ext3_fill_super (struct super_block *sb, void *data, int silent)
 		goto failed_mount;
 	}
 
-	if (le32_to_cpu(es->s_blocks_count) >
-		    (sector_t)(~0ULL) >> (sb->s_blocksize_bits - 9)) {
+	if (generic_check_addressable(sb->s_blocksize_bits,
+				      le32_to_cpu(es->s_blocks_count))) {
 		ext3_msg(sb, KERN_ERR,
 			"error: filesystem is too large to mount safely");
 		if (sizeof(sector_t) < 8)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 4e8983a..979cc57 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2706,15 +2706,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	 * Test whether we have more sectors than will fit in sector_t,
 	 * and whether the max offset is addressable by the page cache.
 	 */
-	if ((ext4_blocks_count(es) >
-	     (sector_t)(~0ULL) >> (sb->s_blocksize_bits - 9)) ||
-	    (ext4_blocks_count(es) >
-	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - sb->s_blocksize_bits))) {
+	ret = generic_check_addressable(sb->s_blocksize_bits,
+					ext4_blocks_count(es));
+	if (ret) {
 		ext4_msg(sb, KERN_ERR, "filesystem"
 			 " too large to mount safely on this system");
 		if (sizeof(sector_t) < 8)
 			ext4_msg(sb, KERN_WARNING, "CONFIG_LBDAF not enabled");
-		ret = -EFBIG;
 		goto failed_mount;
 	}

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Patrick J. LoPresti]

As part of adding support for OCFS2 to mount huge volumes, we need to
check that the sector_t and page cache of the system are capable of
addressing the entire volume.

An identical check already appears in ext3 and ext4.  This patch moves
the addressability check into its own function in fs/libfs.c and
modifies ext3 and ext4 to invoke it.

Signed-off-by: Patrick LoPresti <lopresti@gmail.com>

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 471e1ff..6aff3f5 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2399,6 +2399,8 @@ extern ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
 
 extern int generic_file_fsync(struct file *, int);
 
+extern int generic_check_addressable(unsigned, u64);
+
 #ifdef CONFIG_MIGRATION
 extern int buffer_migrate_page(struct address_space *,
 				struct page *, struct page *);
diff --git a/fs/libfs.c b/fs/libfs.c
index dcaf972..b969648 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -955,6 +955,38 @@ int generic_file_fsync(struct file *file, int datasync)
 }
 EXPORT_SYMBOL(generic_file_fsync);
 
+/**
+ * generic_check_addressable - Check addressability of file system
+ * @blocksize_bits:	log of file system block size
+ * @num_blocks:		number of blocks in file system
+ *
+ * Determine whether a file system with @num_blocks blocks (and a
+ * block size of 2**@blocksize_bits) is addressable by the sector_t
+ * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
+ */
+int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
+{
+	u64 last_fs_block = num_blocks - 1;
+
+	BUG_ON(blocksize_bits < 9);
+	BUG_ON(blocksize_bits > PAGE_CACHE_SHIFT);
+
+	if (unlikely(num_blocks == 0))
+		return 0;
+
+	printk(KERN_INFO "HERE %u %lu %u %u", blocksize_bits, last_fs_block,
+	       sizeof(sector_t), sizeof(pgoff_t));
+
+	if ((last_fs_block >
+	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
+	    (last_fs_block >
+	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
+		return -EFBIG;
+	}
+	return 0;
+}
+EXPORT_SYMBOL(generic_check_addressable);
+
 /*
  * No-op implementation of ->fsync for in-memory filesystems.
  */
diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index 6c953bb..d0643db 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -1862,8 +1862,8 @@ static int ext3_fill_super (struct super_block *sb, void *data, int silent)
 		goto failed_mount;
 	}
 
-	if (le32_to_cpu(es->s_blocks_count) >
-		    (sector_t)(~0ULL) >> (sb->s_blocksize_bits - 9)) {
+	if (generic_check_addressable(sb->s_blocksize_bits,
+				      le32_to_cpu(es->s_blocks_count))) {
 		ext3_msg(sb, KERN_ERR,
 			"error: filesystem is too large to mount safely");
 		if (sizeof(sector_t) < 8)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 4e8983a..979cc57 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2706,15 +2706,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	 * Test whether we have more sectors than will fit in sector_t,
 	 * and whether the max offset is addressable by the page cache.
 	 */
-	if ((ext4_blocks_count(es) >
-	     (sector_t)(~0ULL) >> (sb->s_blocksize_bits - 9)) ||
-	    (ext4_blocks_count(es) >
-	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - sb->s_blocksize_bits))) {
+	ret = generic_check_addressable(sb->s_blocksize_bits,
+					ext4_blocks_count(es));
+	if (ret) {
 		ext4_msg(sb, KERN_ERR, "filesystem"
 			 " too large to mount safely on this system");
 		if (sizeof(sector_t) < 8)
 			ext4_msg(sb, KERN_WARNING, "CONFIG_LBDAF not enabled");
-		ret = -EFBIG;
 		goto failed_mount;
 	}
 

[PATCH 2/3] JBD2: Allow feature checks before journal recovery

[Patrick J. LoPresti]

Before we start accessing a huge (> 16 TiB) OCFS2 volume, we need to
confirm that its journal supports 64-bit offsets.  In particular, we
need to check the journal's feature bits before recovering the journal.

This is not possible with JBD2 at present, because the journal
superblock (where the feature bits reside) is not loaded from disk until
the journal is recovered.

This patch loads the journal superblock in
jbd2_journal_check_used_features() if it has not already been loaded,
allowing us to check the feature bits before journal recovery.

Signed-off-by: Patrick LoPresti <lopresti@gmail.com>

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index bc2ff59..5cfd8d4 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1365,6 +1365,10 @@ int jbd2_journal_check_used_features (journal_t *journal, unsigned long compat,
 
 	if (!compat && !ro && !incompat)
 		return 1;
+	/* Load journal superblock if it is not loaded yet. */
+	if (journal->j_format_version == 0 &&
+	    journal_get_superblock(journal) != 0)
+		return 0;
 	if (journal->j_format_version == 1)
 		return 0;

[Ocfs2-devel] [PATCH 2/3] JBD2: Allow feature checks before journal recovery

[Patrick J. LoPresti]

Before we start accessing a huge (> 16 TiB) OCFS2 volume, we need to
confirm that its journal supports 64-bit offsets.  In particular, we
need to check the journal's feature bits before recovering the journal.

This is not possible with JBD2 at present, because the journal
superblock (where the feature bits reside) is not loaded from disk until
the journal is recovered.

This patch loads the journal superblock in
jbd2_journal_check_used_features() if it has not already been loaded,
allowing us to check the feature bits before journal recovery.

Signed-off-by: Patrick LoPresti <lopresti@gmail.com>

diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
index bc2ff59..5cfd8d4 100644
--- a/fs/jbd2/journal.c
+++ b/fs/jbd2/journal.c
@@ -1365,6 +1365,10 @@ int jbd2_journal_check_used_features (journal_t *journal, unsigned long compat,
 
 	if (!compat && !ro && !incompat)
 		return 1;
+	/* Load journal superblock if it is not loaded yet. */
+	if (journal->j_format_version == 0 &&
+	    journal_get_superblock(journal) != 0)
+		return 0;
 	if (journal->j_format_version == 1)
 		return 0;
 

[PATCH 3/3] OCFS2: Allow huge (> 16 TiB) volumes to mount

[Patrick J. LoPresti]

The OCFS2 developers have already done all of the hard work to allow
volumes larger than 16 TiB.  But there is still a "sanity check" in
fs/ocfs2/super.c that prevents the mounting of such volumes, even when
the cluster size and journal options would allow it.

This patch replaces that sanity check with a more sophisticated one to
mount a huge volume provided that (a) it is addressable by the raw
word/address size of the system (borrowing a test from ext4); (b) the
volume is using JBD2; and (c) the JBD2_FEATURE_INCOMPAT_64BIT flag is
set on the journal.

I factored out the sanity check into its own function.  I also moved it
from ocfs2_initialize_super() down to ocfs2_check_volume(); any earlier,
and the journal will not have been initialized yet.

This patch is one of a pair, and it depends on the other ("JBD2: Allow
feature checks before journal recovery").

I have tested this patch on small volumes, huge volumes, and huge
volumes without 64-bit block support in the journal.  All of them appear
to work or to fail gracefully, as appropriate.

Signed-off-by: Patrick LoPresti <lopresti@gmail.com>

diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 0eaa929..76dac4c 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -1991,6 +1991,36 @@ static int ocfs2_setup_osb_uuid(struct ocfs2_super *osb, const unsigned char *uu
 	return 0;
 }
 
+/* Make sure entire volume is addressable by our journal.  Requires
+   osb_clusters_at_boot to be valid and for the journal to have been
+   initialized by ocfs2_journal_init(). */
+static int ocfs2_journal_addressable(struct ocfs2_super *osb)
+{
+	int status = 0;
+	u64 max_block =
+		ocfs2_clusters_to_blocks(osb->sb,
+					 osb->osb_clusters_at_boot) - 1;
+
+	/* 32-bit block number is always OK. */
+	if (max_block <= (u32)~0ULL)
+		goto out;
+
+	/* Volume is "huge", so see if our journal is new enough to
+	   support it. */
+	if (!(OCFS2_HAS_COMPAT_FEATURE(osb->sb,
+				       OCFS2_FEATURE_COMPAT_JBD2_SB) &&
+	      jbd2_journal_check_used_features(osb->journal->j_journal, 0, 0,
+					       JBD2_FEATURE_INCOMPAT_64BIT))) {
+		mlog(ML_ERROR, "The journal cannot address the entire volume. "
+		     "Enable the 'block64' journal option with tunefs.ocfs2");
+		status = -EFBIG;
+		goto out;
+	}
+
+ out:
+	return status;
+}
+
 static int ocfs2_initialize_super(struct super_block *sb,
 				  struct buffer_head *bh,
 				  int sector_size,
@@ -2003,6 +2033,7 @@ static int ocfs2_initialize_super(struct super_block *sb,
 	struct ocfs2_journal *journal;
 	__le32 uuid_net_key;
 	struct ocfs2_super *osb;
+	u64 total_blocks;
 
 	mlog_entry_void();
 
@@ -2215,11 +2246,15 @@ static int ocfs2_initialize_super(struct super_block *sb,
 		goto bail;
 	}
 
-	if (ocfs2_clusters_to_blocks(osb->sb, le32_to_cpu(di->i_clusters) - 1)
-	    > (u32)~0UL) {
-		mlog(ML_ERROR, "Volume might try to write to blocks beyond "
-		     "what jbd can address in 32 bits.\n");
-		status = -EINVAL;
+	total_blocks = ocfs2_clusters_to_blocks(osb->sb,
+						le32_to_cpu(di->i_clusters));
+
+	status = generic_check_addressable(osb->sb->s_blocksize_bits,
+					   total_blocks);
+	if (status) {
+		mlog(ML_ERROR, "Volume too large "
+		     "to mount safely on this system");
+		status = -EFBIG;
 		goto bail;
 	}
 
@@ -2381,6 +2416,12 @@ static int ocfs2_check_volume(struct ocfs2_super *osb)
 		goto finally;
 	}
 
+	/* Now that journal has been initialized, check to make sure
+	   entire volume is addressable. */
+	status = ocfs2_journal_addressable(osb);
+	if (status)
+		goto finally;
+
 	/* If the journal was unmounted cleanly then we don't want to
 	 * recover anything. Otherwise, journal_load will do that
 	 * dirty work for us :) */

[Ocfs2-devel] [PATCH 3/3] OCFS2: Allow huge (> 16 TiB) volumes to mount

[Patrick J. LoPresti]

The OCFS2 developers have already done all of the hard work to allow
volumes larger than 16 TiB.  But there is still a "sanity check" in
fs/ocfs2/super.c that prevents the mounting of such volumes, even when
the cluster size and journal options would allow it.

This patch replaces that sanity check with a more sophisticated one to
mount a huge volume provided that (a) it is addressable by the raw
word/address size of the system (borrowing a test from ext4); (b) the
volume is using JBD2; and (c) the JBD2_FEATURE_INCOMPAT_64BIT flag is
set on the journal.

I factored out the sanity check into its own function.  I also moved it
from ocfs2_initialize_super() down to ocfs2_check_volume(); any earlier,
and the journal will not have been initialized yet.

This patch is one of a pair, and it depends on the other ("JBD2: Allow
feature checks before journal recovery").

I have tested this patch on small volumes, huge volumes, and huge
volumes without 64-bit block support in the journal.  All of them appear
to work or to fail gracefully, as appropriate.

Signed-off-by: Patrick LoPresti <lopresti@gmail.com>

diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c
index 0eaa929..76dac4c 100644
--- a/fs/ocfs2/super.c
+++ b/fs/ocfs2/super.c
@@ -1991,6 +1991,36 @@ static int ocfs2_setup_osb_uuid(struct ocfs2_super *osb, const unsigned char *uu
 	return 0;
 }
 
+/* Make sure entire volume is addressable by our journal.  Requires
+   osb_clusters_at_boot to be valid and for the journal to have been
+   initialized by ocfs2_journal_init(). */
+static int ocfs2_journal_addressable(struct ocfs2_super *osb)
+{
+	int status = 0;
+	u64 max_block =
+		ocfs2_clusters_to_blocks(osb->sb,
+					 osb->osb_clusters_at_boot) - 1;
+
+	/* 32-bit block number is always OK. */
+	if (max_block <= (u32)~0ULL)
+		goto out;
+
+	/* Volume is "huge", so see if our journal is new enough to
+	   support it. */
+	if (!(OCFS2_HAS_COMPAT_FEATURE(osb->sb,
+				       OCFS2_FEATURE_COMPAT_JBD2_SB) &&
+	      jbd2_journal_check_used_features(osb->journal->j_journal, 0, 0,
+					       JBD2_FEATURE_INCOMPAT_64BIT))) {
+		mlog(ML_ERROR, "The journal cannot address the entire volume. "
+		     "Enable the 'block64' journal option with tunefs.ocfs2");
+		status = -EFBIG;
+		goto out;
+	}
+
+ out:
+	return status;
+}
+
 static int ocfs2_initialize_super(struct super_block *sb,
 				  struct buffer_head *bh,
 				  int sector_size,
@@ -2003,6 +2033,7 @@ static int ocfs2_initialize_super(struct super_block *sb,
 	struct ocfs2_journal *journal;
 	__le32 uuid_net_key;
 	struct ocfs2_super *osb;
+	u64 total_blocks;
 
 	mlog_entry_void();
 
@@ -2215,11 +2246,15 @@ static int ocfs2_initialize_super(struct super_block *sb,
 		goto bail;
 	}
 
-	if (ocfs2_clusters_to_blocks(osb->sb, le32_to_cpu(di->i_clusters) - 1)
-	    > (u32)~0UL) {
-		mlog(ML_ERROR, "Volume might try to write to blocks beyond "
-		     "what jbd can address in 32 bits.\n");
-		status = -EINVAL;
+	total_blocks = ocfs2_clusters_to_blocks(osb->sb,
+						le32_to_cpu(di->i_clusters));
+
+	status = generic_check_addressable(osb->sb->s_blocksize_bits,
+					   total_blocks);
+	if (status) {
+		mlog(ML_ERROR, "Volume too large "
+		     "to mount safely on this system");
+		status = -EFBIG;
 		goto bail;
 	}
 
@@ -2381,6 +2416,12 @@ static int ocfs2_check_volume(struct ocfs2_super *osb)
 		goto finally;
 	}
 
+	/* Now that journal has been initialized, check to make sure
+	   entire volume is addressable. */
+	status = ocfs2_journal_addressable(osb);
+	if (status)
+		goto finally;
+
 	/* If the journal was unmounted cleanly then we don't want to
 	 * recover anything. Otherwise, journal_load will do that
 	 * dirty work for us :) */

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Thu, Jul 22, 2010 at 03:03:41PM -0700, Patrick J. LoPresti wrote:
> As part of adding support for OCFS2 to mount huge volumes, we need to
> check that the sector_t and page cache of the system are capable of
> addressing the entire volume.
> 
> An identical check already appears in ext3 and ext4.  This patch moves
> the addressability check into its own function in fs/libfs.c and
> modifies ext3 and ext4 to invoke it.
> 
> Signed-off-by: Patrick LoPresti <lopresti@gmail.com>

ext4/jbd2 folks,
	I would like to push these upstream.  Should they go through
ocfs2.git or would you rather take them via your trees?

Joel

-- 

"In the arms of the angel, fly away from here,
 From this dark, cold hotel room and the endlessness that you fear.
 You are pulled from the wreckage of your silent reverie.
 In the arms of the angel, may you find some comfort here."

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Thu, Jul 22, 2010 at 03:03:41PM -0700, Patrick J. LoPresti wrote:
> As part of adding support for OCFS2 to mount huge volumes, we need to
> check that the sector_t and page cache of the system are capable of
> addressing the entire volume.
> 
> An identical check already appears in ext3 and ext4.  This patch moves
> the addressability check into its own function in fs/libfs.c and
> modifies ext3 and ext4 to invoke it.
> 
> Signed-off-by: Patrick LoPresti <lopresti@gmail.com>

ext4/jbd2 folks,
	I would like to push these upstream.  Should they go through
ocfs2.git or would you rather take them via your trees?

Joel

-- 

"In the arms of the angel, fly away from here,
 From this dark, cold hotel room and the endlessness that you fear.
 You are pulled from the wreckage of your silent reverie.
 In the arms of the angel, may you find some comfort here."

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Thu, Jul 22, 2010 at 03:03:41PM -0700, Patrick J. LoPresti wrote:
> As part of adding support for OCFS2 to mount huge volumes, we need to
> check that the sector_t and page cache of the system are capable of
> addressing the entire volume.
> 
> An identical check already appears in ext3 and ext4.  This patch moves
> the addressability check into its own function in fs/libfs.c and
> modifies ext3 and ext4 to invoke it.
> 
> Signed-off-by: Patrick LoPresti <lopresti@gmail.com>

Dear ext3/4 folks,
	I've pushed this patch to the merge-window branch of ocfs2.git.
I'm ready to send it to Linus, But I need your OK.

Joel

> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 471e1ff..6aff3f5 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2399,6 +2399,8 @@ extern ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
>  
>  extern int generic_file_fsync(struct file *, int);
>  
> +extern int generic_check_addressable(unsigned, u64);
> +
>  #ifdef CONFIG_MIGRATION
>  extern int buffer_migrate_page(struct address_space *,
>  				struct page *, struct page *);
> diff --git a/fs/libfs.c b/fs/libfs.c
> index dcaf972..b969648 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -955,6 +955,38 @@ int generic_file_fsync(struct file *file, int datasync)
>  }
>  EXPORT_SYMBOL(generic_file_fsync);
>  
> +/**
> + * generic_check_addressable - Check addressability of file system
> + * @blocksize_bits:	log of file system block size
> + * @num_blocks:		number of blocks in file system
> + *
> + * Determine whether a file system with @num_blocks blocks (and a
> + * block size of 2**@blocksize_bits) is addressable by the sector_t
> + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
> + */
> +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
> +{
> +	u64 last_fs_block = num_blocks - 1;
> +
> +	BUG_ON(blocksize_bits < 9);
> +	BUG_ON(blocksize_bits > PAGE_CACHE_SHIFT);
> +
> +	if (unlikely(num_blocks == 0))
> +		return 0;
> +
> +	printk(KERN_INFO "HERE %u %lu %u %u", blocksize_bits, last_fs_block,
> +	       sizeof(sector_t), sizeof(pgoff_t));

Minus this printk(), of course ;-)

> +
> +	if ((last_fs_block >
> +	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
> +	    (last_fs_block >
> +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
> +		return -EFBIG;
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL(generic_check_addressable);
> +
>  /*
>   * No-op implementation of ->fsync for in-memory filesystems.
>   */
> diff --git a/fs/ext3/super.c b/fs/ext3/super.c
> index 6c953bb..d0643db 100644
> --- a/fs/ext3/super.c
> +++ b/fs/ext3/super.c
> @@ -1862,8 +1862,8 @@ static int ext3_fill_super (struct super_block *sb, void *data, int silent)
>  		goto failed_mount;
>  	}
>  
> -	if (le32_to_cpu(es->s_blocks_count) >
> -		    (sector_t)(~0ULL) >> (sb->s_blocksize_bits - 9)) {
> +	if (generic_check_addressable(sb->s_blocksize_bits,
> +				      le32_to_cpu(es->s_blocks_count))) {
>  		ext3_msg(sb, KERN_ERR,
>  			"error: filesystem is too large to mount safely");
>  		if (sizeof(sector_t) < 8)
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 4e8983a..979cc57 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2706,15 +2706,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
>  	 * Test whether we have more sectors than will fit in sector_t,
>  	 * and whether the max offset is addressable by the page cache.
>  	 */
> -	if ((ext4_blocks_count(es) >
> -	     (sector_t)(~0ULL) >> (sb->s_blocksize_bits - 9)) ||
> -	    (ext4_blocks_count(es) >
> -	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - sb->s_blocksize_bits))) {
> +	ret = generic_check_addressable(sb->s_blocksize_bits,
> +					ext4_blocks_count(es));
> +	if (ret) {
>  		ext4_msg(sb, KERN_ERR, "filesystem"
>  			 " too large to mount safely on this system");
>  		if (sizeof(sector_t) < 8)
>  			ext4_msg(sb, KERN_WARNING, "CONFIG_LBDAF not enabled");
> -		ret = -EFBIG;
>  		goto failed_mount;
>  	}
>  
> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel@oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel

-- 

"What does it say about a society's priorities when the time you
 spend in meetings on Monday is greater than the total number of
 hours you spent sleeping over the weekend?"
	- Nat Friedman

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Thu, Jul 22, 2010 at 03:03:41PM -0700, Patrick J. LoPresti wrote:
> As part of adding support for OCFS2 to mount huge volumes, we need to
> check that the sector_t and page cache of the system are capable of
> addressing the entire volume.
> 
> An identical check already appears in ext3 and ext4.  This patch moves
> the addressability check into its own function in fs/libfs.c and
> modifies ext3 and ext4 to invoke it.
> 
> Signed-off-by: Patrick LoPresti <lopresti@gmail.com>

Dear ext3/4 folks,
	I've pushed this patch to the merge-window branch of ocfs2.git.
I'm ready to send it to Linus, But I need your OK.

Joel

> 
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index 471e1ff..6aff3f5 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2399,6 +2399,8 @@ extern ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
>  
>  extern int generic_file_fsync(struct file *, int);
>  
> +extern int generic_check_addressable(unsigned, u64);
> +
>  #ifdef CONFIG_MIGRATION
>  extern int buffer_migrate_page(struct address_space *,
>  				struct page *, struct page *);
> diff --git a/fs/libfs.c b/fs/libfs.c
> index dcaf972..b969648 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -955,6 +955,38 @@ int generic_file_fsync(struct file *file, int datasync)
>  }
>  EXPORT_SYMBOL(generic_file_fsync);
>  
> +/**
> + * generic_check_addressable - Check addressability of file system
> + * @blocksize_bits:	log of file system block size
> + * @num_blocks:		number of blocks in file system
> + *
> + * Determine whether a file system with @num_blocks blocks (and a
> + * block size of 2**@blocksize_bits) is addressable by the sector_t
> + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
> + */
> +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
> +{
> +	u64 last_fs_block = num_blocks - 1;
> +
> +	BUG_ON(blocksize_bits < 9);
> +	BUG_ON(blocksize_bits > PAGE_CACHE_SHIFT);
> +
> +	if (unlikely(num_blocks == 0))
> +		return 0;
> +
> +	printk(KERN_INFO "HERE %u %lu %u %u", blocksize_bits, last_fs_block,
> +	       sizeof(sector_t), sizeof(pgoff_t));

Minus this printk(), of course ;-)

> +
> +	if ((last_fs_block >
> +	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
> +	    (last_fs_block >
> +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
> +		return -EFBIG;
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL(generic_check_addressable);
> +
>  /*
>   * No-op implementation of ->fsync for in-memory filesystems.
>   */
> diff --git a/fs/ext3/super.c b/fs/ext3/super.c
> index 6c953bb..d0643db 100644
> --- a/fs/ext3/super.c
> +++ b/fs/ext3/super.c
> @@ -1862,8 +1862,8 @@ static int ext3_fill_super (struct super_block *sb, void *data, int silent)
>  		goto failed_mount;
>  	}
>  
> -	if (le32_to_cpu(es->s_blocks_count) >
> -		    (sector_t)(~0ULL) >> (sb->s_blocksize_bits - 9)) {
> +	if (generic_check_addressable(sb->s_blocksize_bits,
> +				      le32_to_cpu(es->s_blocks_count))) {
>  		ext3_msg(sb, KERN_ERR,
>  			"error: filesystem is too large to mount safely");
>  		if (sizeof(sector_t) < 8)
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 4e8983a..979cc57 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2706,15 +2706,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
>  	 * Test whether we have more sectors than will fit in sector_t,
>  	 * and whether the max offset is addressable by the page cache.
>  	 */
> -	if ((ext4_blocks_count(es) >
> -	     (sector_t)(~0ULL) >> (sb->s_blocksize_bits - 9)) ||
> -	    (ext4_blocks_count(es) >
> -	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - sb->s_blocksize_bits))) {
> +	ret = generic_check_addressable(sb->s_blocksize_bits,
> +					ext4_blocks_count(es));
> +	if (ret) {
>  		ext4_msg(sb, KERN_ERR, "filesystem"
>  			 " too large to mount safely on this system");
>  		if (sizeof(sector_t) < 8)
>  			ext4_msg(sb, KERN_WARNING, "CONFIG_LBDAF not enabled");
> -		ret = -EFBIG;
>  		goto failed_mount;
>  	}
>  
> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel

-- 

"What does it say about a society's priorities when the time you
 spend in meetings on Monday is greater than the total number of
 hours you spent sleeping over the weekend?"
	- Nat Friedman

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

Re: [Ocfs2-devel] [PATCH 2/3] JBD2: Allow feature checks before journal recovery

[Joel Becker]

On Thu, Jul 22, 2010 at 03:04:16PM -0700, Patrick J. LoPresti wrote:
> Before we start accessing a huge (> 16 TiB) OCFS2 volume, we need to
> confirm that its journal supports 64-bit offsets.  In particular, we
> need to check the journal's feature bits before recovering the journal.
> 
> This is not possible with JBD2 at present, because the journal
> superblock (where the feature bits reside) is not loaded from disk until
> the journal is recovered.
> 
> This patch loads the journal superblock in
> jbd2_journal_check_used_features() if it has not already been loaded,
> allowing us to check the feature bits before journal recovery.
> 
> Signed-off-by: Patrick LoPresti <lopresti@gmail.com>

Dear jbd2 developers,
	I've pushed this patch to the merge-window branch of ocfs2.git.
I'm ready to send it to Linus, but I need your OK.

Joel

> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index bc2ff59..5cfd8d4 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1365,6 +1365,10 @@ int jbd2_journal_check_used_features (journal_t *journal, unsigned long compat,
>  
>  	if (!compat && !ro && !incompat)
>  		return 1;
> +	/* Load journal superblock if it is not loaded yet. */
> +	if (journal->j_format_version == 0 &&
> +	    journal_get_superblock(journal) != 0)
> +		return 0;
>  	if (journal->j_format_version == 1)
>  		return 0;
>  
> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel@oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel

-- 

 Brain: I shall pollute the water supply with this DNAdefibuliser,
        turning everyone into mindless slaves.
 Pinky: What about the people who drink bottled water?
 Brain: Pinky, people who pay 5 dollars for a bottle of water are
        already mindless slaves.

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 2/3] JBD2: Allow feature checks before journal recovery

[Joel Becker]

On Thu, Jul 22, 2010 at 03:04:16PM -0700, Patrick J. LoPresti wrote:
> Before we start accessing a huge (> 16 TiB) OCFS2 volume, we need to
> confirm that its journal supports 64-bit offsets.  In particular, we
> need to check the journal's feature bits before recovering the journal.
> 
> This is not possible with JBD2 at present, because the journal
> superblock (where the feature bits reside) is not loaded from disk until
> the journal is recovered.
> 
> This patch loads the journal superblock in
> jbd2_journal_check_used_features() if it has not already been loaded,
> allowing us to check the feature bits before journal recovery.
> 
> Signed-off-by: Patrick LoPresti <lopresti@gmail.com>

Dear jbd2 developers,
	I've pushed this patch to the merge-window branch of ocfs2.git.
I'm ready to send it to Linus, but I need your OK.

Joel

> diff --git a/fs/jbd2/journal.c b/fs/jbd2/journal.c
> index bc2ff59..5cfd8d4 100644
> --- a/fs/jbd2/journal.c
> +++ b/fs/jbd2/journal.c
> @@ -1365,6 +1365,10 @@ int jbd2_journal_check_used_features (journal_t *journal, unsigned long compat,
>  
>  	if (!compat && !ro && !incompat)
>  		return 1;
> +	/* Load journal superblock if it is not loaded yet. */
> +	if (journal->j_format_version == 0 &&
> +	    journal_get_superblock(journal) != 0)
> +		return 0;
>  	if (journal->j_format_version == 1)
>  		return 0;
>  
> 
> _______________________________________________
> Ocfs2-devel mailing list
> Ocfs2-devel at oss.oracle.com
> http://oss.oracle.com/mailman/listinfo/ocfs2-devel

-- 

 Brain: I shall pollute the water supply with this DNAdefibuliser,
        turning everyone into mindless slaves.
 Pinky: What about the people who drink bottled water?
 Brain: Pinky, people who pay 5 dollars for a bottle of water are
        already mindless slaves.

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Andreas Dilger]

On 2010-08-12, at 11:42, Joel Becker wrote:
>> +/**
>> + * generic_check_addressable - Check addressability of file system
>> + * @blocksize_bits:	log of file system block size
>> + * @num_blocks:		number of blocks in file system
>> + *
>> + * Determine whether a file system with @num_blocks blocks (and a
>> + * block size of 2**@blocksize_bits) is addressable by the sector_t
>> + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
>> + */
>> +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
>> +{
>> +	u64 last_fs_block = num_blocks - 1;
>> +
>> +	BUG_ON(blocksize_bits < 9);
>> +	BUG_ON(blocksize_bits > PAGE_CACHE_SHIFT);

I'd rather not have a BUG_ON() for a "check" function that may be called with on-disk values by some filesystem.  Some filesystems (AFAIR) also handle blocksize > PAGE_SIZE internally, so this helper would not be useful for them.

Cheers, Andreas

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Andreas Dilger]

On 2010-08-12, at 11:42, Joel Becker wrote:
>> +/**
>> + * generic_check_addressable - Check addressability of file system
>> + * @blocksize_bits:	log of file system block size
>> + * @num_blocks:		number of blocks in file system
>> + *
>> + * Determine whether a file system with @num_blocks blocks (and a
>> + * block size of 2**@blocksize_bits) is addressable by the sector_t
>> + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
>> + */
>> +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
>> +{
>> +	u64 last_fs_block = num_blocks - 1;
>> +
>> +	BUG_ON(blocksize_bits < 9);
>> +	BUG_ON(blocksize_bits > PAGE_CACHE_SHIFT);

I'd rather not have a BUG_ON() for a "check" function that may be called with on-disk values by some filesystem.  Some filesystems (AFAIR) also handle blocksize > PAGE_SIZE internally, so this helper would not be useful for them.

Cheers, Andreas

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Thu, Aug 12, 2010 at 12:45:41PM -0600, Andreas Dilger wrote:
> On 2010-08-12, at 11:42, Joel Becker wrote:
> >> +/**
> >> + * generic_check_addressable - Check addressability of file system
> >> + * @blocksize_bits:	log of file system block size
> >> + * @num_blocks:		number of blocks in file system
> >> + *
> >> + * Determine whether a file system with @num_blocks blocks (and a
> >> + * block size of 2**@blocksize_bits) is addressable by the sector_t
> >> + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
> >> + */
> >> +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
> >> +{
> >> +	u64 last_fs_block = num_blocks - 1;
> >> +
> >> +	BUG_ON(blocksize_bits < 9);
> >> +	BUG_ON(blocksize_bits > PAGE_CACHE_SHIFT);
> 
> I'd rather not have a BUG_ON() for a "check" function that may be called with on-disk values by some filesystem.  Some filesystems (AFAIR) also handle blocksize > PAGE_SIZE internally, so this helper would not be useful for them.

	Filesystems that handle their own page cache certainly wouldn't
be interested in a generic helper anyway.  All of our pagecache assumes
blocks between 512<->PAGE_CACHE_SIZE.
	If I change the BUG_ON()s to -EINVAL, does that work?  Or do you
have some way you'd like to allow non-pagecache filesystems to use this
as well?

Joel

-- 

"I am working for the time when unqualified blacks, browns, and
 women join the unqualified men in running our government."
	- Sissy Farenthold

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Thu, Aug 12, 2010 at 12:45:41PM -0600, Andreas Dilger wrote:
> On 2010-08-12, at 11:42, Joel Becker wrote:
> >> +/**
> >> + * generic_check_addressable - Check addressability of file system
> >> + * @blocksize_bits:	log of file system block size
> >> + * @num_blocks:		number of blocks in file system
> >> + *
> >> + * Determine whether a file system with @num_blocks blocks (and a
> >> + * block size of 2**@blocksize_bits) is addressable by the sector_t
> >> + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
> >> + */
> >> +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
> >> +{
> >> +	u64 last_fs_block = num_blocks - 1;
> >> +
> >> +	BUG_ON(blocksize_bits < 9);
> >> +	BUG_ON(blocksize_bits > PAGE_CACHE_SHIFT);
> 
> I'd rather not have a BUG_ON() for a "check" function that may be called with on-disk values by some filesystem.  Some filesystems (AFAIR) also handle blocksize > PAGE_SIZE internally, so this helper would not be useful for them.

	Filesystems that handle their own page cache certainly wouldn't
be interested in a generic helper anyway.  All of our pagecache assumes
blocks between 512<->PAGE_CACHE_SIZE.
	If I change the BUG_ON()s to -EINVAL, does that work?  Or do you
have some way you'd like to allow non-pagecache filesystems to use this
as well?

Joel

-- 

"I am working for the time when unqualified blacks, browns, and
 women join the unqualified men in running our government."
	- Sissy Farenthold

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Andreas Dilger]

On 2010-08-12, at 14:15, Joel Becker wrote:

> On Thu, Aug 12, 2010 at 12:45:41PM -0600, Andreas Dilger wrote:
>> On 2010-08-12, at 11:42, Joel Becker wrote:
>>>> +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
>>>> +{
>>>> +	u64 last_fs_block = num_blocks - 1;
>>>> +
>>>> +	BUG_ON(blocksize_bits < 9);
>>>> +	BUG_ON(blocksize_bits > PAGE_CACHE_SHIFT);
>> 
>> I'd rather not have a BUG_ON() for a "check" function that may be called with on-disk values by some filesystem.  Some filesystems (AFAIR) also handle blocksize > PAGE_SIZE internally, so this helper would not be useful for them.
> 
> 	Filesystems that handle their own page cache certainly wouldn't
> be interested in a generic helper anyway.  All of our pagecache assumes
> blocks between 512<->PAGE_CACHE_SIZE.
> 	If I change the BUG_ON()s to -EINVAL, does that work?  Or do you
> have some way you'd like to allow non-pagecache filesystems to use this
> as well?

That's probably fine for now.  If anyone complains, we can always change it later, since they can't possibly depend on this function yet...

Cheers, Andreas

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Andreas Dilger]

On 2010-08-12, at 14:15, Joel Becker wrote:

> On Thu, Aug 12, 2010 at 12:45:41PM -0600, Andreas Dilger wrote:
>> On 2010-08-12, at 11:42, Joel Becker wrote:
>>>> +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
>>>> +{
>>>> +	u64 last_fs_block = num_blocks - 1;
>>>> +
>>>> +	BUG_ON(blocksize_bits < 9);
>>>> +	BUG_ON(blocksize_bits > PAGE_CACHE_SHIFT);
>> 
>> I'd rather not have a BUG_ON() for a "check" function that may be called with on-disk values by some filesystem.  Some filesystems (AFAIR) also handle blocksize > PAGE_SIZE internally, so this helper would not be useful for them.
> 
> 	Filesystems that handle their own page cache certainly wouldn't
> be interested in a generic helper anyway.  All of our pagecache assumes
> blocks between 512<->PAGE_CACHE_SIZE.
> 	If I change the BUG_ON()s to -EINVAL, does that work?  Or do you
> have some way you'd like to allow non-pagecache filesystems to use this
> as well?

That's probably fine for now.  If anyone complains, we can always change it later, since they can't possibly depend on this function yet...

Cheers, Andreas

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Thu, Aug 12, 2010 at 03:32:27PM -0600, Andreas Dilger wrote:
> > 	If I change the BUG_ON()s to -EINVAL, does that work?  Or do you
> > have some way you'd like to allow non-pagecache filesystems to use this
> > as well?
> 
> That's probably fine for now.  If anyone complains, we can always change it later, since they can't possibly depend on this function yet...

	How's this:

>From f988b04c58039ae9a65fea537656088ea56a8072 Mon Sep 17 00:00:00 2001
>From: Patrick J. LoPresti <lopresti@gmail.com>
Date: Thu, 22 Jul 2010 15:03:41 -0700
Subject: [PATCH] ext3/ext4: Factor out disk addressability check

As part of adding support for OCFS2 to mount huge volumes, we need to
check that the sector_t and page cache of the system are capable of
addressing the entire volume.

An identical check already appears in ext3 and ext4.  This patch moves
the addressability check into its own function in fs/libfs.c and
modifies ext3 and ext4 to invoke it.

[Edited to -EINVAL instead of BUG_ON() for bad blocksize_bits -- Joel]

Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
Cc: linux-ext4@vger.kernel.org
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/ext3/super.c    |    4 ++--
 fs/ext4/super.c    |    8 +++-----
 fs/libfs.c         |   29 +++++++++++++++++++++++++++++
 include/linux/fs.h |    2 ++
 4 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index 6c953bb..d0643db 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -1862,8 +1862,8 @@ static int ext3_fill_super (struct super_block *sb, void *data, int silent)
 		goto failed_mount;
 	}
 
-	if (le32_to_cpu(es->s_blocks_count) >
-		    (sector_t)(~0ULL) >> (sb->s_blocksize_bits - 9)) {
+	if (generic_check_addressable(sb->s_blocksize_bits,
+				      le32_to_cpu(es->s_blocks_count))) {
 		ext3_msg(sb, KERN_ERR,
 			"error: filesystem is too large to mount safely");
 		if (sizeof(sector_t) < 8)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index e72d323..e03a7d2 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2706,15 +2706,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	 * Test whether we have more sectors than will fit in sector_t,
 	 * and whether the max offset is addressable by the page cache.
 	 */
-	if ((ext4_blocks_count(es) >
-	     (sector_t)(~0ULL) >> (sb->s_blocksize_bits - 9)) ||
-	    (ext4_blocks_count(es) >
-	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - sb->s_blocksize_bits))) {
+	ret = generic_check_addressable(sb->s_blocksize_bits,
+					ext4_blocks_count(es));
+	if (ret) {
 		ext4_msg(sb, KERN_ERR, "filesystem"
 			 " too large to mount safely on this system");
 		if (sizeof(sector_t) < 8)
 			ext4_msg(sb, KERN_WARNING, "CONFIG_LBDAF not enabled");
-		ret = -EFBIG;
 		goto failed_mount;
 	}
 
diff --git a/fs/libfs.c b/fs/libfs.c
index dcaf972..f099566 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -955,6 +955,35 @@ int generic_file_fsync(struct file *file, int datasync)
 }
 EXPORT_SYMBOL(generic_file_fsync);
 
+/**
+ * generic_check_addressable - Check addressability of file system
+ * @blocksize_bits:	log of file system block size
+ * @num_blocks:		number of blocks in file system
+ *
+ * Determine whether a file system with @num_blocks blocks (and a
+ * block size of 2**@blocksize_bits) is addressable by the sector_t
+ * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
+ */
+int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
+{
+	u64 last_fs_block = num_blocks - 1;
+
+	if (unlikely(num_blocks == 0))
+		return 0;
+
+	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
+		return -EINVAL;
+
+	if ((last_fs_block >
+	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
+	    (last_fs_block >
+	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
+		return -EFBIG;
+	}
+	return 0;
+}
+EXPORT_SYMBOL(generic_check_addressable);
+
 /*
  * No-op implementation of ->fsync for in-memory filesystems.
  */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e5106e4..7644248 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2414,6 +2414,8 @@ extern ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
 
 extern int generic_file_fsync(struct file *, int);
 
+extern int generic_check_addressable(unsigned, u64);
+
 #ifdef CONFIG_MIGRATION
 extern int buffer_migrate_page(struct address_space *,
 				struct page *, struct page *);
-- 
1.7.1


-- 

"Friends may come and go, but enemies accumulate." 
        - Thomas Jones

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Thu, Aug 12, 2010 at 03:32:27PM -0600, Andreas Dilger wrote:
> > 	If I change the BUG_ON()s to -EINVAL, does that work?  Or do you
> > have some way you'd like to allow non-pagecache filesystems to use this
> > as well?
> 
> That's probably fine for now.  If anyone complains, we can always change it later, since they can't possibly depend on this function yet...

	How's this:

From f988b04c58039ae9a65fea537656088ea56a8072 Mon Sep 17 00:00:00 2001
>From: Patrick J. LoPresti <lopresti@gmail.com>
Date: Thu, 22 Jul 2010 15:03:41 -0700
Subject: [PATCH] ext3/ext4: Factor out disk addressability check

As part of adding support for OCFS2 to mount huge volumes, we need to
check that the sector_t and page cache of the system are capable of
addressing the entire volume.

An identical check already appears in ext3 and ext4.  This patch moves
the addressability check into its own function in fs/libfs.c and
modifies ext3 and ext4 to invoke it.

[Edited to -EINVAL instead of BUG_ON() for bad blocksize_bits -- Joel]

Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
Cc: linux-ext4 at vger.kernel.org
Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/ext3/super.c    |    4 ++--
 fs/ext4/super.c    |    8 +++-----
 fs/libfs.c         |   29 +++++++++++++++++++++++++++++
 include/linux/fs.h |    2 ++
 4 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index 6c953bb..d0643db 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -1862,8 +1862,8 @@ static int ext3_fill_super (struct super_block *sb, void *data, int silent)
 		goto failed_mount;
 	}
 
-	if (le32_to_cpu(es->s_blocks_count) >
-		    (sector_t)(~0ULL) >> (sb->s_blocksize_bits - 9)) {
+	if (generic_check_addressable(sb->s_blocksize_bits,
+				      le32_to_cpu(es->s_blocks_count))) {
 		ext3_msg(sb, KERN_ERR,
 			"error: filesystem is too large to mount safely");
 		if (sizeof(sector_t) < 8)
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index e72d323..e03a7d2 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2706,15 +2706,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	 * Test whether we have more sectors than will fit in sector_t,
 	 * and whether the max offset is addressable by the page cache.
 	 */
-	if ((ext4_blocks_count(es) >
-	     (sector_t)(~0ULL) >> (sb->s_blocksize_bits - 9)) ||
-	    (ext4_blocks_count(es) >
-	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - sb->s_blocksize_bits))) {
+	ret = generic_check_addressable(sb->s_blocksize_bits,
+					ext4_blocks_count(es));
+	if (ret) {
 		ext4_msg(sb, KERN_ERR, "filesystem"
 			 " too large to mount safely on this system");
 		if (sizeof(sector_t) < 8)
 			ext4_msg(sb, KERN_WARNING, "CONFIG_LBDAF not enabled");
-		ret = -EFBIG;
 		goto failed_mount;
 	}
 
diff --git a/fs/libfs.c b/fs/libfs.c
index dcaf972..f099566 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -955,6 +955,35 @@ int generic_file_fsync(struct file *file, int datasync)
 }
 EXPORT_SYMBOL(generic_file_fsync);
 
+/**
+ * generic_check_addressable - Check addressability of file system
+ * @blocksize_bits:	log of file system block size
+ * @num_blocks:		number of blocks in file system
+ *
+ * Determine whether a file system with @num_blocks blocks (and a
+ * block size of 2**@blocksize_bits) is addressable by the sector_t
+ * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
+ */
+int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
+{
+	u64 last_fs_block = num_blocks - 1;
+
+	if (unlikely(num_blocks == 0))
+		return 0;
+
+	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
+		return -EINVAL;
+
+	if ((last_fs_block >
+	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
+	    (last_fs_block >
+	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
+		return -EFBIG;
+	}
+	return 0;
+}
+EXPORT_SYMBOL(generic_check_addressable);
+
 /*
  * No-op implementation of ->fsync for in-memory filesystems.
  */
diff --git a/include/linux/fs.h b/include/linux/fs.h
index e5106e4..7644248 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2414,6 +2414,8 @@ extern ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
 
 extern int generic_file_fsync(struct file *, int);
 
+extern int generic_check_addressable(unsigned, u64);
+
 #ifdef CONFIG_MIGRATION
 extern int buffer_migrate_page(struct address_space *,
 				struct page *, struct page *);
-- 
1.7.1


-- 

"Friends may come and go, but enemies accumulate." 
        - Thomas Jones

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

Re: [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Thu, Aug 12, 2010 at 10:42:16AM -0700, Joel Becker wrote:
> On Thu, Jul 22, 2010 at 03:03:41PM -0700, Patrick J. LoPresti wrote:
> > As part of adding support for OCFS2 to mount huge volumes, we need to
> > check that the sector_t and page cache of the system are capable of
> > addressing the entire volume.
> > 
> > An identical check already appears in ext3 and ext4.  This patch moves
> > the addressability check into its own function in fs/libfs.c and
> > modifies ext3 and ext4 to invoke it.
> > 
> > Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> 
> Dear ext3/4 folks,
> 	I've pushed this patch to the merge-window branch of ocfs2.git.
> I'm ready to send it to Linus, But I need your OK.

	To be specific, I would like an Acked-by.

Joel

-- 

Life's Little Instruction Book #456

	"Send your loved one flowers.  Think of a reason later."

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Thu, Aug 12, 2010 at 10:42:16AM -0700, Joel Becker wrote:
> On Thu, Jul 22, 2010 at 03:03:41PM -0700, Patrick J. LoPresti wrote:
> > As part of adding support for OCFS2 to mount huge volumes, we need to
> > check that the sector_t and page cache of the system are capable of
> > addressing the entire volume.
> > 
> > An identical check already appears in ext3 and ext4.  This patch moves
> > the addressability check into its own function in fs/libfs.c and
> > modifies ext3 and ext4 to invoke it.
> > 
> > Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> 
> Dear ext3/4 folks,
> 	I've pushed this patch to the merge-window branch of ocfs2.git.
> I'm ready to send it to Linus, But I need your OK.

	To be specific, I would like an Acked-by.

Joel

-- 

Life's Little Instruction Book #456

	"Send your loved one flowers.  Think of a reason later."

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

Re: [Ocfs2-devel] [PATCH 2/3] JBD2: Allow feature checks before journal recovery

[Joel Becker]

On Thu, Aug 12, 2010 at 10:43:19AM -0700, Joel Becker wrote:
> On Thu, Jul 22, 2010 at 03:04:16PM -0700, Patrick J. LoPresti wrote:
> > Before we start accessing a huge (> 16 TiB) OCFS2 volume, we need to
> > confirm that its journal supports 64-bit offsets.  In particular, we
> > need to check the journal's feature bits before recovering the journal.
> > 
> > This is not possible with JBD2 at present, because the journal
> > superblock (where the feature bits reside) is not loaded from disk until
> > the journal is recovered.
> > 
> > This patch loads the journal superblock in
> > jbd2_journal_check_used_features() if it has not already been loaded,
> > allowing us to check the feature bits before journal recovery.
> > 
> > Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> 
> Dear jbd2 developers,
> 	I've pushed this patch to the merge-window branch of ocfs2.git.
> I'm ready to send it to Linus, but I need your OK.

	To be specific, I'd really like an Acked-by.

Joel

-- 

"The doctrine of human equality reposes on this: that there is no
 man really clever who has not found that he is stupid."
	- Gilbert K. Chesterson

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

Re: [PATCH 2/3] JBD2: Allow feature checks before journal recovery

[Joel Becker]

On Thu, Aug 12, 2010 at 10:43:19AM -0700, Joel Becker wrote:
> On Thu, Jul 22, 2010 at 03:04:16PM -0700, Patrick J. LoPresti wrote:
> > Before we start accessing a huge (> 16 TiB) OCFS2 volume, we need to
> > confirm that its journal supports 64-bit offsets.  In particular, we
> > need to check the journal's feature bits before recovering the journal.
> > 
> > This is not possible with JBD2 at present, because the journal
> > superblock (where the feature bits reside) is not loaded from disk until
> > the journal is recovered.
> > 
> > This patch loads the journal superblock in
> > jbd2_journal_check_used_features() if it has not already been loaded,
> > allowing us to check the feature bits before journal recovery.
> > 
> > Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> 
> Dear jbd2 developers,
> 	I've pushed this patch to the merge-window branch of ocfs2.git.
> I'm ready to send it to Linus, but I need your OK.

	To be specific, I'd really like an Acked-by.

Joel

-- 

"The doctrine of human equality reposes on this: that there is no
 man really clever who has not found that he is stupid."
	- Gilbert K. Chesterson

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

Re: [Ocfs2-devel] [PATCH 2/3] JBD2: Allow feature checks before journal recovery

[Joel Becker]

On Thu, Aug 12, 2010 at 10:43:19AM -0700, Joel Becker wrote:
> On Thu, Jul 22, 2010 at 03:04:16PM -0700, Patrick J. LoPresti wrote:
> > Before we start accessing a huge (> 16 TiB) OCFS2 volume, we need to
> > confirm that its journal supports 64-bit offsets.  In particular, we
> > need to check the journal's feature bits before recovering the journal.
> > 
> > This is not possible with JBD2 at present, because the journal
> > superblock (where the feature bits reside) is not loaded from disk until
> > the journal is recovered.
> > 
> > This patch loads the journal superblock in
> > jbd2_journal_check_used_features() if it has not already been loaded,
> > allowing us to check the feature bits before journal recovery.
> > 
> > Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> 
> Dear jbd2 developers,
> 	I've pushed this patch to the merge-window branch of ocfs2.git.
> I'm ready to send it to Linus, but I need your OK.

	To be specific, I'd really like an Acked-by.

Joel

-- 

"The doctrine of human equality reposes on this: that there is no
 man really clever who has not found that he is stupid."
	- Gilbert K. Chesterson

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

Re: [Ocfs2-devel] [PATCH 2/3] JBD2: Allow feature checks before journal recovery

[Joel Becker]

On Thu, Aug 12, 2010 at 10:43:19AM -0700, Joel Becker wrote:
> On Thu, Jul 22, 2010 at 03:04:16PM -0700, Patrick J. LoPresti wrote:
> > Before we start accessing a huge (> 16 TiB) OCFS2 volume, we need to
> > confirm that its journal supports 64-bit offsets.  In particular, we
> > need to check the journal's feature bits before recovering the journal.
> > 
> > This is not possible with JBD2 at present, because the journal
> > superblock (where the feature bits reside) is not loaded from disk until
> > the journal is recovered.
> > 
> > This patch loads the journal superblock in
> > jbd2_journal_check_used_features() if it has not already been loaded,
> > allowing us to check the feature bits before journal recovery.
> > 
> > Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> 
> Dear jbd2 developers,
> 	I've pushed this patch to the merge-window branch of ocfs2.git.
> I'm ready to send it to Linus, but I need your OK.

	To be specific, I'd really like an Acked-by.

Joel

-- 

"The doctrine of human equality reposes on this: that there is no
 man really clever who has not found that he is stupid."
	- Gilbert K. Chesterson

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 2/3] JBD2: Allow feature checks before journal recovery

[Joel Becker]

On Thu, Aug 12, 2010 at 10:43:19AM -0700, Joel Becker wrote:
> On Thu, Jul 22, 2010 at 03:04:16PM -0700, Patrick J. LoPresti wrote:
> > Before we start accessing a huge (> 16 TiB) OCFS2 volume, we need to
> > confirm that its journal supports 64-bit offsets.  In particular, we
> > need to check the journal's feature bits before recovering the journal.
> > 
> > This is not possible with JBD2 at present, because the journal
> > superblock (where the feature bits reside) is not loaded from disk until
> > the journal is recovered.
> > 
> > This patch loads the journal superblock in
> > jbd2_journal_check_used_features() if it has not already been loaded,
> > allowing us to check the feature bits before journal recovery.
> > 
> > Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> 
> Dear jbd2 developers,
> 	I've pushed this patch to the merge-window branch of ocfs2.git.
> I'm ready to send it to Linus, but I need your OK.

	To be specific, I'd really like an Acked-by.

Joel

-- 

"The doctrine of human equality reposes on this: that there is no
 man really clever who has not found that he is stupid."
	- Gilbert K. Chesterson

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Andreas Dilger]

You can add my:

Acked-by: Andreas Dilger <adilger@dilger.ca>

On 2010-08-12, at 16:29, Joel Becker wrote:

> On Thu, Aug 12, 2010 at 03:32:27PM -0600, Andreas Dilger wrote:
>>> 	If I change the BUG_ON()s to -EINVAL, does that work?  Or do you
>>> have some way you'd like to allow non-pagecache filesystems to use this
>>> as well?
>> 
>> That's probably fine for now.  If anyone complains, we can always change it later, since they can't possibly depend on this function yet...
> 
> 	How's this:
> 
>> From f988b04c58039ae9a65fea537656088ea56a8072 Mon Sep 17 00:00:00 2001
>> From: Patrick J. LoPresti <lopresti@gmail.com>
> Date: Thu, 22 Jul 2010 15:03:41 -0700
> Subject: [PATCH] ext3/ext4: Factor out disk addressability check
> 
> As part of adding support for OCFS2 to mount huge volumes, we need to
> check that the sector_t and page cache of the system are capable of
> addressing the entire volume.
> 
> An identical check already appears in ext3 and ext4.  This patch moves
> the addressability check into its own function in fs/libfs.c and
> modifies ext3 and ext4 to invoke it.
> 
> [Edited to -EINVAL instead of BUG_ON() for bad blocksize_bits -- Joel]
> 
> Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> Cc: linux-ext4@vger.kernel.org
> Signed-off-by: Joel Becker <joel.becker@oracle.com>
> ---
> fs/ext3/super.c    |    4 ++--
> fs/ext4/super.c    |    8 +++-----
> fs/libfs.c         |   29 +++++++++++++++++++++++++++++
> include/linux/fs.h |    2 ++
> 4 files changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/ext3/super.c b/fs/ext3/super.c
> index 6c953bb..d0643db 100644
> --- a/fs/ext3/super.c
> +++ b/fs/ext3/super.c
> @@ -1862,8 +1862,8 @@ static int ext3_fill_super (struct super_block *sb, void *data, int silent)
> 		goto failed_mount;
> 	}
> 
> -	if (le32_to_cpu(es->s_blocks_count) >
> -		    (sector_t)(~0ULL) >> (sb->s_blocksize_bits - 9)) {
> +	if (generic_check_addressable(sb->s_blocksize_bits,
> +				      le32_to_cpu(es->s_blocks_count))) {
> 		ext3_msg(sb, KERN_ERR,
> 			"error: filesystem is too large to mount safely");
> 		if (sizeof(sector_t) < 8)
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index e72d323..e03a7d2 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2706,15 +2706,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> 	 * Test whether we have more sectors than will fit in sector_t,
> 	 * and whether the max offset is addressable by the page cache.
> 	 */
> -	if ((ext4_blocks_count(es) >
> -	     (sector_t)(~0ULL) >> (sb->s_blocksize_bits - 9)) ||
> -	    (ext4_blocks_count(es) >
> -	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - sb->s_blocksize_bits))) {
> +	ret = generic_check_addressable(sb->s_blocksize_bits,
> +					ext4_blocks_count(es));
> +	if (ret) {
> 		ext4_msg(sb, KERN_ERR, "filesystem"
> 			 " too large to mount safely on this system");
> 		if (sizeof(sector_t) < 8)
> 			ext4_msg(sb, KERN_WARNING, "CONFIG_LBDAF not enabled");
> -		ret = -EFBIG;
> 		goto failed_mount;
> 	}
> 
> diff --git a/fs/libfs.c b/fs/libfs.c
> index dcaf972..f099566 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -955,6 +955,35 @@ int generic_file_fsync(struct file *file, int datasync)
> }
> EXPORT_SYMBOL(generic_file_fsync);
> 
> +/**
> + * generic_check_addressable - Check addressability of file system
> + * @blocksize_bits:	log of file system block size
> + * @num_blocks:		number of blocks in file system
> + *
> + * Determine whether a file system with @num_blocks blocks (and a
> + * block size of 2**@blocksize_bits) is addressable by the sector_t
> + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
> + */
> +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
> +{
> +	u64 last_fs_block = num_blocks - 1;
> +
> +	if (unlikely(num_blocks == 0))
> +		return 0;
> +
> +	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
> +		return -EINVAL;
> +
> +	if ((last_fs_block >
> +	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
> +	    (last_fs_block >
> +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
> +		return -EFBIG;
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL(generic_check_addressable);
> +
> /*
>  * No-op implementation of ->fsync for in-memory filesystems.
>  */
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index e5106e4..7644248 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2414,6 +2414,8 @@ extern ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
> 
> extern int generic_file_fsync(struct file *, int);
> 
> +extern int generic_check_addressable(unsigned, u64);
> +
> #ifdef CONFIG_MIGRATION
> extern int buffer_migrate_page(struct address_space *,
> 				struct page *, struct page *);
> -- 
> 1.7.1
> 
> 
> -- 
> 
> "Friends may come and go, but enemies accumulate." 
>        - Thomas Jones
> 
> Joel Becker
> Consulting Software Developer
> Oracle
> E-mail: joel.becker@oracle.com
> Phone: (650) 506-8127
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers, Andreas

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Andreas Dilger]

You can add my:

Acked-by: Andreas Dilger <adilger@dilger.ca>

On 2010-08-12, at 16:29, Joel Becker wrote:

> On Thu, Aug 12, 2010 at 03:32:27PM -0600, Andreas Dilger wrote:
>>> 	If I change the BUG_ON()s to -EINVAL, does that work?  Or do you
>>> have some way you'd like to allow non-pagecache filesystems to use this
>>> as well?
>> 
>> That's probably fine for now.  If anyone complains, we can always change it later, since they can't possibly depend on this function yet...
> 
> 	How's this:
> 
>> From f988b04c58039ae9a65fea537656088ea56a8072 Mon Sep 17 00:00:00 2001
>> From: Patrick J. LoPresti <lopresti@gmail.com>
> Date: Thu, 22 Jul 2010 15:03:41 -0700
> Subject: [PATCH] ext3/ext4: Factor out disk addressability check
> 
> As part of adding support for OCFS2 to mount huge volumes, we need to
> check that the sector_t and page cache of the system are capable of
> addressing the entire volume.
> 
> An identical check already appears in ext3 and ext4.  This patch moves
> the addressability check into its own function in fs/libfs.c and
> modifies ext3 and ext4 to invoke it.
> 
> [Edited to -EINVAL instead of BUG_ON() for bad blocksize_bits -- Joel]
> 
> Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> Cc: linux-ext4 at vger.kernel.org
> Signed-off-by: Joel Becker <joel.becker@oracle.com>
> ---
> fs/ext3/super.c    |    4 ++--
> fs/ext4/super.c    |    8 +++-----
> fs/libfs.c         |   29 +++++++++++++++++++++++++++++
> include/linux/fs.h |    2 ++
> 4 files changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/ext3/super.c b/fs/ext3/super.c
> index 6c953bb..d0643db 100644
> --- a/fs/ext3/super.c
> +++ b/fs/ext3/super.c
> @@ -1862,8 +1862,8 @@ static int ext3_fill_super (struct super_block *sb, void *data, int silent)
> 		goto failed_mount;
> 	}
> 
> -	if (le32_to_cpu(es->s_blocks_count) >
> -		    (sector_t)(~0ULL) >> (sb->s_blocksize_bits - 9)) {
> +	if (generic_check_addressable(sb->s_blocksize_bits,
> +				      le32_to_cpu(es->s_blocks_count))) {
> 		ext3_msg(sb, KERN_ERR,
> 			"error: filesystem is too large to mount safely");
> 		if (sizeof(sector_t) < 8)
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index e72d323..e03a7d2 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -2706,15 +2706,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
> 	 * Test whether we have more sectors than will fit in sector_t,
> 	 * and whether the max offset is addressable by the page cache.
> 	 */
> -	if ((ext4_blocks_count(es) >
> -	     (sector_t)(~0ULL) >> (sb->s_blocksize_bits - 9)) ||
> -	    (ext4_blocks_count(es) >
> -	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - sb->s_blocksize_bits))) {
> +	ret = generic_check_addressable(sb->s_blocksize_bits,
> +					ext4_blocks_count(es));
> +	if (ret) {
> 		ext4_msg(sb, KERN_ERR, "filesystem"
> 			 " too large to mount safely on this system");
> 		if (sizeof(sector_t) < 8)
> 			ext4_msg(sb, KERN_WARNING, "CONFIG_LBDAF not enabled");
> -		ret = -EFBIG;
> 		goto failed_mount;
> 	}
> 
> diff --git a/fs/libfs.c b/fs/libfs.c
> index dcaf972..f099566 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -955,6 +955,35 @@ int generic_file_fsync(struct file *file, int datasync)
> }
> EXPORT_SYMBOL(generic_file_fsync);
> 
> +/**
> + * generic_check_addressable - Check addressability of file system
> + * @blocksize_bits:	log of file system block size
> + * @num_blocks:		number of blocks in file system
> + *
> + * Determine whether a file system with @num_blocks blocks (and a
> + * block size of 2**@blocksize_bits) is addressable by the sector_t
> + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
> + */
> +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
> +{
> +	u64 last_fs_block = num_blocks - 1;
> +
> +	if (unlikely(num_blocks == 0))
> +		return 0;
> +
> +	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
> +		return -EINVAL;
> +
> +	if ((last_fs_block >
> +	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
> +	    (last_fs_block >
> +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
> +		return -EFBIG;
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL(generic_check_addressable);
> +
> /*
>  * No-op implementation of ->fsync for in-memory filesystems.
>  */
> diff --git a/include/linux/fs.h b/include/linux/fs.h
> index e5106e4..7644248 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -2414,6 +2414,8 @@ extern ssize_t simple_write_to_buffer(void *to, size_t available, loff_t *ppos,
> 
> extern int generic_file_fsync(struct file *, int);
> 
> +extern int generic_check_addressable(unsigned, u64);
> +
> #ifdef CONFIG_MIGRATION
> extern int buffer_migrate_page(struct address_space *,
> 				struct page *, struct page *);
> -- 
> 1.7.1
> 
> 
> -- 
> 
> "Friends may come and go, but enemies accumulate." 
>        - Thomas Jones
> 
> Joel Becker
> Consulting Software Developer
> Oracle
> E-mail: joel.becker at oracle.com
> Phone: (650) 506-8127
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html


Cheers, Andreas

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Thu, Aug 12, 2010 at 05:07:36PM -0600, Andreas Dilger wrote:
> You can add my:
> 
> Acked-by: Andreas Dilger <adilger@dilger.ca>

	Thanks!

Joel

-- 

 There are morethings in heaven and earth, Horatio,
 Than are dreamt of in your philosophy.

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Thu, Aug 12, 2010 at 05:07:36PM -0600, Andreas Dilger wrote:
> You can add my:
> 
> Acked-by: Andreas Dilger <adilger@dilger.ca>

	Thanks!

Joel

-- 

 There are morethings in heaven and earth, Horatio,
 Than are dreamt of in your philosophy.

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Ted Ts'o]

On Thu, Aug 12, 2010 at 10:42:16AM -0700, Joel Becker wrote:
> On Thu, Jul 22, 2010 at 03:03:41PM -0700, Patrick J. LoPresti wrote:
> > As part of adding support for OCFS2 to mount huge volumes, we need to
> > check that the sector_t and page cache of the system are capable of
> > addressing the entire volume.
> > 
> > An identical check already appears in ext3 and ext4.  This patch moves
> > the addressability check into its own function in fs/libfs.c and
> > modifies ext3 and ext4 to invoke it.
> > 
> > Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> 
> Dear ext3/4 folks,
> 	I've pushed this patch to the merge-window branch of ocfs2.git.
> I'm ready to send it to Linus, But I need your OK.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

for the ext4 bits.

(Sorry for not responding earlier to some of these patches.  The fact
that the merge window overlapped with LinuxCon and the Linux
Storage/Filesystem workshop has been quite stressful/tiring for some
of us....)

					- Ted

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Ted Ts'o]

On Thu, Aug 12, 2010 at 10:42:16AM -0700, Joel Becker wrote:
> On Thu, Jul 22, 2010 at 03:03:41PM -0700, Patrick J. LoPresti wrote:
> > As part of adding support for OCFS2 to mount huge volumes, we need to
> > check that the sector_t and page cache of the system are capable of
> > addressing the entire volume.
> > 
> > An identical check already appears in ext3 and ext4.  This patch moves
> > the addressability check into its own function in fs/libfs.c and
> > modifies ext3 and ext4 to invoke it.
> > 
> > Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> 
> Dear ext3/4 folks,
> 	I've pushed this patch to the merge-window branch of ocfs2.git.
> I'm ready to send it to Linus, But I need your OK.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

for the ext4 bits.

(Sorry for not responding earlier to some of these patches.  The fact
that the merge window overlapped with LinuxCon and the Linux
Storage/Filesystem workshop has been quite stressful/tiring for some
of us....)

					- Ted

Re: [Ocfs2-devel] [PATCH 2/3] JBD2: Allow feature checks before journal recovery

[Ted Ts'o]

On Thu, Aug 12, 2010 at 10:43:19AM -0700, Joel Becker wrote:
> On Thu, Jul 22, 2010 at 03:04:16PM -0700, Patrick J. LoPresti wrote:
> > Before we start accessing a huge (> 16 TiB) OCFS2 volume, we need to
> > confirm that its journal supports 64-bit offsets.  In particular, we
> > need to check the journal's feature bits before recovering the journal.
> > 
> > This is not possible with JBD2 at present, because the journal
> > superblock (where the feature bits reside) is not loaded from disk until
> > the journal is recovered.
> > 
> > This patch loads the journal superblock in
> > jbd2_journal_check_used_features() if it has not already been loaded,
> > allowing us to check the feature bits before journal recovery.
> > 
> > Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> 
> Dear jbd2 developers,
> 	I've pushed this patch to the merge-window branch of ocfs2.git.
> I'm ready to send it to Linus, but I need your OK.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

	       		       - Ted

[Ocfs2-devel] [PATCH 2/3] JBD2: Allow feature checks before journal recovery

[Ted Ts'o]

On Thu, Aug 12, 2010 at 10:43:19AM -0700, Joel Becker wrote:
> On Thu, Jul 22, 2010 at 03:04:16PM -0700, Patrick J. LoPresti wrote:
> > Before we start accessing a huge (> 16 TiB) OCFS2 volume, we need to
> > confirm that its journal supports 64-bit offsets.  In particular, we
> > need to check the journal's feature bits before recovering the journal.
> > 
> > This is not possible with JBD2 at present, because the journal
> > superblock (where the feature bits reside) is not loaded from disk until
> > the journal is recovered.
> > 
> > This patch loads the journal superblock in
> > jbd2_journal_check_used_features() if it has not already been loaded,
> > allowing us to check the feature bits before journal recovery.
> > 
> > Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> 
> Dear jbd2 developers,
> 	I've pushed this patch to the merge-window branch of ocfs2.git.
> I'm ready to send it to Linus, but I need your OK.

Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

	       		       - Ted

Re: [Ocfs2-devel] [PATCH 2/3] JBD2: Allow feature checks before journal recovery

[Joel Becker]

On Thu, Aug 12, 2010 at 11:39:30PM -0400, Ted Ts'o wrote:
> On Thu, Aug 12, 2010 at 10:43:19AM -0700, Joel Becker wrote:
> > On Thu, Jul 22, 2010 at 03:04:16PM -0700, Patrick J. LoPresti wrote:
> > > Before we start accessing a huge (> 16 TiB) OCFS2 volume, we need to
> > > confirm that its journal supports 64-bit offsets.  In particular, we
> > > need to check the journal's feature bits before recovering the journal.
> > > 
> > > This is not possible with JBD2 at present, because the journal
> > > superblock (where the feature bits reside) is not loaded from disk until
> > > the journal is recovered.
> > > 
> > > This patch loads the journal superblock in
> > > jbd2_journal_check_used_features() if it has not already been loaded,
> > > allowing us to check the feature bits before journal recovery.
> > > 
> > > Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> > 
> > Dear jbd2 developers,
> > 	I've pushed this patch to the merge-window branch of ocfs2.git.
> > I'm ready to send it to Linus, but I need your OK.
> 
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

	Thanks Ted!

Joel

-- 

"I don't even butter my bread; I consider that cooking."
         - Katherine Cebrian

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

Re: [Ocfs2-devel] [PATCH 2/3] JBD2: Allow feature checks before journal recovery

[Joel Becker]

On Thu, Aug 12, 2010 at 11:39:30PM -0400, Ted Ts'o wrote:
> On Thu, Aug 12, 2010 at 10:43:19AM -0700, Joel Becker wrote:
> > On Thu, Jul 22, 2010 at 03:04:16PM -0700, Patrick J. LoPresti wrote:
> > > Before we start accessing a huge (> 16 TiB) OCFS2 volume, we need to
> > > confirm that its journal supports 64-bit offsets.  In particular, we
> > > need to check the journal's feature bits before recovering the journal.
> > > 
> > > This is not possible with JBD2 at present, because the journal
> > > superblock (where the feature bits reside) is not loaded from disk until
> > > the journal is recovered.
> > > 
> > > This patch loads the journal superblock in
> > > jbd2_journal_check_used_features() if it has not already been loaded,
> > > allowing us to check the feature bits before journal recovery.
> > > 
> > > Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> > 
> > Dear jbd2 developers,
> > 	I've pushed this patch to the merge-window branch of ocfs2.git.
> > I'm ready to send it to Linus, but I need your OK.
> 
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

	Thanks Ted!

Joel

-- 

"I don't even butter my bread; I consider that cooking."
         - Katherine Cebrian

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 2/3] JBD2: Allow feature checks before journal recovery

[Joel Becker]

On Thu, Aug 12, 2010 at 11:39:30PM -0400, Ted Ts'o wrote:
> On Thu, Aug 12, 2010 at 10:43:19AM -0700, Joel Becker wrote:
> > On Thu, Jul 22, 2010 at 03:04:16PM -0700, Patrick J. LoPresti wrote:
> > > Before we start accessing a huge (> 16 TiB) OCFS2 volume, we need to
> > > confirm that its journal supports 64-bit offsets.  In particular, we
> > > need to check the journal's feature bits before recovering the journal.
> > > 
> > > This is not possible with JBD2 at present, because the journal
> > > superblock (where the feature bits reside) is not loaded from disk until
> > > the journal is recovered.
> > > 
> > > This patch loads the journal superblock in
> > > jbd2_journal_check_used_features() if it has not already been loaded,
> > > allowing us to check the feature bits before journal recovery.
> > > 
> > > Signed-off-by: Patrick LoPresti <lopresti@gmail.com>
> > 
> > Dear jbd2 developers,
> > 	I've pushed this patch to the merge-window branch of ocfs2.git.
> > I'm ready to send it to Linus, but I need your OK.
> 
> Signed-off-by: "Theodore Ts'o" <tytso@mit.edu>

	Thanks Ted!

Joel

-- 

"I don't even butter my bread; I consider that cooking."
         - Katherine Cebrian

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Jan Kara]

On Thu 12-08-10 15:29:49, Joel Becker wrote:
> diff --git a/fs/libfs.c b/fs/libfs.c
> index dcaf972..f099566 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -955,6 +955,35 @@ int generic_file_fsync(struct file *file, int datasync)
>  }
>  EXPORT_SYMBOL(generic_file_fsync);
>  
> +/**
> + * generic_check_addressable - Check addressability of file system
> + * @blocksize_bits:	log of file system block size
> + * @num_blocks:		number of blocks in file system
> + *
> + * Determine whether a file system with @num_blocks blocks (and a
> + * block size of 2**@blocksize_bits) is addressable by the sector_t
> + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
> + */
> +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
> +{
> +	u64 last_fs_block = num_blocks - 1;
> +
> +	if (unlikely(num_blocks == 0))
> +		return 0;
> +
> +	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
> +		return -EINVAL;
> +
> +	if ((last_fs_block >
> +	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
> +	    (last_fs_block >
> +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
            ^^^ I don't get the pgoff_t check. Shouldn't it rather be
(u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)?
Because on 32-bit arch we are able to address 16TB device, which is for 1KB
blocksize 1<<34 blocks. But your math gives 1<<30 blocks...

									Honza

-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Jan Kara]

On Thu 12-08-10 15:29:49, Joel Becker wrote:
> diff --git a/fs/libfs.c b/fs/libfs.c
> index dcaf972..f099566 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -955,6 +955,35 @@ int generic_file_fsync(struct file *file, int datasync)
>  }
>  EXPORT_SYMBOL(generic_file_fsync);
>  
> +/**
> + * generic_check_addressable - Check addressability of file system
> + * @blocksize_bits:	log of file system block size
> + * @num_blocks:		number of blocks in file system
> + *
> + * Determine whether a file system with @num_blocks blocks (and a
> + * block size of 2**@blocksize_bits) is addressable by the sector_t
> + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
> + */
> +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
> +{
> +	u64 last_fs_block = num_blocks - 1;
> +
> +	if (unlikely(num_blocks == 0))
> +		return 0;
> +
> +	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
> +		return -EINVAL;
> +
> +	if ((last_fs_block >
> +	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
> +	    (last_fs_block >
> +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
            ^^^ I don't get the pgoff_t check. Shouldn't it rather be
(u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)?
Because on 32-bit arch we are able to address 16TB device, which is for 1KB
blocksize 1<<34 blocks. But your math gives 1<<30 blocks...

									Honza

-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Fri, Aug 13, 2010 at 06:30:07PM +0200, Jan Kara wrote:
> On Thu 12-08-10 15:29:49, Joel Becker wrote:
> > +/**
> > + * generic_check_addressable - Check addressability of file system
> > + * @blocksize_bits:	log of file system block size
> > + * @num_blocks:		number of blocks in file system
> > + *
> > + * Determine whether a file system with @num_blocks blocks (and a
> > + * block size of 2**@blocksize_bits) is addressable by the sector_t
> > + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
> > + */
> > +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
> > +{
> > +	u64 last_fs_block = num_blocks - 1;
> > +
> > +	if (unlikely(num_blocks == 0))
> > +		return 0;
> > +
> > +	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
> > +		return -EINVAL;
> > +
> > +	if ((last_fs_block >
> > +	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
> > +	    (last_fs_block >
> > +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
>             ^^^ I don't get the pgoff_t check. Shouldn't it rather be
> (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)?
> Because on 32-bit arch we are able to address 16TB device, which is for 1KB
> blocksize 1<<34 blocks. But your math gives 1<<30 blocks...

	This code is directly lifted from ext4.  But that said, I am
starting to think you're right.  1 page == 4 x 1K blocks, rather than 4
pages == 1 1K block.

Joel

-- 

"I always thought the hardest questions were those I could not answer.
 Now I know they are the ones I can never ask."
			- Charlie Watkins

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Fri, Aug 13, 2010 at 06:30:07PM +0200, Jan Kara wrote:
> On Thu 12-08-10 15:29:49, Joel Becker wrote:
> > +/**
> > + * generic_check_addressable - Check addressability of file system
> > + * @blocksize_bits:	log of file system block size
> > + * @num_blocks:		number of blocks in file system
> > + *
> > + * Determine whether a file system with @num_blocks blocks (and a
> > + * block size of 2**@blocksize_bits) is addressable by the sector_t
> > + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
> > + */
> > +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
> > +{
> > +	u64 last_fs_block = num_blocks - 1;
> > +
> > +	if (unlikely(num_blocks == 0))
> > +		return 0;
> > +
> > +	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
> > +		return -EINVAL;
> > +
> > +	if ((last_fs_block >
> > +	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
> > +	    (last_fs_block >
> > +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
>             ^^^ I don't get the pgoff_t check. Shouldn't it rather be
> (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)?
> Because on 32-bit arch we are able to address 16TB device, which is for 1KB
> blocksize 1<<34 blocks. But your math gives 1<<30 blocks...

	This code is directly lifted from ext4.  But that said, I am
starting to think you're right.  1 page == 4 x 1K blocks, rather than 4
pages == 1 1K block.

Joel

-- 

"I always thought the hardest questions were those I could not answer.
 Now I know they are the ones I can never ask."
			- Charlie Watkins

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Fri, Aug 13, 2010 at 01:47:01PM -0700, Joel Becker wrote:
> On Fri, Aug 13, 2010 at 06:30:07PM +0200, Jan Kara wrote:
> > On Thu 12-08-10 15:29:49, Joel Becker wrote:
> > > +/**
> > > + * generic_check_addressable - Check addressability of file system
> > > + * @blocksize_bits:	log of file system block size
> > > + * @num_blocks:		number of blocks in file system
> > > + *
> > > + * Determine whether a file system with @num_blocks blocks (and a
> > > + * block size of 2**@blocksize_bits) is addressable by the sector_t
> > > + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
> > > + */
> > > +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
> > > +{
> > > +	u64 last_fs_block = num_blocks - 1;
> > > +
> > > +	if (unlikely(num_blocks == 0))
> > > +		return 0;
> > > +
> > > +	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
> > > +		return -EINVAL;
> > > +
> > > +	if ((last_fs_block >
> > > +	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
> > > +	    (last_fs_block >
> > > +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
> >             ^^^ I don't get the pgoff_t check. Shouldn't it rather be
> > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)?
> > Because on 32-bit arch we are able to address 16TB device, which is for 1KB
> > blocksize 1<<34 blocks. But your math gives 1<<30 blocks...
> 
> 	This code is directly lifted from ext4.  But that said, I am
> starting to think you're right.  1 page == 4 x 1K blocks, rather than 4
> pages == 1 1K block.

	Wouldn't it rather be:

	    ... ||
	    ((last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits)) >
	     (pgoff_t)(!0ULL))) {

Joel

-- 

"What no boss of a programmer can ever understand is that a programmer
 is working when he's staring out of the window"
	- With apologies to Burton Rascoe

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Fri, Aug 13, 2010 at 01:47:01PM -0700, Joel Becker wrote:
> On Fri, Aug 13, 2010 at 06:30:07PM +0200, Jan Kara wrote:
> > On Thu 12-08-10 15:29:49, Joel Becker wrote:
> > > +/**
> > > + * generic_check_addressable - Check addressability of file system
> > > + * @blocksize_bits:	log of file system block size
> > > + * @num_blocks:		number of blocks in file system
> > > + *
> > > + * Determine whether a file system with @num_blocks blocks (and a
> > > + * block size of 2**@blocksize_bits) is addressable by the sector_t
> > > + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
> > > + */
> > > +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
> > > +{
> > > +	u64 last_fs_block = num_blocks - 1;
> > > +
> > > +	if (unlikely(num_blocks == 0))
> > > +		return 0;
> > > +
> > > +	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
> > > +		return -EINVAL;
> > > +
> > > +	if ((last_fs_block >
> > > +	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
> > > +	    (last_fs_block >
> > > +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
> >             ^^^ I don't get the pgoff_t check. Shouldn't it rather be
> > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)?
> > Because on 32-bit arch we are able to address 16TB device, which is for 1KB
> > blocksize 1<<34 blocks. But your math gives 1<<30 blocks...
> 
> 	This code is directly lifted from ext4.  But that said, I am
> starting to think you're right.  1 page == 4 x 1K blocks, rather than 4
> pages == 1 1K block.

	Wouldn't it rather be:

	    ... ||
	    ((last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits)) >
	     (pgoff_t)(!0ULL))) {

Joel

-- 

"What no boss of a programmer can ever understand is that a programmer
 is working when he's staring out of the window"
	- With apologies to Burton Rascoe

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Eric Sandeen]

Jan Kara wrote:
> On Thu 12-08-10 15:29:49, Joel Becker wrote:
>> diff --git a/fs/libfs.c b/fs/libfs.c
>> index dcaf972..f099566 100644
>> --- a/fs/libfs.c
>> +++ b/fs/libfs.c
>> @@ -955,6 +955,35 @@ int generic_file_fsync(struct file *file, int datasync)
>>  }
>>  EXPORT_SYMBOL(generic_file_fsync);
>>  
>> +/**
>> + * generic_check_addressable - Check addressability of file system
>> + * @blocksize_bits:	log of file system block size
>> + * @num_blocks:		number of blocks in file system
>> + *
>> + * Determine whether a file system with @num_blocks blocks (and a
>> + * block size of 2**@blocksize_bits) is addressable by the sector_t
>> + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
>> + */
>> +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
>> +{
>> +	u64 last_fs_block = num_blocks - 1;
>> +
>> +	if (unlikely(num_blocks == 0))
>> +		return 0;
>> +
>> +	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
>> +		return -EINVAL;
>> +
>> +	if ((last_fs_block >
>> +	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
>> +	    (last_fs_block >
>> +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
>             ^^^ I don't get the pgoff_t check. Shouldn't it rather be
> (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)?

Argh that was my fault...  Thankfully not too many 1k-blocksize-formatted
16T devices out there, I guess.

I went through the math again and also came up with:

total fs pages is blocks / (blocks per page)
total pages is blocks / (1 << PAGE_CACHE_SHIFT / 1 << blocksize_bits)
total pages is blocks / (1 << (PAGE_CACHE_SHIFT - blocksize_bits))
total pages is blocks * (1 >> (PAGE_CACHE_SHIFT - blocksize_bits))
total pages is blocks >> (PAGE_CACHE_SHIFT - blocksize_bits)

too big if total pages is > (pgoff_t)(~0ULL)
too big if blocks >> (PAGE_CACHE_SHIFT - blocksize_bits) > (pgoff_t)(~0ULL)
too big if blocks > (pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)
and to not overflow:
too big if blocks > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)

so seems like the test is:

last_fs_block > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)

Given the density of the logic in the helper it seems like maybe breaking it
up and adding specific comments might be helpful to the reader:

	/* can IO layers fit total fs sectors in a sector_t? */
	if (last_fs_block >
	    (sector_t)(~0ULL) >> (blocksize_bits - 9))
		return -EFBIG;

	/* can page cache fit total fs pages in a pgoff_t? */
	if (last_fs_block >
	    (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)
		return -EFBIG;

Or something like that.

Sorry for chiming in late...

-Eric

> Because on 32-bit arch we are able to address 16TB device, which is for 1KB
> blocksize 1<<34 blocks. But your math gives 1<<30 blocks...
> 
> 									Honza
> 

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Eric Sandeen]

Jan Kara wrote:
> On Thu 12-08-10 15:29:49, Joel Becker wrote:
>> diff --git a/fs/libfs.c b/fs/libfs.c
>> index dcaf972..f099566 100644
>> --- a/fs/libfs.c
>> +++ b/fs/libfs.c
>> @@ -955,6 +955,35 @@ int generic_file_fsync(struct file *file, int datasync)
>>  }
>>  EXPORT_SYMBOL(generic_file_fsync);
>>  
>> +/**
>> + * generic_check_addressable - Check addressability of file system
>> + * @blocksize_bits:	log of file system block size
>> + * @num_blocks:		number of blocks in file system
>> + *
>> + * Determine whether a file system with @num_blocks blocks (and a
>> + * block size of 2**@blocksize_bits) is addressable by the sector_t
>> + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
>> + */
>> +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
>> +{
>> +	u64 last_fs_block = num_blocks - 1;
>> +
>> +	if (unlikely(num_blocks == 0))
>> +		return 0;
>> +
>> +	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
>> +		return -EINVAL;
>> +
>> +	if ((last_fs_block >
>> +	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
>> +	    (last_fs_block >
>> +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
>             ^^^ I don't get the pgoff_t check. Shouldn't it rather be
> (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)?

Argh that was my fault...  Thankfully not too many 1k-blocksize-formatted
16T devices out there, I guess.

I went through the math again and also came up with:

total fs pages is blocks / (blocks per page)
total pages is blocks / (1 << PAGE_CACHE_SHIFT / 1 << blocksize_bits)
total pages is blocks / (1 << (PAGE_CACHE_SHIFT - blocksize_bits))
total pages is blocks * (1 >> (PAGE_CACHE_SHIFT - blocksize_bits))
total pages is blocks >> (PAGE_CACHE_SHIFT - blocksize_bits)

too big if total pages is > (pgoff_t)(~0ULL)
too big if blocks >> (PAGE_CACHE_SHIFT - blocksize_bits) > (pgoff_t)(~0ULL)
too big if blocks > (pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)
and to not overflow:
too big if blocks > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)

so seems like the test is:

last_fs_block > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)

Given the density of the logic in the helper it seems like maybe breaking it
up and adding specific comments might be helpful to the reader:

	/* can IO layers fit total fs sectors in a sector_t? */
	if (last_fs_block >
	    (sector_t)(~0ULL) >> (blocksize_bits - 9))
		return -EFBIG;

	/* can page cache fit total fs pages in a pgoff_t? */
	if (last_fs_block >
	    (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)
		return -EFBIG;

Or something like that.

Sorry for chiming in late...

-Eric

> Because on 32-bit arch we are able to address 16TB device, which is for 1KB
> blocksize 1<<34 blocks. But your math gives 1<<30 blocks...
> 
> 									Honza
> 

Re: [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Sun, Aug 15, 2010 at 12:19:36PM -0500, Eric Sandeen wrote:
> >> +	    (last_fs_block >
> >> +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
> >             ^^^ I don't get the pgoff_t check. Shouldn't it rather be
> > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)?
> 
> Argh that was my fault...  Thankfully not too many 1k-blocksize-formatted
> 16T devices out there, I guess.
> 
> I went through the math again and also came up with:
> 
> total fs pages is blocks / (blocks per page)
> total pages is blocks / (1 << PAGE_CACHE_SHIFT / 1 << blocksize_bits)
> total pages is blocks / (1 << (PAGE_CACHE_SHIFT - blocksize_bits))
> total pages is blocks * (1 >> (PAGE_CACHE_SHIFT - blocksize_bits))
> total pages is blocks >> (PAGE_CACHE_SHIFT - blocksize_bits)
> 
> too big if total pages is > (pgoff_t)(~0ULL)
> too big if blocks >> (PAGE_CACHE_SHIFT - blocksize_bits) > (pgoff_t)(~0ULL)

	Why not stop here, which is what I put in my other email?
"blocks >> SHIFT-bits" is "how many pages do I need?".

> too big if blocks > (pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)
> and to not overflow:
> too big if blocks > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)

	This still overflows.  pgoff_t is a u64 on 64bit machines,
right?  So shift that left by anything and you wrap.

Joel

-- 

"You cannot bring about prosperity by discouraging thrift. You cannot
 strengthen the weak by weakening the strong. You cannot help the wage
 earner by pulling down the wage payer. You cannot further the
 brotherhood of man by encouraging class hatred. You cannot help the
 poor by destroying the rich. You cannot build character and courage by
 taking away a man's initiative and independence. You cannot help men
 permanently by doing for them what they could and should do for
 themselves."
	- Abraham Lincoln 

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Sun, Aug 15, 2010 at 12:19:36PM -0500, Eric Sandeen wrote:
> >> +	    (last_fs_block >
> >> +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
> >             ^^^ I don't get the pgoff_t check. Shouldn't it rather be
> > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)?
> 
> Argh that was my fault...  Thankfully not too many 1k-blocksize-formatted
> 16T devices out there, I guess.
> 
> I went through the math again and also came up with:
> 
> total fs pages is blocks / (blocks per page)
> total pages is blocks / (1 << PAGE_CACHE_SHIFT / 1 << blocksize_bits)
> total pages is blocks / (1 << (PAGE_CACHE_SHIFT - blocksize_bits))
> total pages is blocks * (1 >> (PAGE_CACHE_SHIFT - blocksize_bits))
> total pages is blocks >> (PAGE_CACHE_SHIFT - blocksize_bits)
> 
> too big if total pages is > (pgoff_t)(~0ULL)
> too big if blocks >> (PAGE_CACHE_SHIFT - blocksize_bits) > (pgoff_t)(~0ULL)

	Why not stop here, which is what I put in my other email?
"blocks >> SHIFT-bits" is "how many pages do I need?".

> too big if blocks > (pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)
> and to not overflow:
> too big if blocks > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)

	This still overflows.  pgoff_t is a u64 on 64bit machines,
right?  So shift that left by anything and you wrap.

Joel

-- 

"You cannot bring about prosperity by discouraging thrift. You cannot
 strengthen the weak by weakening the strong. You cannot help the wage
 earner by pulling down the wage payer. You cannot further the
 brotherhood of man by encouraging class hatred. You cannot help the
 poor by destroying the rich. You cannot build character and courage by
 taking away a man's initiative and independence. You cannot help men
 permanently by doing for them what they could and should do for
 themselves."
	- Abraham Lincoln 

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Eric Sandeen]

Joel Becker wrote:
> On Sun, Aug 15, 2010 at 12:19:36PM -0500, Eric Sandeen wrote:
>>>> +	    (last_fs_block >
>>>> +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
>>>             ^^^ I don't get the pgoff_t check. Shouldn't it rather be
>>> (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)?
>> Argh that was my fault...  Thankfully not too many 1k-blocksize-formatted
>> 16T devices out there, I guess.
>>
>> I went through the math again and also came up with:
>>
>> total fs pages is blocks / (blocks per page)
>> total pages is blocks / (1 << PAGE_CACHE_SHIFT / 1 << blocksize_bits)
>> total pages is blocks / (1 << (PAGE_CACHE_SHIFT - blocksize_bits))
>> total pages is blocks * (1 >> (PAGE_CACHE_SHIFT - blocksize_bits))
>> total pages is blocks >> (PAGE_CACHE_SHIFT - blocksize_bits)
>>
>> too big if total pages is > (pgoff_t)(~0ULL)
>> too big if blocks >> (PAGE_CACHE_SHIFT - blocksize_bits) > (pgoff_t)(~0ULL)
> 
> 	Why not stop here, which is what I put in my other email?
> "blocks >> SHIFT-bits" is "how many pages do I need?".

yeah, ok.  Was going for pointless symmetry w/ the other test...

>> too big if blocks > (pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)
>> and to not overflow:
>> too big if blocks > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)
> 
> 	This still overflows.  pgoff_t is a u64 on 64bit machines,
> right?  So shift that left by anything and you wrap.

Er, yeah.  I had 32 bits in my head since that's the case we're
checking for... whoops.

So I guess your

 	    ... ||
	    ((last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits)) >
	     (pgoff_t)(!0ULL))) {

is right :)  (my feeble brain has a hard time reading that, though, TBH)

-Eric

> Joel
> 

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Eric Sandeen]

Joel Becker wrote:
> On Sun, Aug 15, 2010 at 12:19:36PM -0500, Eric Sandeen wrote:
>>>> +	    (last_fs_block >
>>>> +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
>>>             ^^^ I don't get the pgoff_t check. Shouldn't it rather be
>>> (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)?
>> Argh that was my fault...  Thankfully not too many 1k-blocksize-formatted
>> 16T devices out there, I guess.
>>
>> I went through the math again and also came up with:
>>
>> total fs pages is blocks / (blocks per page)
>> total pages is blocks / (1 << PAGE_CACHE_SHIFT / 1 << blocksize_bits)
>> total pages is blocks / (1 << (PAGE_CACHE_SHIFT - blocksize_bits))
>> total pages is blocks * (1 >> (PAGE_CACHE_SHIFT - blocksize_bits))
>> total pages is blocks >> (PAGE_CACHE_SHIFT - blocksize_bits)
>>
>> too big if total pages is > (pgoff_t)(~0ULL)
>> too big if blocks >> (PAGE_CACHE_SHIFT - blocksize_bits) > (pgoff_t)(~0ULL)
> 
> 	Why not stop here, which is what I put in my other email?
> "blocks >> SHIFT-bits" is "how many pages do I need?".

yeah, ok.  Was going for pointless symmetry w/ the other test...

>> too big if blocks > (pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)
>> and to not overflow:
>> too big if blocks > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)
> 
> 	This still overflows.  pgoff_t is a u64 on 64bit machines,
> right?  So shift that left by anything and you wrap.

Er, yeah.  I had 32 bits in my head since that's the case we're
checking for... whoops.

So I guess your

 	    ... ||
	    ((last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits)) >
	     (pgoff_t)(!0ULL))) {

is right :)  (my feeble brain has a hard time reading that, though, TBH)

-Eric

> Joel
> 

Re: [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Sun, Aug 15, 2010 at 10:36:36PM -0500, Eric Sandeen wrote:
> Er, yeah.  I had 32 bits in my head since that's the case we're
> checking for... whoops.
> 
> So I guess your
> 
>  	    ... ||
> 	    ((last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits)) >
> 	     (pgoff_t)(!0ULL))) {
> 
> is right :)  (my feeble brain has a hard time reading that, though, TBH)

	Well, note the bug in my quickly typed version: "(!0ULL)" vs
"(~0ULL)".  How about:

	u64 last_fs_page = last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits);

	... ||
	(last_fs_page > (pgoff_t)(~0ULL))) {

Is that more readable?

Joel

-- 

Life's Little Instruction Book #99

	"Think big thoughts, but relish small pleasures."

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Sun, Aug 15, 2010 at 10:36:36PM -0500, Eric Sandeen wrote:
> Er, yeah.  I had 32 bits in my head since that's the case we're
> checking for... whoops.
> 
> So I guess your
> 
>  	    ... ||
> 	    ((last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits)) >
> 	     (pgoff_t)(!0ULL))) {
> 
> is right :)  (my feeble brain has a hard time reading that, though, TBH)

	Well, note the bug in my quickly typed version: "(!0ULL)" vs
"(~0ULL)".  How about:

	u64 last_fs_page = last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits);

	... ||
	(last_fs_page > (pgoff_t)(~0ULL))) {

Is that more readable?

Joel

-- 

Life's Little Instruction Book #99

	"Think big thoughts, but relish small pleasures."

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Eric Sandeen]

Joel Becker wrote:
> On Sun, Aug 15, 2010 at 10:36:36PM -0500, Eric Sandeen wrote:
>> Er, yeah.  I had 32 bits in my head since that's the case we're
>> checking for... whoops.
>>
>> So I guess your
>>
>>  	    ... ||
>> 	    ((last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits)) >
>> 	     (pgoff_t)(!0ULL))) {
>>
>> is right :)  (my feeble brain has a hard time reading that, though, TBH)
> 
> 	Well, note the bug in my quickly typed version: "(!0ULL)" vs
> "(~0ULL)".  

*nod* saw that but figured it was just a typo & didn't mention it ;)

> How about:
> 
> 	u64 last_fs_page = last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits);
> 
> 	... ||
> 	(last_fs_page > (pgoff_t)(~0ULL))) {
> 
> Is that more readable?

To me, yes.  Maybe do similar for last_fs_sector.

If it's getting too verbose I understand, but less dense is a lot easier
to read, IMHO.  Just style though, really, so *shrug*

Thanks,
-Eric

> Joel
> 

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Eric Sandeen]

Joel Becker wrote:
> On Sun, Aug 15, 2010 at 10:36:36PM -0500, Eric Sandeen wrote:
>> Er, yeah.  I had 32 bits in my head since that's the case we're
>> checking for... whoops.
>>
>> So I guess your
>>
>>  	    ... ||
>> 	    ((last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits)) >
>> 	     (pgoff_t)(!0ULL))) {
>>
>> is right :)  (my feeble brain has a hard time reading that, though, TBH)
> 
> 	Well, note the bug in my quickly typed version: "(!0ULL)" vs
> "(~0ULL)".  

*nod* saw that but figured it was just a typo & didn't mention it ;)

> How about:
> 
> 	u64 last_fs_page = last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits);
> 
> 	... ||
> 	(last_fs_page > (pgoff_t)(~0ULL))) {
> 
> Is that more readable?

To me, yes.  Maybe do similar for last_fs_sector.

If it's getting too verbose I understand, but less dense is a lot easier
to read, IMHO.  Just style though, really, so *shrug*

Thanks,
-Eric

> Joel
> 

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Eric Sandeen]

Joel Becker wrote:
> On Sun, Aug 15, 2010 at 10:36:36PM -0500, Eric Sandeen wrote:
>> Er, yeah.  I had 32 bits in my head since that's the case we're
>> checking for... whoops.
>>
>> So I guess your
>>
>>  	    ... ||
>> 	    ((last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits)) >
>> 	     (pgoff_t)(!0ULL))) {
>>
>> is right :)  (my feeble brain has a hard time reading that, though, TBH)
> 
> 	Well, note the bug in my quickly typed version: "(!0ULL)" vs
> "(~0ULL)".  

*nod* saw that but figured it was just a typo & didn't mention it ;)

> How about:
> 
> 	u64 last_fs_page = last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits);
> 
> 	... ||
> 	(last_fs_page > (pgoff_t)(~0ULL))) {
> 
> Is that more readable?

To me, yes.  Maybe do similar for last_fs_sector.

If it's getting too verbose I understand, but less dense is a lot easier
to read, IMHO.  Just style though, really, so *shrug*

Thanks,
-Eric

> Joel
> 

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Jan Kara]

On Fri 13-08-10 15:52:46, Joel Becker wrote:
> On Fri, Aug 13, 2010 at 01:47:01PM -0700, Joel Becker wrote:
> > On Fri, Aug 13, 2010 at 06:30:07PM +0200, Jan Kara wrote:
> > > On Thu 12-08-10 15:29:49, Joel Becker wrote:
> > > > +/**
> > > > + * generic_check_addressable - Check addressability of file system
> > > > + * @blocksize_bits:	log of file system block size
> > > > + * @num_blocks:		number of blocks in file system
> > > > + *
> > > > + * Determine whether a file system with @num_blocks blocks (and a
> > > > + * block size of 2**@blocksize_bits) is addressable by the sector_t
> > > > + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
> > > > + */
> > > > +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
> > > > +{
> > > > +	u64 last_fs_block = num_blocks - 1;
> > > > +
> > > > +	if (unlikely(num_blocks == 0))
> > > > +		return 0;
> > > > +
> > > > +	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
> > > > +		return -EINVAL;
> > > > +
> > > > +	if ((last_fs_block >
> > > > +	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
> > > > +	    (last_fs_block >
> > > > +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
> > >             ^^^ I don't get the pgoff_t check. Shouldn't it rather be
> > > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)?
> > > Because on 32-bit arch we are able to address 16TB device, which is for 1KB
> > > blocksize 1<<34 blocks. But your math gives 1<<30 blocks...
> > 
> > 	This code is directly lifted from ext4.  But that said, I am
> > starting to think you're right.  1 page == 4 x 1K blocks, rather than 4
> > pages == 1 1K block.
> 
> 	Wouldn't it rather be:
> 
> 	    ... ||
> 	    ((last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits)) >
> 	     (pgoff_t)(!0ULL))) {
  Yes, this would be even better than what I suggested.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Jan Kara]

On Fri 13-08-10 15:52:46, Joel Becker wrote:
> On Fri, Aug 13, 2010 at 01:47:01PM -0700, Joel Becker wrote:
> > On Fri, Aug 13, 2010 at 06:30:07PM +0200, Jan Kara wrote:
> > > On Thu 12-08-10 15:29:49, Joel Becker wrote:
> > > > +/**
> > > > + * generic_check_addressable - Check addressability of file system
> > > > + * @blocksize_bits:	log of file system block size
> > > > + * @num_blocks:		number of blocks in file system
> > > > + *
> > > > + * Determine whether a file system with @num_blocks blocks (and a
> > > > + * block size of 2**@blocksize_bits) is addressable by the sector_t
> > > > + * and page cache of the system.  Return 0 if so and -EFBIG otherwise.
> > > > + */
> > > > +int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
> > > > +{
> > > > +	u64 last_fs_block = num_blocks - 1;
> > > > +
> > > > +	if (unlikely(num_blocks == 0))
> > > > +		return 0;
> > > > +
> > > > +	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
> > > > +		return -EINVAL;
> > > > +
> > > > +	if ((last_fs_block >
> > > > +	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
> > > > +	    (last_fs_block >
> > > > +	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
> > >             ^^^ I don't get the pgoff_t check. Shouldn't it rather be
> > > (u64)(pgoff_t)(~0ULL) << (PAGE_CACHE_SHIFT - blocksize_bits)?
> > > Because on 32-bit arch we are able to address 16TB device, which is for 1KB
> > > blocksize 1<<34 blocks. But your math gives 1<<30 blocks...
> > 
> > 	This code is directly lifted from ext4.  But that said, I am
> > starting to think you're right.  1 page == 4 x 1K blocks, rather than 4
> > pages == 1 1K block.
> 
> 	Wouldn't it rather be:
> 
> 	    ... ||
> 	    ((last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits)) >
> 	     (pgoff_t)(!0ULL))) {
  Yes, this would be even better than what I suggested.

								Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Mon, Aug 16, 2010 at 09:44:35AM -0500, Eric Sandeen wrote:
> Joel Becker wrote:
> > How about:
> > 
> > 	u64 last_fs_page = last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits);
> > 
> > 	... ||
> > 	(last_fs_page > (pgoff_t)(~0ULL))) {
> > 
> > Is that more readable?
> 
> To me, yes.  Maybe do similar for last_fs_sector.

	last_fs_sector would be shifting up, which could wrap a really
large last_fs_blocks.  So I'm going to keep the sector_t check as-is.
How's this:

>From 8de5cb9164cdc179ba84a07b282a895d0eb794b0 Mon Sep 17 00:00:00 2001
>From: Joel Becker <joel.becker@oracle.com>
Date: Mon, 16 Aug 2010 12:10:17 -0700
Subject: [PATCH] libfs: Fix shift bug in generic_check_addressable()

generic_check_addressable() erroneously shifts pages down by a block
factor when it should be shifting up.  To prevent overflow, we shift
blocks down to pages.

Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/libfs.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/libfs.c b/fs/libfs.c
index 8debe7b..62baa03 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -925,6 +925,8 @@ EXPORT_SYMBOL(generic_file_fsync);
 int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
 {
 	u64 last_fs_block = num_blocks - 1;
+	u64 last_fs_page =
+		last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits);
 
 	if (unlikely(num_blocks == 0))
 		return 0;
@@ -932,10 +934,8 @@ int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
 	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
 		return -EINVAL;
 
-	if ((last_fs_block >
-	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
-	    (last_fs_block >
-	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
+	if ((last_fs_block > (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
+	    (last_fs_page > (pgoff_t)(~0ULL))) {
 		return -EFBIG;
 	}
 	return 0;
-- 
1.7.1

-- 

Life's Little Instruction Book #267

	"Lie on your back and look at the stars."

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Mon, Aug 16, 2010 at 09:44:35AM -0500, Eric Sandeen wrote:
> Joel Becker wrote:
> > How about:
> > 
> > 	u64 last_fs_page = last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits);
> > 
> > 	... ||
> > 	(last_fs_page > (pgoff_t)(~0ULL))) {
> > 
> > Is that more readable?
> 
> To me, yes.  Maybe do similar for last_fs_sector.

	last_fs_sector would be shifting up, which could wrap a really
large last_fs_blocks.  So I'm going to keep the sector_t check as-is.
How's this:

From 8de5cb9164cdc179ba84a07b282a895d0eb794b0 Mon Sep 17 00:00:00 2001
>From: Joel Becker <joel.becker@oracle.com>
Date: Mon, 16 Aug 2010 12:10:17 -0700
Subject: [PATCH] libfs: Fix shift bug in generic_check_addressable()

generic_check_addressable() erroneously shifts pages down by a block
factor when it should be shifting up.  To prevent overflow, we shift
blocks down to pages.

Signed-off-by: Joel Becker <joel.becker@oracle.com>
---
 fs/libfs.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/libfs.c b/fs/libfs.c
index 8debe7b..62baa03 100644
--- a/fs/libfs.c
+++ b/fs/libfs.c
@@ -925,6 +925,8 @@ EXPORT_SYMBOL(generic_file_fsync);
 int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
 {
 	u64 last_fs_block = num_blocks - 1;
+	u64 last_fs_page =
+		last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits);
 
 	if (unlikely(num_blocks == 0))
 		return 0;
@@ -932,10 +934,8 @@ int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
 	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
 		return -EINVAL;
 
-	if ((last_fs_block >
-	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
-	    (last_fs_block >
-	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
+	if ((last_fs_block > (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
+	    (last_fs_page > (pgoff_t)(~0ULL))) {
 		return -EFBIG;
 	}
 	return 0;
-- 
1.7.1

-- 

Life's Little Instruction Book #267

	"Lie on your back and look at the stars."

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127

Re: [Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Jan Kara]

On Mon 16-08-10 12:13:19, Joel Becker wrote:
> On Mon, Aug 16, 2010 at 09:44:35AM -0500, Eric Sandeen wrote:
> > Joel Becker wrote:
> > > How about:
> > > 
> > > 	u64 last_fs_page = last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits);
> > > 
> > > 	... ||
> > > 	(last_fs_page > (pgoff_t)(~0ULL))) {
> > > 
> > > Is that more readable?
> > 
> > To me, yes.  Maybe do similar for last_fs_sector.
> 
> 	last_fs_sector would be shifting up, which could wrap a really
> large last_fs_blocks.  So I'm going to keep the sector_t check as-is.
> How's this:
> 
> >From 8de5cb9164cdc179ba84a07b282a895d0eb794b0 Mon Sep 17 00:00:00 2001
> >From: Joel Becker <joel.becker@oracle.com>
> Date: Mon, 16 Aug 2010 12:10:17 -0700
> Subject: [PATCH] libfs: Fix shift bug in generic_check_addressable()
> 
> generic_check_addressable() erroneously shifts pages down by a block
> factor when it should be shifting up.  To prevent overflow, we shift
> blocks down to pages.
> 
> Signed-off-by: Joel Becker <joel.becker@oracle.com>
  Looks good.
Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/libfs.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 8debe7b..62baa03 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -925,6 +925,8 @@ EXPORT_SYMBOL(generic_file_fsync);
>  int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
>  {
>  	u64 last_fs_block = num_blocks - 1;
> +	u64 last_fs_page =
> +		last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits);
>  
>  	if (unlikely(num_blocks == 0))
>  		return 0;
> @@ -932,10 +934,8 @@ int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
>  	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
>  		return -EINVAL;
>  
> -	if ((last_fs_block >
> -	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
> -	    (last_fs_block >
> -	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
> +	if ((last_fs_block > (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
> +	    (last_fs_page > (pgoff_t)(~0ULL))) {
>  		return -EFBIG;
>  	}
>  	return 0;
> -- 
> 1.7.1
> 
> -- 
> 
> Life's Little Instruction Book #267
> 
> 	"Lie on your back and look at the stars."
> 
> Joel Becker
> Consulting Software Developer
> Oracle
> E-mail: joel.becker@oracle.com
> Phone: (650) 506-8127
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Jan Kara]

On Mon 16-08-10 12:13:19, Joel Becker wrote:
> On Mon, Aug 16, 2010 at 09:44:35AM -0500, Eric Sandeen wrote:
> > Joel Becker wrote:
> > > How about:
> > > 
> > > 	u64 last_fs_page = last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits);
> > > 
> > > 	... ||
> > > 	(last_fs_page > (pgoff_t)(~0ULL))) {
> > > 
> > > Is that more readable?
> > 
> > To me, yes.  Maybe do similar for last_fs_sector.
> 
> 	last_fs_sector would be shifting up, which could wrap a really
> large last_fs_blocks.  So I'm going to keep the sector_t check as-is.
> How's this:
> 
> >From 8de5cb9164cdc179ba84a07b282a895d0eb794b0 Mon Sep 17 00:00:00 2001
> >From: Joel Becker <joel.becker@oracle.com>
> Date: Mon, 16 Aug 2010 12:10:17 -0700
> Subject: [PATCH] libfs: Fix shift bug in generic_check_addressable()
> 
> generic_check_addressable() erroneously shifts pages down by a block
> factor when it should be shifting up.  To prevent overflow, we shift
> blocks down to pages.
> 
> Signed-off-by: Joel Becker <joel.becker@oracle.com>
  Looks good.
Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/libfs.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/libfs.c b/fs/libfs.c
> index 8debe7b..62baa03 100644
> --- a/fs/libfs.c
> +++ b/fs/libfs.c
> @@ -925,6 +925,8 @@ EXPORT_SYMBOL(generic_file_fsync);
>  int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
>  {
>  	u64 last_fs_block = num_blocks - 1;
> +	u64 last_fs_page =
> +		last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits);
>  
>  	if (unlikely(num_blocks == 0))
>  		return 0;
> @@ -932,10 +934,8 @@ int generic_check_addressable(unsigned blocksize_bits, u64 num_blocks)
>  	if ((blocksize_bits < 9) || (blocksize_bits > PAGE_CACHE_SHIFT))
>  		return -EINVAL;
>  
> -	if ((last_fs_block >
> -	     (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
> -	    (last_fs_block >
> -	     (pgoff_t)(~0ULL) >> (PAGE_CACHE_SHIFT - blocksize_bits))) {
> +	if ((last_fs_block > (sector_t)(~0ULL) >> (blocksize_bits - 9)) ||
> +	    (last_fs_page > (pgoff_t)(~0ULL))) {
>  		return -EFBIG;
>  	}
>  	return 0;
> -- 
> 1.7.1
> 
> -- 
> 
> Life's Little Instruction Book #267
> 
> 	"Lie on your back and look at the stars."
> 
> Joel Becker
> Consulting Software Developer
> Oracle
> E-mail: joel.becker at oracle.com
> Phone: (650) 506-8127
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR

Re: [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Mon, Aug 16, 2010 at 09:21:00PM +0200, Jan Kara wrote:
> On Mon 16-08-10 12:13:19, Joel Becker wrote:
> > On Mon, Aug 16, 2010 at 09:44:35AM -0500, Eric Sandeen wrote:
> > > Joel Becker wrote:
> > > > How about:
> > > > 
> > > > 	u64 last_fs_page = last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits);
> > > > 
> > > > 	... ||
> > > > 	(last_fs_page > (pgoff_t)(~0ULL))) {
> > > > 
> > > > Is that more readable?
> > > 
> > > To me, yes.  Maybe do similar for last_fs_sector.
> > 
> > 	last_fs_sector would be shifting up, which could wrap a really
> > large last_fs_blocks.  So I'm going to keep the sector_t check as-is.
> > How's this:
> > 
> > >From 8de5cb9164cdc179ba84a07b282a895d0eb794b0 Mon Sep 17 00:00:00 2001
> > >From: Joel Becker <joel.becker@oracle.com>
> > Date: Mon, 16 Aug 2010 12:10:17 -0700
> > Subject: [PATCH] libfs: Fix shift bug in generic_check_addressable()
> > 
> > generic_check_addressable() erroneously shifts pages down by a block
> > factor when it should be shifting up.  To prevent overflow, we shift
> > blocks down to pages.
> > 
> > Signed-off-by: Joel Becker <joel.becker@oracle.com>
>   Looks good.
> Reviewed-by: Jan Kara <jack@suse.cz>

	Ok, I'm going to keep this atop my existing branch, and if it
all shakes out in linux-next for a few days, send it along.

Joel

-- 

"When ideas fail, words come in very handy." 
         - Goethe

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker@oracle.com
Phone: (650) 506-8127

[Ocfs2-devel] [PATCH 1/3] ext3/ext4: Factor out disk addressability check

[Joel Becker]

On Mon, Aug 16, 2010 at 09:21:00PM +0200, Jan Kara wrote:
> On Mon 16-08-10 12:13:19, Joel Becker wrote:
> > On Mon, Aug 16, 2010 at 09:44:35AM -0500, Eric Sandeen wrote:
> > > Joel Becker wrote:
> > > > How about:
> > > > 
> > > > 	u64 last_fs_page = last_fs_block >> (PAGE_CACHE_SHIFT - blocksize_bits);
> > > > 
> > > > 	... ||
> > > > 	(last_fs_page > (pgoff_t)(~0ULL))) {
> > > > 
> > > > Is that more readable?
> > > 
> > > To me, yes.  Maybe do similar for last_fs_sector.
> > 
> > 	last_fs_sector would be shifting up, which could wrap a really
> > large last_fs_blocks.  So I'm going to keep the sector_t check as-is.
> > How's this:
> > 
> > >From 8de5cb9164cdc179ba84a07b282a895d0eb794b0 Mon Sep 17 00:00:00 2001
> > >From: Joel Becker <joel.becker@oracle.com>
> > Date: Mon, 16 Aug 2010 12:10:17 -0700
> > Subject: [PATCH] libfs: Fix shift bug in generic_check_addressable()
> > 
> > generic_check_addressable() erroneously shifts pages down by a block
> > factor when it should be shifting up.  To prevent overflow, we shift
> > blocks down to pages.
> > 
> > Signed-off-by: Joel Becker <joel.becker@oracle.com>
>   Looks good.
> Reviewed-by: Jan Kara <jack@suse.cz>

	Ok, I'm going to keep this atop my existing branch, and if it
all shakes out in linux-next for a few days, send it along.

Joel

-- 

"When ideas fail, words come in very handy." 
         - Goethe

Joel Becker
Consulting Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127