* [PATCH] ext4: do not normalize block requests from fallocate. @ 2011-05-13 21:19 Vivek Haldar 2011-05-16 9:39 ` Lukas Czerner 2011-05-23 1:11 ` Ted Ts'o 0 siblings, 2 replies; 5+ messages in thread From: Vivek Haldar @ 2011-05-13 21:19 UTC (permalink / raw) To: Theodore Ts'o, Andreas Dilger; +Cc: linux-ext4, linux-kernel, Vivek Haldar Currently, an fallocate request of size slightly larger than a power of 2 is turned into two block requests, each a power of 2, with the extra blocks pre-allocated for future use. When an application calls fallocate, it already has an idea about how large the file may grow so there is usually little benefit to reserve extra blocks on the preallocation list. This reduces disk fragmentation. Tested: fsstress. Also verified manually that fallocat'ed files are contiguously laid out with this change (whereas without it they begin at power-of-2 boundaries, leaving blocks in between). CPU usage of fallocate is not appreciably higher. In a tight fallocate loop, CPU usage hovers between 5%-8% with this change, and 5%-7% without it. Signed-off-by: Vivek Haldar <haldar@google.com> --- fs/ext4/ext4.h | 2 ++ fs/ext4/extents.c | 5 ++++- 2 files changed, 6 insertions(+), 1 deletions(-) diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h index 076c5d2..e606d34 100644 --- a/fs/ext4/ext4.h +++ b/fs/ext4/ext4.h @@ -512,6 +512,8 @@ struct ext4_new_group_data { /* Convert extent to initialized after IO complete */ #define EXT4_GET_BLOCKS_IO_CONVERT_EXT (EXT4_GET_BLOCKS_CONVERT|\ EXT4_GET_BLOCKS_CREATE_UNINIT_EXT) + /* Don't normalize when fallocat'ing */ +#define EXT4_GET_BLOCKS_NO_NORMALIZE 0x0020 /* * Flags used by ext4_free_blocks diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c index e363f21..f02dd52 100644 --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -3304,6 +3304,8 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode, else /* disable in-core preallocation for non-regular files */ ar.flags = 0; + if (flags & EXT4_GET_BLOCKS_NO_NORMALIZE) + ar.flags |= EXT4_MB_HINT_NOPREALLOC; newblock = ext4_mb_new_blocks(handle, &ar, &err); if (!newblock) goto out2; @@ -3549,7 +3551,8 @@ retry: break; } ret = ext4_map_blocks(handle, inode, &map, - EXT4_GET_BLOCKS_CREATE_UNINIT_EXT); + EXT4_GET_BLOCKS_CREATE_UNINIT_EXT | + EXT4_GET_BLOCKS_NO_NORMALIZE); if (ret <= 0) { #ifdef EXT4FS_DEBUG WARN_ON(ret <= 0); -- 1.7.3.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ext4: do not normalize block requests from fallocate. 2011-05-13 21:19 [PATCH] ext4: do not normalize block requests from fallocate Vivek Haldar @ 2011-05-16 9:39 ` Lukas Czerner 2011-05-16 17:18 ` Vivek Haldar 2011-05-23 1:11 ` Ted Ts'o 1 sibling, 1 reply; 5+ messages in thread From: Lukas Czerner @ 2011-05-16 9:39 UTC (permalink / raw) To: Vivek Haldar; +Cc: Theodore Ts'o, Andreas Dilger, linux-ext4, linux-kernel On Fri, 13 May 2011, Vivek Haldar wrote: > Currently, an fallocate request of size slightly larger than a power of > 2 is turned into two block requests, each a power of 2, with the extra > blocks pre-allocated for future use. When an application calls > fallocate, it already has an idea about how large the file may grow so > there is usually little benefit to reserve extra blocks on the > preallocation list. This reduces disk fragmentation. > > Tested: fsstress. Also verified manually that fallocat'ed files are > contiguously laid out with this change (whereas without it they begin at > power-of-2 boundaries, leaving blocks in between). CPU usage of > fallocate is not appreciably higher. In a tight fallocate loop, CPU > usage hovers between 5%-8% with this change, and 5%-7% without it. Hi Vivek, the patch looks good, but I do not understand why you are introducing new #define when you can simply use EXT4_MB_HINT_NOPREALLOC and then in the ext4_ext_map_blocks() just do : ar.flags |= (flags & EXT4_MB_HINT_NOPREALLOC) and you do not need to introduce new condition. Am I missing something ? Thanks! -Lukas > > Signed-off-by: Vivek Haldar <haldar@google.com> > --- > fs/ext4/ext4.h | 2 ++ > fs/ext4/extents.c | 5 ++++- > 2 files changed, 6 insertions(+), 1 deletions(-) > > diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > index 076c5d2..e606d34 100644 > --- a/fs/ext4/ext4.h > +++ b/fs/ext4/ext4.h > @@ -512,6 +512,8 @@ struct ext4_new_group_data { > /* Convert extent to initialized after IO complete */ > #define EXT4_GET_BLOCKS_IO_CONVERT_EXT (EXT4_GET_BLOCKS_CONVERT|\ > EXT4_GET_BLOCKS_CREATE_UNINIT_EXT) > + /* Don't normalize when fallocat'ing */ > +#define EXT4_GET_BLOCKS_NO_NORMALIZE 0x0020 > > /* > * Flags used by ext4_free_blocks > diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > index e363f21..f02dd52 100644 > --- a/fs/ext4/extents.c > +++ b/fs/ext4/extents.c > @@ -3304,6 +3304,8 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode, > else > /* disable in-core preallocation for non-regular files */ > ar.flags = 0; > + if (flags & EXT4_GET_BLOCKS_NO_NORMALIZE) > + ar.flags |= EXT4_MB_HINT_NOPREALLOC; > newblock = ext4_mb_new_blocks(handle, &ar, &err); > if (!newblock) > goto out2; > @@ -3549,7 +3551,8 @@ retry: > break; > } > ret = ext4_map_blocks(handle, inode, &map, > - EXT4_GET_BLOCKS_CREATE_UNINIT_EXT); > + EXT4_GET_BLOCKS_CREATE_UNINIT_EXT | > + EXT4_GET_BLOCKS_NO_NORMALIZE); > if (ret <= 0) { > #ifdef EXT4FS_DEBUG > WARN_ON(ret <= 0); > -- ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ext4: do not normalize block requests from fallocate. 2011-05-16 9:39 ` Lukas Czerner @ 2011-05-16 17:18 ` Vivek Haldar 2011-05-17 6:47 ` Lukas Czerner 0 siblings, 1 reply; 5+ messages in thread From: Vivek Haldar @ 2011-05-16 17:18 UTC (permalink / raw) To: Lukas Czerner; +Cc: Theodore Ts'o, Andreas Dilger, linux-ext4, linux-kernel EXT4_GET_BLOCKS_* and EXT4_MB_HINT_* are different flag spaces and should not be mixed. ext4_ext_map_blocks should not be using EXT4_MB_HINT_*, it only uses EXT4_GET_BLOCKS_*. On Mon, May 16, 2011 at 2:39 AM, Lukas Czerner <lczerner@redhat.com> wrote: > On Fri, 13 May 2011, Vivek Haldar wrote: > >> Currently, an fallocate request of size slightly larger than a power of >> 2 is turned into two block requests, each a power of 2, with the extra >> blocks pre-allocated for future use. When an application calls >> fallocate, it already has an idea about how large the file may grow so >> there is usually little benefit to reserve extra blocks on the >> preallocation list. This reduces disk fragmentation. >> >> Tested: fsstress. Also verified manually that fallocat'ed files are >> contiguously laid out with this change (whereas without it they begin at >> power-of-2 boundaries, leaving blocks in between). CPU usage of >> fallocate is not appreciably higher. In a tight fallocate loop, CPU >> usage hovers between 5%-8% with this change, and 5%-7% without it. > > Hi Vivek, > > the patch looks good, but I do not understand why you are introducing > new #define when you can simply use EXT4_MB_HINT_NOPREALLOC and then in > the ext4_ext_map_blocks() just do : > > ar.flags |= (flags & EXT4_MB_HINT_NOPREALLOC) > > and you do not need to introduce new condition. Am I missing something ? > > Thanks! > -Lukas > >> >> Signed-off-by: Vivek Haldar <haldar@google.com> >> --- >> fs/ext4/ext4.h | 2 ++ >> fs/ext4/extents.c | 5 ++++- >> 2 files changed, 6 insertions(+), 1 deletions(-) >> >> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h >> index 076c5d2..e606d34 100644 >> --- a/fs/ext4/ext4.h >> +++ b/fs/ext4/ext4.h >> @@ -512,6 +512,8 @@ struct ext4_new_group_data { >> /* Convert extent to initialized after IO complete */ >> #define EXT4_GET_BLOCKS_IO_CONVERT_EXT (EXT4_GET_BLOCKS_CONVERT|\ >> EXT4_GET_BLOCKS_CREATE_UNINIT_EXT) >> + /* Don't normalize when fallocat'ing */ >> +#define EXT4_GET_BLOCKS_NO_NORMALIZE 0x0020 >> >> /* >> * Flags used by ext4_free_blocks >> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c >> index e363f21..f02dd52 100644 >> --- a/fs/ext4/extents.c >> +++ b/fs/ext4/extents.c >> @@ -3304,6 +3304,8 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode, >> else >> /* disable in-core preallocation for non-regular files */ >> ar.flags = 0; >> + if (flags & EXT4_GET_BLOCKS_NO_NORMALIZE) >> + ar.flags |= EXT4_MB_HINT_NOPREALLOC; >> newblock = ext4_mb_new_blocks(handle, &ar, &err); >> if (!newblock) >> goto out2; >> @@ -3549,7 +3551,8 @@ retry: >> break; >> } >> ret = ext4_map_blocks(handle, inode, &map, >> - EXT4_GET_BLOCKS_CREATE_UNINIT_EXT); >> + EXT4_GET_BLOCKS_CREATE_UNINIT_EXT | >> + EXT4_GET_BLOCKS_NO_NORMALIZE); >> if (ret <= 0) { >> #ifdef EXT4FS_DEBUG >> WARN_ON(ret <= 0); >> > > -- > -- Regards, Vivek ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ext4: do not normalize block requests from fallocate. 2011-05-16 17:18 ` Vivek Haldar @ 2011-05-17 6:47 ` Lukas Czerner 0 siblings, 0 replies; 5+ messages in thread From: Lukas Czerner @ 2011-05-17 6:47 UTC (permalink / raw) To: Vivek Haldar Cc: Lukas Czerner, Theodore Ts'o, Andreas Dilger, linux-ext4, linux-kernel [-- Attachment #1: Type: TEXT/PLAIN, Size: 3620 bytes --] On Mon, 16 May 2011, Vivek Haldar wrote: > EXT4_GET_BLOCKS_* and EXT4_MB_HINT_* are different flag spaces and > should not be mixed. ext4_ext_map_blocks should not be using > EXT4_MB_HINT_*, it only uses EXT4_GET_BLOCKS_*. Agh, yes you're right of course, somehow I just missed that ext4_map_blocks() take separate flag argument and EXT4_GET_* flag might use the same bit as EXT4_MB_HINT_NOPREALLOC. Sorry. Thanks! -Lukas > > On Mon, May 16, 2011 at 2:39 AM, Lukas Czerner <lczerner@redhat.com> wrote: > > On Fri, 13 May 2011, Vivek Haldar wrote: > > > >> Currently, an fallocate request of size slightly larger than a power of > >> 2 is turned into two block requests, each a power of 2, with the extra > >> blocks pre-allocated for future use. When an application calls > >> fallocate, it already has an idea about how large the file may grow so > >> there is usually little benefit to reserve extra blocks on the > >> preallocation list. This reduces disk fragmentation. > >> > >> Tested: fsstress. Also verified manually that fallocat'ed files are > >> contiguously laid out with this change (whereas without it they begin at > >> power-of-2 boundaries, leaving blocks in between). CPU usage of > >> fallocate is not appreciably higher. In a tight fallocate loop, CPU > >> usage hovers between 5%-8% with this change, and 5%-7% without it. > > > > Hi Vivek, > > > > the patch looks good, but I do not understand why you are introducing > > new #define when you can simply use EXT4_MB_HINT_NOPREALLOC and then in > > the ext4_ext_map_blocks() just do : > > > > ar.flags |= (flags & EXT4_MB_HINT_NOPREALLOC) > > > > and you do not need to introduce new condition. Am I missing something ? > > > > Thanks! > > -Lukas > > > >> > >> Signed-off-by: Vivek Haldar <haldar@google.com> > >> --- > >> fs/ext4/ext4.h | 2 ++ > >> fs/ext4/extents.c | 5 ++++- > >> 2 files changed, 6 insertions(+), 1 deletions(-) > >> > >> diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h > >> index 076c5d2..e606d34 100644 > >> --- a/fs/ext4/ext4.h > >> +++ b/fs/ext4/ext4.h > >> @@ -512,6 +512,8 @@ struct ext4_new_group_data { > >> /* Convert extent to initialized after IO complete */ > >> #define EXT4_GET_BLOCKS_IO_CONVERT_EXT (EXT4_GET_BLOCKS_CONVERT|\ > >> EXT4_GET_BLOCKS_CREATE_UNINIT_EXT) > >> + /* Don't normalize when fallocat'ing */ > >> +#define EXT4_GET_BLOCKS_NO_NORMALIZE 0x0020 > >> > >> /* > >> * Flags used by ext4_free_blocks > >> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c > >> index e363f21..f02dd52 100644 > >> --- a/fs/ext4/extents.c > >> +++ b/fs/ext4/extents.c > >> @@ -3304,6 +3304,8 @@ int ext4_ext_map_blocks(handle_t *handle, struct inode *inode, > >> else > >> /* disable in-core preallocation for non-regular files */ > >> ar.flags = 0; > >> + if (flags & EXT4_GET_BLOCKS_NO_NORMALIZE) > >> + ar.flags |= EXT4_MB_HINT_NOPREALLOC; > >> newblock = ext4_mb_new_blocks(handle, &ar, &err); > >> if (!newblock) > >> goto out2; > >> @@ -3549,7 +3551,8 @@ retry: > >> break; > >> } > >> ret = ext4_map_blocks(handle, inode, &map, > >> - EXT4_GET_BLOCKS_CREATE_UNINIT_EXT); > >> + EXT4_GET_BLOCKS_CREATE_UNINIT_EXT | > >> + EXT4_GET_BLOCKS_NO_NORMALIZE); > >> if (ret <= 0) { > >> #ifdef EXT4FS_DEBUG > >> WARN_ON(ret <= 0); > >> > > > > -- > > > > > > -- ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] ext4: do not normalize block requests from fallocate. 2011-05-13 21:19 [PATCH] ext4: do not normalize block requests from fallocate Vivek Haldar 2011-05-16 9:39 ` Lukas Czerner @ 2011-05-23 1:11 ` Ted Ts'o 1 sibling, 0 replies; 5+ messages in thread From: Ted Ts'o @ 2011-05-23 1:11 UTC (permalink / raw) To: Vivek Haldar; +Cc: Andreas Dilger, linux-ext4, linux-kernel On Fri, May 13, 2011 at 02:19:05PM -0700, Vivek Haldar wrote: > Currently, an fallocate request of size slightly larger than a power of > 2 is turned into two block requests, each a power of 2, with the extra > blocks pre-allocated for future use. When an application calls > fallocate, it already has an idea about how large the file may grow so > there is usually little benefit to reserve extra blocks on the > preallocation list. This reduces disk fragmentation. > > Tested: fsstress. Also verified manually that fallocat'ed files are > contiguously laid out with this change (whereas without it they begin at > power-of-2 boundaries, leaving blocks in between). CPU usage of > fallocate is not appreciably higher. In a tight fallocate loop, CPU > usage hovers between 5%-8% with this change, and 5%-7% without it. > > Signed-off-by: Vivek Haldar <haldar@google.com> Applied, with a minor change to avoid a bit assignment conflict with the punch patches. Also, I changed the commit message to note that using a file aging simulator which filled the file system to 70%, the percentage of free extents greater than 8MB (as measured using e2freefrag) increased from 38.8% without this commit, to 69.4% with this commit. - Ted ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-05-23 1:11 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-05-13 21:19 [PATCH] ext4: do not normalize block requests from fallocate Vivek Haldar 2011-05-16 9:39 ` Lukas Czerner 2011-05-16 17:18 ` Vivek Haldar 2011-05-17 6:47 ` Lukas Czerner 2011-05-23 1:11 ` Ted Ts'o
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox