[PATCH] block: Fix LSF default inconsistency

[PATCH] block: Fix LSF default inconsistency

[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.

Signed-off-by: Jean Delvare <khali@linux-fr.org>
Cc: Jens Axboe <axboe@kernel.dk>
Cc: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 block/Kconfig |    1 +
 1 file changed, 1 insertion(+)

--- linux-2.6.28-rc8.orig/block/Kconfig	2008-10-10 09:27:54.000000000 +0200
+++ linux-2.6.28-rc8/block/Kconfig	2008-12-11 11:09:33.000000000 +0100
@@ -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.


-- 
Jean Delvare

Re: [PATCH] block: Fix LSF default inconsistency

[Jens Axboe]

On Thu, Dec 11 2008, Jean Delvare wrote:
> 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?

-- 
Jens Axboe

Re: [PATCH] block: Fix LSF default inconsistency

[Jean Delvare]

On Thu, 11 Dec 2008 11:41:10 +0100, Jens Axboe wrote:
> On Thu, Dec 11 2008, Jean Delvare wrote:
> > 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?

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

-- 
Jean Delvare

Re: [PATCH] block: Fix LSF default inconsistency

[Jens Axboe]

On Thu, Dec 11 2008, Jean Delvare wrote:
> On Thu, 11 Dec 2008 11:41:10 +0100, Jens Axboe wrote:
> > On Thu, Dec 11 2008, Jean Delvare wrote:
> > > 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?
> 
> 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...

diff --git a/block/Kconfig b/block/Kconfig
index 1ab7c15..ce566dd 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -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

-- 
Jens Axboe

Re: [PATCH] block: Fix LSF default inconsistency

[KOSAKI Motohiro]

> > 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.

Re: [PATCH] block: Fix LSF default inconsistency

[Jens Axboe]

On Thu, Dec 11 2008, KOSAKI Motohiro wrote:
> > > 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?

-- 
Jens Axboe

Re: [PATCH] block: Fix LSF default inconsistency

[KOSAKI Motohiro]

> On Thu, Dec 11 2008, KOSAKI Motohiro wrote:
> > > > 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?

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.

Re: [PATCH] block: Fix LSF default inconsistency

[Jens Axboe]

On Thu, Dec 11 2008, KOSAKI Motohiro wrote:
> > On Thu, Dec 11 2008, KOSAKI Motohiro wrote:
> > > > > 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?
> 
> 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?

-- 
Jens Axboe

Re: [PATCH] block: Fix LSF default inconsistency

[Rik van Riel]

Jens Axboe wrote:

> 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...

-- 
All rights reversed.

Re: [PATCH] block: Fix LSF default inconsistency

[KOSAKI Motohiro]

>> 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.

Re: [PATCH] block: Fix LSF default inconsistency

[Hugh Dickins]

On Sat, 13 Dec 2008, KOSAKI Motohiro wrote:
> >> 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.

> 
> 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.

Re: [PATCH] block: Fix LSF default inconsistency

[Rik van Riel]

Hugh Dickins wrote:
> On Sat, 13 Dec 2008, KOSAKI Motohiro wrote:
>   
>> 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.

Re: [PATCH] block: Fix LSF default inconsistency

[Hugh Dickins]

On Fri, 12 Dec 2008, Rik van Riel wrote:
> Hugh Dickins wrote:
> > On Sat, 13 Dec 2008, KOSAKI Motohiro wrote:
> >   
> > > 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.

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

Re: [PATCH] block: Fix LSF default inconsistency

[Rik van Riel]

Hugh Dickins wrote:

> 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.

-- 
All Rights Reversed

Re: [PATCH] block: Fix LSF default inconsistency

[Hugh Dickins]

On Fri, 12 Dec 2008, Rik van Riel wrote:
> 
> 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

Re: [PATCH] block: Fix LSF default inconsistency

[Jean Delvare]

Hi Jens,

On Thu, 11 Dec 2008 12:08:39 +0100, Jens Axboe wrote:
> Anyway, how about something like this? Totally untested...
> 
> diff --git a/block/Kconfig b/block/Kconfig
> index 1ab7c15..ce566dd 100644
> --- a/block/Kconfig
> +++ b/block/Kconfig
> @@ -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...

>  	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

... 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.

> -	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
> 


-- 
Jean Delvare

Re: [PATCH] block: Fix LSF default inconsistency

[Jens Axboe]

On Thu, Dec 11 2008, Jean Delvare wrote:
> Hi Jens,
> 
> On Thu, 11 Dec 2008 12:08:39 +0100, Jens Axboe wrote:
> > Anyway, how about something like this? Totally untested...
> > 
> > diff --git a/block/Kconfig b/block/Kconfig
> > index 1ab7c15..ce566dd 100644
> > --- a/block/Kconfig
> > +++ b/block/Kconfig
> > @@ -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...
> 
> >  	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
> 
> ... 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.

diff --git a/block/Kconfig b/block/Kconfig
index 1ab7c15..a785e65 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -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

-- 
Jens Axboe

Re: [PATCH] block: Fix LSF default inconsistency

[Jean Delvare]

Hi Jens,

On Thu, 11 Dec 2008 13:50:01 +0100, Jens Axboe wrote:
> Oh right, that's pretty dumb. Lets just do the former, then a later
> patch can replace it completely if wanted.
> 
> diff --git a/block/Kconfig b/block/Kconfig
> index 1ab7c15..a785e65 100644
> --- a/block/Kconfig
> +++ b/block/Kconfig
> @@ -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.

>  	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
> 

Acked-by: Jean Delvare <khali@linux-fr.org>

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

-- 
Jean Delvare

Re: [PATCH] block: Fix LSF default inconsistency

[Jens Axboe]

On Fri, Dec 12 2008, Jean Delvare wrote:
> Hi Jens,
> 
> On Thu, 11 Dec 2008 13:50:01 +0100, Jens Axboe wrote:
> > Oh right, that's pretty dumb. Lets just do the former, then a later
> > patch can replace it completely if wanted.
> > 
> > diff --git a/block/Kconfig b/block/Kconfig
> > index 1ab7c15..a785e65 100644
> > --- a/block/Kconfig
> > +++ b/block/Kconfig
> > @@ -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.

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

> >  	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
> > 
> 
> Acked-by: Jean Delvare <khali@linux-fr.org>
> 
> 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 :-)

diff --git a/block/Kconfig b/block/Kconfig
index 1ab7c15..195968e 100644
--- a/block/Kconfig
+++ b/block/Kconfig
@@ -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
diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index e4a241c..04158ad 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -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;
 		}
 	}
diff --git a/include/linux/types.h b/include/linux/types.h
index 1d98330..121f349 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -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
 

-- 
Jens Axboe

Re: [PATCH] block: Fix LSF default inconsistency

[Jean Delvare]

Hi Jens,

On Fri, 12 Dec 2008 08:54:33 +0100, Jens Axboe wrote:
> Agree, it's pretty ugly. Modified patch below, it drops CONFIG_LSF
> completely as it's basically only used in a single spot.

> On Fri, Dec 12 2008, Jean Delvare wrote:
> > 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.

> diff --git a/block/Kconfig b/block/Kconfig
> index 1ab7c15..195968e 100644
> --- a/block/Kconfig
> +++ b/block/Kconfig
> @@ -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
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index e4a241c..04158ad 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -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;
>  		}
>  	}
> diff --git a/include/linux/types.h b/include/linux/types.h
> index 1d98330..121f349 100644
> --- a/include/linux/types.h
> +++ b/include/linux/types.h
> @@ -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.

>  #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
>  

Acked-by: Jean Delvare <khali@linux-fr.org>

-- 
Jean Delvare

Re: [PATCH] block: Fix LSF default inconsistency

[Jens Axboe]

On Fri, Dec 12 2008, Jean Delvare wrote:
> > diff --git a/include/linux/types.h b/include/linux/types.h
> > index 1d98330..121f349 100644
> > --- a/include/linux/types.h
> > +++ b/include/linux/types.h
> > @@ -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.

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

> >  #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
> >  
> 
> Acked-by: Jean Delvare <khali@linux-fr.org>

Thanks, I added your Acked-by.

-- 
Jens Axboe