Skip to contentSkip to conversation

block: Fix LSF default inconsistency

PATCH In review Jean Delvare 5 participants 21 messages Dec 11, 2008 – Dec 12, 2008

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 block: Fix LSF default inconsistency

Configuration option LSF has a default which contradicts its help text. The help text says "if unsure, say Y" but there is no explicit default, and the default default is N.

This inconsistency was introduced by commit 88b9adb073b7a69a54b1b14423103bc24587ebdc. According to the commit message, we want users to enable this option, so it should default to Y.

Cc Jens Axboe <[email protected]>
Cc Trond Myklebust <[email protected]>
---
 block/Kconfig |    1 +
 1 file changed, 1 insertion(+)
block/Kconfig +1 -0
@@ -60,6 +60,7 @@ config BLK_DEV_IO_TRACE
 config LSF
 	bool "Support for Large Single Files"
 	depends on !64BIT
+	default y
 	help
 	  Say Y here if you want to be able to handle very large files (2TB
 	  and larger), otherwise say N.
Signature · 1 line
Jean Delvare
#1 lore raw
Quoted 8 lines · Jean Delvare

Configuration option LSF has a default which contradicts its help text. The help text says "if unsure, say Y" but there is no explicit default, and the default default is N.

This inconsistency was introduced by commit 88b9adb073b7a69a54b1b14423103bc24587ebdc. According to the commit message, we want users to enable this option, so it should default to Y.

I wonder if we just shouldn't get rid of this option and just have the single CONFIG_LBD option control both of these. If you set CONFIG_LBD, you probably want large files as well. And CONFIG_LSF without CONFIG_LBD doesn't make a lot of sense.

Would anyone object to such a change?

Signature · 1 line
Jens Axboe
#2 lore raw
Quoted 10 lines · Jens Axboe
Quoted 8 lines · Jean Delvare

Configuration option LSF has a default which contradicts its help text. The help text says "if unsure, say Y" but there is no explicit default, and the default default is N.

This inconsistency was introduced by commit 88b9adb073b7a69a54b1b14423103bc24587ebdc. According to the commit message, we want users to enable this option, so it should default to Y.

Quoted 6 lines

I wonder if we just shouldn't get rid of this option and just have the single CONFIG_LBD option control both of these. If you set CONFIG_LBD, you probably want large files as well. And CONFIG_LSF without CONFIG_LBD doesn't make a lot of sense.

Would anyone object to such a change?

No objection from me, getting rid of configuration options almost always gets my vote :)

Signature · 1 line
Jean Delvare
#3 lore raw
Quoted 16 lines · Jean Delvare
Quoted 15 lines · Jens Axboe
Quoted 8 lines · Jean Delvare

Configuration option LSF has a default which contradicts its help text. The help text says "if unsure, say Y" but there is no explicit default, and the default default is N.

This inconsistency was introduced by commit 88b9adb073b7a69a54b1b14423103bc24587ebdc. According to the commit message, we want users to enable this option, so it should default to Y.

I wonder if we just shouldn't get rid of this option and just have the single CONFIG_LBD option control both of these. If you set CONFIG_LBD, you probably want large files as well. And CONFIG_LSF without CONFIG_LBD doesn't make a lot of sense.

Quoted 4 lines
Quoted 1 line

Would anyone object to such a change?

No objection from me, getting rid of configuration options almost always gets my vote :)

Yeah, mine too. One recent addition was CONFIG_UNEVICTABLE_LRU - why on earth is that an option?!

Anyway, how about something like this? Totally untested...

block/Kconfig +6 -18
index 1ab7c15..ce566dd 100644
@@ -24,21 +24,18 @@ menuconfig BLOCK
 if BLOCK
 
 config LBD
-	bool "Support for Large Block Devices"
+	bool "Support for Large Block Devices and files"
 	depends on !64BIT
+	select LSF
 	help
-	  Enable block devices of size 2TB and larger.
+	  Enable block devices or files of size 2TB and larger.
 
 	  This option is required to support the full capacity of large
 	  (2TB+) block devices, including RAID, disk, Network Block Device,
 	  Logical Volume Manager (LVM) and loopback.
-
-	  For example, RAID devices are frequently bigger than the capacity
-	  of the largest individual hard drive.
-
-	  This option is not required if you have individual disk drives
-	  which total 2TB+ and you are not aggregating the capacity into
-	  a large block device (e.g. using RAID or LVM).
+	
+	  This option also enables support for single files larger than
+	  2TB.
 
 	  If unsure, say N.
 
@@ -57,15 +54,6 @@ config BLK_DEV_IO_TRACE
 
 	  If unsure, say N.
 
-config LSF
-	bool "Support for Large Single Files"
-	depends on !64BIT
-	help
-	  Say Y here if you want to be able to handle very large files (2TB
-	  and larger), otherwise say N.
-
-	  If unsure, say Y.
-
 config BLK_DEV_BSG
 	bool "Block layer SG support v4 (EXPERIMENTAL)"
 	depends on EXPERIMENTAL
Signature · 1 line
Jens Axboe
#4 lore raw
Quoted 5 lines
Quoted 2 lines

No objection from me, getting rid of configuration options almost always gets my vote :)

Yeah, mine too. One recent addition was CONFIG_UNEVICTABLE_LRU - why on earth is that an option?!

As far as I know, CONFIG_UNEVICTABLE_LRU depend on CONFIG_MMU because any unevictable lru developer don't have nommu machine ;)

I expect that nobody of mmu user don't turn off unevictable lru feature.

#5 lore raw
Quoted 10 lines · KOSAKI Motohiro
Quoted 5 lines
Quoted 2 lines

No objection from me, getting rid of configuration options almost always gets my vote :)

Yeah, mine too. One recent addition was CONFIG_UNEVICTABLE_LRU - why on earth is that an option?!

As far as I know, CONFIG_UNEVICTABLE_LRU depend on CONFIG_MMU because any unevictable lru developer don't have nommu machine ;)

I expect that nobody of mmu user don't turn off unevictable lru feature.

Perhaps I didn't frase the question correctly. My question is, why is it a visible option? Does it make ANY sense to turn off CONFIG_UNEVICTABLE_LRU?

Signature · 1 line
Jens Axboe
#6 lore raw
Quoted 7 lines · KOSAKI Motohiro
Quoted 6 lines
Quoted 5 lines
Quoted 2 lines

No objection from me, getting rid of configuration options almost always gets my vote :)

Yeah, mine too. One recent addition was CONFIG_UNEVICTABLE_LRU - why on earth is that an option?!

Quoted 8 lines
Quoted 4 lines

As far as I know, CONFIG_UNEVICTABLE_LRU depend on CONFIG_MMU because any unevictable lru developer don't have nommu machine ;)

I expect that nobody of mmu user don't turn off unevictable lru feature.

Perhaps I didn't frase the question correctly. My question is, why is it a visible option? Does it make ANY sense to turn off CONFIG_UNEVICTABLE_LRU?

very difficult question...

As far as I know, CONFIG_UNEVICTABLE_LRU doesn't have any bad side effect. So, I expect we can remove UNEVICTABLE_LRU Kconfig option in the future.

but it is _not_ VM developr consensus. just my thinking.

#7 lore raw
Quoted 16 lines · KOSAKI Motohiro
Quoted 7 lines · KOSAKI Motohiro
Quoted 6 lines
Quoted 5 lines
Quoted 2 lines

No objection from me, getting rid of configuration options almost always gets my vote :)

Yeah, mine too. One recent addition was CONFIG_UNEVICTABLE_LRU - why on earth is that an option?!

Quoted 8 lines
Quoted 4 lines

As far as I know, CONFIG_UNEVICTABLE_LRU depend on CONFIG_MMU because any unevictable lru developer don't have nommu machine ;)

I expect that nobody of mmu user don't turn off unevictable lru feature.

Perhaps I didn't frase the question correctly. My question is, why is it a visible option? Does it make ANY sense to turn off CONFIG_UNEVICTABLE_LRU?

Quoted 6 lines

very difficult question...

As far as I know, CONFIG_UNEVICTABLE_LRU doesn't have any bad side effect. So, I expect we can remove UNEVICTABLE_LRU Kconfig option in the future.

but it is _not_ VM developr consensus. just my thinking.

Me neither, lets ask the originator of the patch. Rik, why is unevictable lru an option?

Signature · 1 line
Jens Axboe
#8 lore raw
Quoted 2 lines · Jens Axboe

Me neither, lets ask the originator of the patch. Rik, why is unevictable lru an option?

I think we had some reasons to keep it as a separate config option during development, but nowadays we might as well put it all just under CONFIG_SWAP...

Signature · 1 line
All rights reversed.
#9 lore raw
Quoted 6 lines
Quoted 2 lines

Me neither, lets ask the originator of the patch. Rik, why is unevictable lru an option?

I think we had some reasons to keep it as a separate config option during development, but nowadays we might as well put it all just under CONFIG_SWAP...

I'm sad ;)

To move mlocked file cache page to unevictable list is useful although swapless embedded device.

Actually, number of scanning pages of reclaim is calculated by number of pages in list.
if unevictable page stay in evictable list a lot, reclaim logic can calculate wrong scanning number.

#10 lore raw
Quoted 11 lines · KOSAKI Motohiro
Quoted 6 lines
Quoted 2 lines

Me neither, lets ask the originator of the patch. Rik, why is unevictable lru an option?

I think we had some reasons to keep it as a separate config option during development, but nowadays we might as well put it all just under CONFIG_SWAP...

I'm sad ;)

To move mlocked file cache page to unevictable list is useful although swapless embedded device.

Yes, I don't understand why Rik suggested CONFIG_SWAP for it either.

Quoted 5 lines

Actually, number of scanning pages of reclaim is calculated by number of pages in list.
if unevictable page stay in evictable list a lot, reclaim logic can calculate wrong scanning number.

#11 lore raw
Quoted 7 lines · Hugh Dickins
Quoted 3 lines · KOSAKI Motohiro

To move mlocked file cache page to unevictable list is useful although swapless embedded device.

Yes, I don't understand why Rik suggested CONFIG_SWAP for it either.

On swapless systems (with CONFIG_SWAP=n), we never scan the anon lists, so we do not need to split out the mlocked pages from the anon pages.

On a system with swap, we want to split out the mlocked pages so that scan balancing always works right. So whenever CONFIG_SWAP is on, we want CONFIG_UNEVICTABLE_LRU to be on as well.

That means we can replace CONFIG_UNEVICTABLE_LRU with CONFIG_SWAP throughout the code, and get a kernel with the desired behaviour either way.

#12 lore raw
Quoted 11 lines · Rik van Riel
Quoted 6 lines · Hugh Dickins
Quoted 2 lines · KOSAKI Motohiro

To move mlocked file cache page to unevictable list is useful although swapless embedded device.

Yes, I don't understand why Rik suggested CONFIG_SWAP for it either.

On swapless systems (with CONFIG_SWAP=n), we never scan the anon lists, so we do not need to split out the mlocked pages from the anon pages.

Quoted 8 lines

On a system with swap, we want to split out the mlocked pages so that scan balancing always works right. So whenever CONFIG_SWAP is on, we want CONFIG_UNEVICTABLE_LRU to be on as well.

That means we can replace CONFIG_UNEVICTABLE_LRU with CONFIG_SWAP throughout the code, and get a kernel with the desired behaviour either way.

Sorry, I'm still puzzled: I thought the unevictable lru was for shared file pages as well as for anon pages? Of course I agree with you over the anons (and shm and tmpfs get counted along with the anons in this context), but wouldn't we still need an unevictable lru for mlocked shared file pages?

Hugh

#13 lore raw
Quoted 5 lines · Hugh Dickins

Sorry, I'm still puzzled: I thought the unevictable lru was for shared file pages as well as for anon pages? Of course I agree with you over the anons (and shm and tmpfs get counted along with the anons in this context), but wouldn't we still need an unevictable lru for mlocked shared file pages?

Good point, you are correct. I had forgotten about that part completely - keeping CONFIG_UNEVICTABLE_LRU may be a good idea after all.

Signature · 1 line
All Rights Reversed
#14 lore raw
Quoted 2 lines · Rik van Riel

keeping CONFIG_UNEVICTABLE_LRU may be a good idea after all.

Do you mean, keeping the CONFIG_UNEVICTABLE_LRU codepath even when CONFIG_SWAP=n is a good idea? Or do you see a reason we actually still need CONFIG_UNEVICTABLE_LRU as an option?

It does add about 2.6k to kernel text (YMMV), is saving that worth the extra config option? For others to vote, really.

Hugh

#15 lore raw
In reply to Jens Axboe

Hi Jens,

Quoted 7 lines · Jens Axboe

Anyway, how about something like this? Totally untested...

block/Kconfig +0 -0
index 1ab7c15..ce566dd 100644

block/Kconfig menuconfig BLOCK · line 29

Quoted 7 lines
block/Kconfig +2 -1
@@ -24,21 +24,18 @@ menuconfig BLOCK
 if BLOCK
 
 config LBD
-	bool "Support for Large Block Devices"
+	bool "Support for Large Block Devices and files"
 	depends on !64BIT
+	select LSF

You can't select something...

Quoted 21 lines
block/Kconfig +4 -8
 	help
-	  Enable block devices of size 2TB and larger.
+	  Enable block devices or files of size 2TB and larger.
 
 	  This option is required to support the full capacity of large
 	  (2TB+) block devices, including RAID, disk, Network Block Device,
 	  Logical Volume Manager (LVM) and loopback.
-
-	  For example, RAID devices are frequently bigger than the capacity
-	  of the largest individual hard drive.
-
-	  This option is not required if you have individual disk drives
-	  which total 2TB+ and you are not aggregating the capacity into
-	  a large block device (e.g. using RAID or LVM).
+	
+	  This option also enables support for single files larger than
+	  2TB.
 
 	  If unsure, say N.
 

block/Kconfig config BLK_DEV_IO_TRACE · line 60

Quoted 4 lines
block/Kconfig +0 -1
@@ -57,15 +54,6 @@ config BLK_DEV_IO_TRACE
 
 	  If unsure, say N.
 
-config LSF

... that no longer exists. You must either leave the config LSF as a bool with no label and no help file, or drop it altogether and replace all occurrences of CONFIG_LSF in the kernel source with CONFIG_LBD.

Quoted 12 lines
block/Kconfig +0 -8
-	bool "Support for Large Single Files"
-	depends on !64BIT
-	help
-	  Say Y here if you want to be able to handle very large files (2TB
-	  and larger), otherwise say N.
-
-	  If unsure, say Y.
-
 config BLK_DEV_BSG
 	bool "Block layer SG support v4 (EXPERIMENTAL)"
 	depends on EXPERIMENTAL
Signature · 1 line
Jean Delvare
#16 lore raw
Quoted 42 lines · Jean Delvare

Hi Jens,

Quoted 7 lines · Jens Axboe

Anyway, how about something like this? Totally untested...

block/Kconfig +0 -0
index 1ab7c15..ce566dd 100644

block/Kconfig menuconfig BLOCK · line 29

Quoted 7 lines
block/Kconfig +2 -1
@@ -24,21 +24,18 @@ menuconfig BLOCK
 if BLOCK
 
 config LBD
-	bool "Support for Large Block Devices"
+	bool "Support for Large Block Devices and files"
 	depends on !64BIT
+	select LSF

You can't select something...

Quoted 22 lines
block/Kconfig +4 -8
 	help
-	  Enable block devices of size 2TB and larger.
+	  Enable block devices or files of size 2TB and larger.
 
 	  This option is required to support the full capacity of large
 	  (2TB+) block devices, including RAID, disk, Network Block Device,
 	  Logical Volume Manager (LVM) and loopback.
-
-	  For example, RAID devices are frequently bigger than the capacity
-	  of the largest individual hard drive.
-
-	  This option is not required if you have individual disk drives
-	  which total 2TB+ and you are not aggregating the capacity into
-	  a large block device (e.g. using RAID or LVM).
+	
+	  This option also enables support for single files larger than
+	  2TB.
 
 	  If unsure, say N.
 
@@ -57,15 +54,6 @@ config BLK_DEV_IO_TRACE
 
Quoted 7 lines

block/Kconfig config BLK_DEV_IO_TRACE · line 60

Quoted 3 lines
block/Kconfig +0 -1
 	  If unsure, say N.
 
-config LSF

... that no longer exists. You must either leave the config LSF as a bool with no label and no help file, or drop it altogether and replace all occurrences of CONFIG_LSF in the kernel source with CONFIG_LBD.

Oh right, that's pretty dumb. Lets just do the former, then a later patch can replace it completely if wanted.

block/Kconfig +9 -18
index 1ab7c15..a785e65 100644
@@ -23,22 +23,22 @@ menuconfig BLOCK
 
 if BLOCK
 
+config LSF
+	bool
+
 config LBD
-	bool "Support for Large Block Devices"
+	bool "Support for Large Block Devices and files"
 	depends on !64BIT
+	select LSF
 	help
-	  Enable block devices of size 2TB and larger.
+	  Enable block devices or files of size 2TB and larger.
 
 	  This option is required to support the full capacity of large
 	  (2TB+) block devices, including RAID, disk, Network Block Device,
 	  Logical Volume Manager (LVM) and loopback.
-
-	  For example, RAID devices are frequently bigger than the capacity
-	  of the largest individual hard drive.
-
-	  This option is not required if you have individual disk drives
-	  which total 2TB+ and you are not aggregating the capacity into
-	  a large block device (e.g. using RAID or LVM).
+	
+	  This option also enables support for single files larger than
+	  2TB.
 
 	  If unsure, say N.
 
@@ -57,15 +57,6 @@ config BLK_DEV_IO_TRACE
 
 	  If unsure, say N.
 
-config LSF
-	bool "Support for Large Single Files"
-	depends on !64BIT
-	help
-	  Say Y here if you want to be able to handle very large files (2TB
-	  and larger), otherwise say N.
-
-	  If unsure, say Y.
-
 config BLK_DEV_BSG
 	bool "Block layer SG support v4 (EXPERIMENTAL)"
 	depends on EXPERIMENTAL
Signature · 1 line
Jens Axboe
author Acked-by
#17 lore raw

Hi Jens,

Quoted 8 lines · Jens Axboe

Oh right, that's pretty dumb. Lets just do the former, then a later patch can replace it completely if wanted.

block/Kconfig +0 -0
index 1ab7c15..a785e65 100644

block/Kconfig menuconfig BLOCK · line 30

Quoted 9 lines
block/Kconfig +4 -1
@@ -23,22 +23,22 @@ menuconfig BLOCK
 
 if BLOCK
 
+config LSF
+	bool
+
 config LBD
-	bool "Support for Large Block Devices"
+	bool "Support for Large Block Devices and files"

It might be the right time to drop the extra capital letters, for consistency.

Quoted 29 lines
block/Kconfig +5 -10
 	depends on !64BIT
+	select LSF
 	help
-	  Enable block devices of size 2TB and larger.
+	  Enable block devices or files of size 2TB and larger.
 
 	  This option is required to support the full capacity of large
 	  (2TB+) block devices, including RAID, disk, Network Block Device,
 	  Logical Volume Manager (LVM) and loopback.
-
-	  For example, RAID devices are frequently bigger than the capacity
-	  of the largest individual hard drive.
-
-	  This option is not required if you have individual disk drives
-	  which total 2TB+ and you are not aggregating the capacity into
-	  a large block device (e.g. using RAID or LVM).
+	
+	  This option also enables support for single files larger than
+	  2TB.
 
 	  If unsure, say N.
 
@@ -57,15 +57,6 @@ config BLK_DEV_IO_TRACE
 
 	  If unsure, say N.
 
-config LSF
-	bool "Support for Large Single Files"

block/Kconfig config BLK_DEV_IO_TRACE · line 62

Quoted 10 lines
block/Kconfig +0 -7
-	depends on !64BIT
-	help
-	  Say Y here if you want to be able to handle very large files (2TB
-	  and larger), otherwise say N.
-
-	  If unsure, say Y.
-
 config BLK_DEV_BSG
 	bool "Block layer SG support v4 (EXPERIMENTAL)"
 	depends on EXPERIMENTAL

As a side note, someone will have to tell me why changing the value of option LSF triggers a full kernel rebuild.

Signature · 1 line
Jean Delvare
#18 lore raw
Quoted 14 lines · Jean Delvare

Hi Jens,

Quoted 11 lines · Jens Axboe

Oh right, that's pretty dumb. Lets just do the former, then a later patch can replace it completely if wanted.

block/Kconfig +0 -0
index 1ab7c15..a785e65 100644
@@ -23,22 +23,22 @@ menuconfig BLOCK
 
 if BLOCK
 
Quoted 9 lines

block/Kconfig menuconfig BLOCK · line 30

Quoted 6 lines
block/Kconfig +4 -1
+config LSF
+	bool
+
 config LBD
-	bool "Support for Large Block Devices"
+	bool "Support for Large Block Devices and files"

It might be the right time to drop the extra capital letters, for consistency.

Agree, it's pretty ugly. Modified patch below, it drops CONFIG_LSF completely as it's basically only used in a single spot.

Quoted 42 lines
Quoted 29 lines
block/Kconfig +5 -10
 	depends on !64BIT
+	select LSF
 	help
-	  Enable block devices of size 2TB and larger.
+	  Enable block devices or files of size 2TB and larger.
 
 	  This option is required to support the full capacity of large
 	  (2TB+) block devices, including RAID, disk, Network Block Device,
 	  Logical Volume Manager (LVM) and loopback.
-
-	  For example, RAID devices are frequently bigger than the capacity
-	  of the largest individual hard drive.
-
-	  This option is not required if you have individual disk drives
-	  which total 2TB+ and you are not aggregating the capacity into
-	  a large block device (e.g. using RAID or LVM).
+	
+	  This option also enables support for single files larger than
+	  2TB.
 
 	  If unsure, say N.
 
@@ -57,15 +57,6 @@ config BLK_DEV_IO_TRACE
 
 	  If unsure, say N.
 
-config LSF
-	bool "Support for Large Single Files"

block/Kconfig config BLK_DEV_IO_TRACE · line 62

Quoted 10 lines
block/Kconfig +0 -7
-	depends on !64BIT
-	help
-	  Say Y here if you want to be able to handle very large files (2TB
-	  and larger), otherwise say N.
-
-	  If unsure, say Y.
-
 config BLK_DEV_BSG
 	bool "Block layer SG support v4 (EXPERIMENTAL)"
 	depends on EXPERIMENTAL
Quoted 2 lines

As a side note, someone will have to tell me why changing the value of option LSF triggers a full kernel rebuild.

It changes types.h, I guess pretty much everything has that included :-)

block/Kconfig +5 -18
index 1ab7c15..195968e 100644
@@ -24,21 +24,17 @@ menuconfig BLOCK
 if BLOCK
 
 config LBD
-	bool "Support for Large Block Devices"
+	bool "Support for large block devices and files"
 	depends on !64BIT
 	help
-	  Enable block devices of size 2TB and larger.
+	  Enable block devices or files of size 2TB and larger.
 
 	  This option is required to support the full capacity of large
 	  (2TB+) block devices, including RAID, disk, Network Block Device,
 	  Logical Volume Manager (LVM) and loopback.
-
-	  For example, RAID devices are frequently bigger than the capacity
-	  of the largest individual hard drive.
-
-	  This option is not required if you have individual disk drives
-	  which total 2TB+ and you are not aggregating the capacity into
-	  a large block device (e.g. using RAID or LVM).
+	
+	  This option also enables support for single files larger than
+	  2TB.
 
 	  If unsure, say N.
 
@@ -57,15 +53,6 @@ config BLK_DEV_IO_TRACE
 
 	  If unsure, say N.
 
-config LSF
-	bool "Support for Large Single Files"
-	depends on !64BIT
-	help
-	  Say Y here if you want to be able to handle very large files (2TB
-	  and larger), otherwise say N.
-
-	  If unsure, say Y.
-
 config BLK_DEV_BSG
 	bool "Block layer SG support v4 (EXPERIMENTAL)"
 	depends on EXPERIMENTAL
fs/ext4/super.c +4 -4
index e4a241c..04158ad 100644
@@ -1721,7 +1721,7 @@ static loff_t ext4_max_size(int blkbits, int has_huge_files)
 	/* small i_blocks in vfs inode? */
 	if (!has_huge_files || sizeof(blkcnt_t) < sizeof(u64)) {
 		/*
-		 * CONFIG_LSF is not enabled implies the inode
+		 * CONFIG_LBD is not enabled implies the inode
 		 * i_block represent total blocks in 512 bytes
 		 * 32 == size of vfs inode i_blocks * 8
 		 */
@@ -1764,7 +1764,7 @@ static loff_t ext4_max_bitmap_size(int bits, int has_huge_files)
 
 	if (!has_huge_files || sizeof(blkcnt_t) < sizeof(u64)) {
 		/*
-		 * !has_huge_files or CONFIG_LSF is not enabled
+		 * !has_huge_files or CONFIG_LBD is not enabled
 		 * implies the inode i_block represent total blocks in
 		 * 512 bytes 32 == size of vfs inode i_blocks * 8
 		 */
@@ -2021,13 +2021,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	if (has_huge_files) {
 		/*
 		 * Large file size enabled file system can only be
-		 * mount if kernel is build with CONFIG_LSF
+		 * mount if kernel is build with CONFIG_LBD
 		 */
 		if (sizeof(root->i_blocks) < sizeof(u64) &&
 				!(sb->s_flags & MS_RDONLY)) {
 			printk(KERN_ERR "EXT4-fs: %s: Filesystem with huge "
 					"files cannot be mounted read-write "
-					"without CONFIG_LSF.\n", sb->s_id);
+					"without CONFIG_LBD.\n", sb->s_id);
 			goto failed_mount;
 		}
 	}
include/linux/types.h +3 -8
index 1d98330..121f349 100644
@@ -135,19 +135,14 @@ typedef		__s64		int64_t;
  *
  * Linux always considers sectors to be 512 bytes long independently
  * of the devices real block size.
+ *
+ * blkcnt_t is the type of the inode's block count.
  */
 #ifdef CONFIG_LBD
 typedef u64 sector_t;
-#else
-typedef unsigned long sector_t;
-#endif
-
-/*
- * The type of the inode's block count.
- */
-#ifdef CONFIG_LSF
 typedef u64 blkcnt_t;
 #else
+typedef unsigned long sector_t;
 typedef unsigned long blkcnt_t;
 #endif
 
Signature · 1 line
Jens Axboe
author Acked-by
#19 lore raw

Hi Jens,

Quoted 8 lines · Jens Axboe

Agree, it's pretty ugly. Modified patch below, it drops CONFIG_LSF completely as it's basically only used in a single spot.

Quoted 2 lines · Jean Delvare

As a side note, someone will have to tell me why changing the value of option LSF triggers a full kernel rebuild.

It changes types.h, I guess pretty much everything has that included :-)

Indeed, that was the reason.

Quoted 90 lines
block/Kconfig +5 -18
index 1ab7c15..195968e 100644
@@ -24,21 +24,17 @@ menuconfig BLOCK
 if BLOCK
 
 config LBD
-	bool "Support for Large Block Devices"
+	bool "Support for large block devices and files"
 	depends on !64BIT
 	help
-	  Enable block devices of size 2TB and larger.
+	  Enable block devices or files of size 2TB and larger.
 
 	  This option is required to support the full capacity of large
 	  (2TB+) block devices, including RAID, disk, Network Block Device,
 	  Logical Volume Manager (LVM) and loopback.
-
-	  For example, RAID devices are frequently bigger than the capacity
-	  of the largest individual hard drive.
-
-	  This option is not required if you have individual disk drives
-	  which total 2TB+ and you are not aggregating the capacity into
-	  a large block device (e.g. using RAID or LVM).
+	
+	  This option also enables support for single files larger than
+	  2TB.
 
 	  If unsure, say N.
 
@@ -57,15 +53,6 @@ config BLK_DEV_IO_TRACE
 
 	  If unsure, say N.
 
-config LSF
-	bool "Support for Large Single Files"
-	depends on !64BIT
-	help
-	  Say Y here if you want to be able to handle very large files (2TB
-	  and larger), otherwise say N.
-
-	  If unsure, say Y.
-
 config BLK_DEV_BSG
 	bool "Block layer SG support v4 (EXPERIMENTAL)"
 	depends on EXPERIMENTAL
fs/ext4/super.c +4 -4
index e4a241c..04158ad 100644
@@ -1721,7 +1721,7 @@ static loff_t ext4_max_size(int blkbits, int has_huge_files)
 	/* small i_blocks in vfs inode? */
 	if (!has_huge_files || sizeof(blkcnt_t) < sizeof(u64)) {
 		/*
-		 * CONFIG_LSF is not enabled implies the inode
+		 * CONFIG_LBD is not enabled implies the inode
 		 * i_block represent total blocks in 512 bytes
 		 * 32 == size of vfs inode i_blocks * 8
 		 */
@@ -1764,7 +1764,7 @@ static loff_t ext4_max_bitmap_size(int bits, int has_huge_files)
 
 	if (!has_huge_files || sizeof(blkcnt_t) < sizeof(u64)) {
 		/*
-		 * !has_huge_files or CONFIG_LSF is not enabled
+		 * !has_huge_files or CONFIG_LBD is not enabled
 		 * implies the inode i_block represent total blocks in
 		 * 512 bytes 32 == size of vfs inode i_blocks * 8
 		 */
@@ -2021,13 +2021,13 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent)
 	if (has_huge_files) {
 		/*
 		 * Large file size enabled file system can only be
-		 * mount if kernel is build with CONFIG_LSF
+		 * mount if kernel is build with CONFIG_LBD
 		 */
 		if (sizeof(root->i_blocks) < sizeof(u64) &&
 				!(sb->s_flags & MS_RDONLY)) {
 			printk(KERN_ERR "EXT4-fs: %s: Filesystem with huge "
 					"files cannot be mounted read-write "
-					"without CONFIG_LSF.\n", sb->s_id);
+					"without CONFIG_LBD.\n", sb->s_id);
 			goto failed_mount;
 		}
 	}
include/linux/types.h +0 -0
index 1d98330..121f349 100644

include/linux/types.h typedef __s64 int64_t; · line 140

Quoted 6 lines
include/linux/types.h +2 -0
@@ -135,19 +135,14 @@ typedef		__s64		int64_t;
  *
  * Linux always considers sectors to be 512 bytes long independently
  * of the devices real block size.
+ *
+ * blkcnt_t is the type of the inode's block count.
  */

You might want to edit the start of this comment a bit, as it currently says "The type used for...". You probably want instead "sector_t is the type...". And drop the extra "*" while you're here, as this is a regular comment and not a kerneldoc thing.

Quoted 6 lines
include/linux/types.h +0 -4
 #ifdef CONFIG_LBD
 typedef u64 sector_t;
-#else
-typedef unsigned long sector_t;
-#endif
-

include/linux/types.h typedef __s64 int64_t; · line 148

Quoted 10 lines
include/linux/types.h +1 -4
-/*
- * The type of the inode's block count.
- */
-#ifdef CONFIG_LSF
 typedef u64 blkcnt_t;
 #else
+typedef unsigned long sector_t;
 typedef unsigned long blkcnt_t;
 #endif
 
Signature · 1 line
Jean Delvare
#20 lore raw
Quoted 8 lines · Jean Delvare
Quoted 8 lines
include/linux/types.h +0 -0
index 1d98330..121f349 100644
@@ -135,19 +135,14 @@ typedef		__s64		int64_t;
  *
  * Linux always considers sectors to be 512 bytes long independently
  * of the devices real block size.
Quoted 8 lines

include/linux/types.h typedef __s64 int64_t; · line 140

Quoted 3 lines
include/linux/types.h +2 -0
+ *
+ * blkcnt_t is the type of the inode's block count.
  */

You might want to edit the start of this comment a bit, as it currently says "The type used for...". You probably want instead "sector_t is the type...". And drop the extra "*" while you're here, as this is a regular comment and not a kerneldoc thing.

Agree, that'll make it cleaner. I'll make those changes next time I have to shuffle the patches.

Quoted 10 lines
Quoted 10 lines
include/linux/types.h +0 -8
 #ifdef CONFIG_LBD
 typedef u64 sector_t;
-#else
-typedef unsigned long sector_t;
-#endif
-
-/*
- * The type of the inode's block count.
- */
-#ifdef CONFIG_LSF
Quoted 8 lines

include/linux/types.h typedef __s64 int64_t; · line 148

Quoted 6 lines
include/linux/types.h +1 -0
 typedef u64 blkcnt_t;
 #else
+typedef unsigned long sector_t;
 typedef unsigned long blkcnt_t;
 #endif
 

Thanks, I added your Acked-by.

Signature · 1 line
Jens Axboe