Re: [RFC][PATCH 2/3] Move the file data to the new blocks
On Thu, Feb 08, 2007 at 11:47:39AM +0100, Jan Kara wrote: Well. Do we really? Are we looking for a 100% solution here, or a 90% one? Umm, I think that for ext3 having data on one end of the disk and indirect blocks on the other end of the disk does not quite help (not mentioning that it can create bad free space fragmentation over the time). I have not measured it but I'd guess that it would erase the effect of moving data closer together. At least for sequential reads.. I don't think anyone is saying we can ignore the metadata; but the fact is, the cleanest solution for 90% of the problem is to use the page cache, and as far as the other 10%, Linus has been pushing us to move at least the directories into the page cache, and it's not insane to consider moving the rest of the metadata into page cache. At least it's something we should consider carefully. - Ted - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 2/3] Move the file data to the new blocks
On Thu, 8 Feb 2007 10:29:45 +0100 Jan Kara [EMAIL PROTECTED] wrote:
On Wed 07-02-07 12:56:59, Andrew Morton wrote:
On Wed, 7 Feb 2007 13:46:57 -0700
Andreas Dilger [EMAIL PROTECTED] wrote:
On Feb 06, 2007 17:35 -0800, Andrew Morton wrote:
On Mon, 5 Feb 2007 14:12:04 +0100
Jan Kara [EMAIL PROTECTED] wrote:
Move the blocks on the temporary inode to the original inode
by a page.
1. Read the file data from the old blocks to the page
2. Move the block on the temporary inode to the original inode
3. Write the file data on the page into the new blocks
I have one thing - it's probably not good to use page cache for
defragmentation.
Then it is no longer online defragmentation. The issues with
maintaining
correctness and coherency with ongoing VFS activity would be truly
ghastly.
If we're worried about pagecache pollution then it would be better to
control
that from userspace via fadvise().
It should be possible to have the online defrag tool lock the inode
against
any changes,
Sounds easy when you say it fast. But how do we lock against, say, a
read pagefault? Only by writing back then removing the pagecache page then
reinstantiating it as a locked, not-uptodate page and then removing it from
pagecache afterwards prior to unlocking it. Or something.
I don't think we want to go there.
I though Andreas meant any write changes - i.e. you check that noone
has open file descriptor for writing and block any new open for writing.
That can be done quite easily.
Anyway, I agree with you that userspace solution to a possible page
cache pollution is preferable after thinking about it for a while.
As I've been thinking about it, we could actually do the copying
from user space. We could do something like:
block any writes to file (as I described above)
craft new inode with blocks allocated as we want (using preallocation,
we should mostly have the kernel infrastructure we need)
copy data using splice syscall
call the kernel to switch data
I don't think we need to block any writes to any file or anything.
To move a page within a file:
fd = open(file);
p = mmap(fd);
the_page_was_in_core = mincore(p, offset);
munmap(p);
ioctl(fd, ..., new_block);
kernel
read_cache_page(inode, offset);
lock_page(page);
if (try_to_free_buffers(page)) {
relocate the page
set_page_dirty(page);
}
unlock_page(page);
if (the_page_was_in_core) {
sync_file_range(fd, offset SYNC_FILE_RANGE_WAIT_BEFORE|
SYNC_FILE_RANGE_WRITE|
SYNC_FILE_RANGE_WAIT_AFTER);
fadvise(fd, offset, FADV_DONTNEED);
}
completely coherent with pagecache, quite safe in the presence of mmap,
mlock, O_DIRECT, everything else. Also fully journallable in-kernel.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 2/3] Move the file data to the new blocks
On Thu 08-02-07 01:45:29, Andrew Morton wrote:
snip
I though Andreas meant any write changes - i.e. you check that noone
has open file descriptor for writing and block any new open for writing.
That can be done quite easily.
Anyway, I agree with you that userspace solution to a possible page
cache pollution is preferable after thinking about it for a while.
As I've been thinking about it, we could actually do the copying
from user space. We could do something like:
block any writes to file (as I described above)
craft new inode with blocks allocated as we want (using preallocation,
we should mostly have the kernel infrastructure we need)
copy data using splice syscall
call the kernel to switch data
I don't think we need to block any writes to any file or anything.
To move a page within a file:
fd = open(file);
p = mmap(fd);
the_page_was_in_core = mincore(p, offset);
munmap(p);
ioctl(fd, ..., new_block);
kernel
read_cache_page(inode, offset);
lock_page(page);
if (try_to_free_buffers(page)) {
relocate the page
set_page_dirty(page);
}
unlock_page(page);
if (the_page_was_in_core) {
sync_file_range(fd, offset SYNC_FILE_RANGE_WAIT_BEFORE|
SYNC_FILE_RANGE_WRITE|
SYNC_FILE_RANGE_WAIT_AFTER);
fadvise(fd, offset, FADV_DONTNEED);
}
completely coherent with pagecache, quite safe in the presence of mmap,
mlock, O_DIRECT, everything else. Also fully journallable in-kernel.
Yes, this is the simple way. But I see two disadvantages:
1) You'd like to relocate metadata (indirect blocks) too. For that you need
a different mechanism. In my approach, you can mostly assume you've got
sanely laid out metadata and so the existence of such mechanism is not
so important.
2) You'd like to allocate new blocks in big chunks. So your kernel function
should rather take a range. Also when you fail in the middle of
relocating a file (for example the block you'd like to use is already
taken by someone else), I find it nice if you can return at least to the
original state. But that's probably not important.
Honza
--
Jan Kara [EMAIL PROTECTED]
SuSE CR Labs
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 2/3] Move the file data to the new blocks
On Thu, 8 Feb 2007 11:21:02 +0100 Jan Kara [EMAIL PROTECTED] wrote:
On Thu 08-02-07 01:45:29, Andrew Morton wrote:
snip
I though Andreas meant any write changes - i.e. you check that noone
has open file descriptor for writing and block any new open for writing.
That can be done quite easily.
Anyway, I agree with you that userspace solution to a possible page
cache pollution is preferable after thinking about it for a while.
As I've been thinking about it, we could actually do the copying
from user space. We could do something like:
block any writes to file (as I described above)
craft new inode with blocks allocated as we want (using preallocation,
we should mostly have the kernel infrastructure we need)
copy data using splice syscall
call the kernel to switch data
I don't think we need to block any writes to any file or anything.
To move a page within a file:
fd = open(file);
p = mmap(fd);
the_page_was_in_core = mincore(p, offset);
munmap(p);
ioctl(fd, ..., new_block);
kernel
read_cache_page(inode, offset);
lock_page(page);
if (try_to_free_buffers(page)) {
relocate the page
set_page_dirty(page);
}
unlock_page(page);
if (the_page_was_in_core) {
sync_file_range(fd, offset SYNC_FILE_RANGE_WAIT_BEFORE|
SYNC_FILE_RANGE_WRITE|
SYNC_FILE_RANGE_WAIT_AFTER);
fadvise(fd, offset, FADV_DONTNEED);
}
completely coherent with pagecache, quite safe in the presence of mmap,
mlock, O_DIRECT, everything else. Also fully journallable in-kernel.
Yes, this is the simple way. But I see two disadvantages:
1) You'd like to relocate metadata (indirect blocks) too.
Well. Do we really? Are we looking for a 100% solution here, or a 90% one?
Relocating data is the main thing. After that, yeah, relocating metadata,
inodes and directories is probably a second-order thing.
For that you need
a different mechanism.
I suspect a similar approach will work there: load and lock the
buffer_heads (or maybe just the top-level buffer_head) and then alter their
contents. It could be that verify_chain() will just magically do the right
thing there, but some changes might be needed.
In my approach, you can mostly assume you've got
sanely laid out metadata and so the existence of such mechanism is not
so important.
2) You'd like to allocate new blocks in big chunks. So your kernel function
should rather take a range. Also when you fail in the middle of
relocating a file (for example the block you'd like to use is already
taken by someone else), I find it nice if you can return at least to the
original state. But that's probably not important.
Well yes, that was a minimal sketch.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 2/3] Move the file data to the new blocks
On Thu 08-02-07 02:32:13, Andrew Morton wrote:
On Thu, 8 Feb 2007 11:21:02 +0100 Jan Kara [EMAIL PROTECTED] wrote:
On Thu 08-02-07 01:45:29, Andrew Morton wrote:
snip
I though Andreas meant any write changes - i.e. you check that noone
has open file descriptor for writing and block any new open for writing.
That can be done quite easily.
Anyway, I agree with you that userspace solution to a possible page
cache pollution is preferable after thinking about it for a while.
As I've been thinking about it, we could actually do the copying
from user space. We could do something like:
block any writes to file (as I described above)
craft new inode with blocks allocated as we want (using preallocation,
we should mostly have the kernel infrastructure we need)
copy data using splice syscall
call the kernel to switch data
I don't think we need to block any writes to any file or anything.
To move a page within a file:
fd = open(file);
p = mmap(fd);
the_page_was_in_core = mincore(p, offset);
munmap(p);
ioctl(fd, ..., new_block);
kernel
read_cache_page(inode, offset);
lock_page(page);
if (try_to_free_buffers(page)) {
relocate the page
set_page_dirty(page);
}
unlock_page(page);
if (the_page_was_in_core) {
sync_file_range(fd, offset SYNC_FILE_RANGE_WAIT_BEFORE|
SYNC_FILE_RANGE_WRITE|
SYNC_FILE_RANGE_WAIT_AFTER);
fadvise(fd, offset, FADV_DONTNEED);
}
completely coherent with pagecache, quite safe in the presence of mmap,
mlock, O_DIRECT, everything else. Also fully journallable in-kernel.
Yes, this is the simple way. But I see two disadvantages:
1) You'd like to relocate metadata (indirect blocks) too.
Well. Do we really? Are we looking for a 100% solution here, or a 90% one?
Umm, I think that for ext3 having data on one end of the disk and
indirect blocks on the other end of the disk does not quite help (not
mentioning that it can create bad free space fragmentation over the time).
I have not measured it but I'd guess that it would erase the effect of
moving data closer together. At least for sequential reads..
Relocating data is the main thing. After that, yeah, relocating metadata,
inodes and directories is probably a second-order thing.
For that you need
a different mechanism.
I suspect a similar approach will work there: load and lock the
buffer_heads (or maybe just the top-level buffer_head) and then alter their
contents. It could be that verify_chain() will just magically do the right
thing there, but some changes might be needed.
Yes, it could be done. I just wanted to point to the fact that things may
not be as simple in your solution either...
Honza
--
Jan Kara [EMAIL PROTECTED]
SuSE CR Labs
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 2/3] Move the file data to the new blocks
Hi,
+ext4_ext_replace_branches(struct inode *org_inode, struct inode *dest_inode,
+ pgoff_t from_page, pgoff_t dest_from_page,
+ pgoff_t count_page, unsigned long *delete_start) +{
+ struct ext4_ext_path *org_path = NULL;
+ struct ext4_ext_path *dest_path = NULL;
+ struct ext4_extent *oext, *dext;
+ struct ext4_extent tmp_ext;
+ int err = 0;
+ int depth;
+ unsigned long from, count, dest_off, diff, replaced_count = 0;
These should be sector_t, shouldn't they?
At some point should we start using blkcnt_t properly? (block-in[-large]-file, not
block-in[-large]-device?) I think that's what it was introduced for, although it's not
in wide use at this point.
I guess the type really isn't used anywhere else; just in the inode's i_blocks.
Hmm.
On reflection, I think we should use ext4_fsblk_t in this case, because
some ext4 codes such as ext4_ext_get_blocks() use it.
int ext4_ext_get_blocks(handle_t *handle, struct inode *inode,
ext4_fsblk_t iblock,
So I would like to change unsigned long into ext4_fsblk_t in my next patch.
Cheers, Takashi
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 2/3] Move the file data to the new blocks
On Feb 06, 2007 17:35 -0800, Andrew Morton wrote: On Mon, 5 Feb 2007 14:12:04 +0100 Jan Kara [EMAIL PROTECTED] wrote: Move the blocks on the temporary inode to the original inode by a page. 1. Read the file data from the old blocks to the page 2. Move the block on the temporary inode to the original inode 3. Write the file data on the page into the new blocks I have one thing - it's probably not good to use page cache for defragmentation. Then it is no longer online defragmentation. The issues with maintaining correctness and coherency with ongoing VFS activity would be truly ghastly. If we're worried about pagecache pollution then it would be better to control that from userspace via fadvise(). It should be possible to have the online defrag tool lock the inode against any changes, flush all pages out of the cache for that inode, and then do the reallocated outside of the page cache. For inodes not already in cache this is a no-op. For the (hopefully rare) case were the inode already has cached pages and also needs to be reallocated it would be a performance hit. Alternately, we could skip files currently being modified (or mmaped), or even recently modified (e.g. within the last 30 minutes) in the default case, on the assumption that they might be deleted soon anyways. Cheers, Andreas -- Andreas Dilger Principal Software Engineer Cluster File Systems, Inc. - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 2/3] Move the file data to the new blocks
On Wed, 7 Feb 2007 13:46:57 -0700 Andreas Dilger [EMAIL PROTECTED] wrote: On Feb 06, 2007 17:35 -0800, Andrew Morton wrote: On Mon, 5 Feb 2007 14:12:04 +0100 Jan Kara [EMAIL PROTECTED] wrote: Move the blocks on the temporary inode to the original inode by a page. 1. Read the file data from the old blocks to the page 2. Move the block on the temporary inode to the original inode 3. Write the file data on the page into the new blocks I have one thing - it's probably not good to use page cache for defragmentation. Then it is no longer online defragmentation. The issues with maintaining correctness and coherency with ongoing VFS activity would be truly ghastly. If we're worried about pagecache pollution then it would be better to control that from userspace via fadvise(). It should be possible to have the online defrag tool lock the inode against any changes, Sounds easy when you say it fast. But how do we lock against, say, a read pagefault? Only by writing back then removing the pagecache page then reinstantiating it as a locked, not-uptodate page and then removing it from pagecache afterwards prior to unlocking it. Or something. I don't think we want to go there. flush all pages out of the cache for that inode, and then do the reallocated outside of the page cache. For inodes not already in cache this is a no-op. For the (hopefully rare) case were the inode already has cached pages and also needs to be reallocated it would be a performance hit. Alternately, we could skip files currently being modified (or mmaped), or even recently modified (e.g. within the last 30 minutes) in the default case, on the assumption that they might be deleted soon anyways. argh. It's simple to just use pagecache. The we don't want to swamp the machine with pagecache argument is bogus. If it becomes a problem (and it might not) then it is very simple to control the pagecache from userspace. - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 2/3] Move the file data to the new blocks
On Tue, 16 Jan 2007 21:05:20 +0900
[EMAIL PROTECTED] wrote:
Move the blocks on the temporary inode to the original inode
by a page.
1. Read the file data from the old blocks to the page
2. Move the block on the temporary inode to the original inode
3. Write the file data on the page into the new blocks
...
+
+/**
+ * ext4_ext_replace_branches - replace original extents with new extents.
+ * @org_inodeOriginal inode
+ * @dest_inode temporary inode
+ * @from_pagePage offset
+ * @count_page Page count to be replaced
+ * @delete_start block offset for deletion
+ *
+ * This function returns 0 if succeed, otherwise returns error value.
+ * Replace extents for blocks from from to from+count-1.
+ */
+static int
+ext4_ext_replace_branches(struct inode *org_inode, struct inode *dest_inode,
+ pgoff_t from_page, pgoff_t dest_from_page,
+ pgoff_t count_page, unsigned long *delete_start)
+{
+ struct ext4_ext_path *org_path = NULL;
+ struct ext4_ext_path *dest_path = NULL;
+ struct ext4_extent *oext, *dext;
+ struct ext4_extent tmp_ext;
+ int err = 0;
+ int depth;
+ unsigned long from, count, dest_off, diff, replaced_count = 0;
These should be sector_t, shouldn't they?
+ handle_t *handle = NULL;
+ unsigned jnum;
+
+ from = from_page (PAGE_CACHE_SHIFT - dest_inode-i_blkbits);
In which case one needs to be very careful to avoid overflows in
expressions such as this one.
+ wait_on_page_locked(page);
+ lock_page(page);
The wait_on_page_locked() is unneeded here.
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 2/3] Move the file data to the new blocks
On Mon, 5 Feb 2007 14:12:04 +0100 Jan Kara [EMAIL PROTECTED] wrote: Move the blocks on the temporary inode to the original inode by a page. 1. Read the file data from the old blocks to the page 2. Move the block on the temporary inode to the original inode 3. Write the file data on the page into the new blocks I have one thing - it's probably not good to use page cache for defragmentation. Then it is no longer online defragmentation. The issues with maintaining correctness and coherency with ongoing VFS activity would be truly ghastly. If we're worried about pagecache pollution then it would be better to control that from userspace via fadvise(). - To unsubscribe from this list: send the line unsubscribe linux-fsdevel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC][PATCH 2/3] Move the file data to the new blocks
Andrew Morton wrote:
On Tue, 16 Jan 2007 21:05:20 +0900
[EMAIL PROTECTED] wrote:
...
+ext4_ext_replace_branches(struct inode *org_inode, struct inode *dest_inode,
+ pgoff_t from_page, pgoff_t dest_from_page,
+ pgoff_t count_page, unsigned long *delete_start)
+{
+ struct ext4_ext_path *org_path = NULL;
+ struct ext4_ext_path *dest_path = NULL;
+ struct ext4_extent *oext, *dext;
+ struct ext4_extent tmp_ext;
+ int err = 0;
+ int depth;
+ unsigned long from, count, dest_off, diff, replaced_count = 0;
These should be sector_t, shouldn't they?
At some point should we start using blkcnt_t properly?
(block-in[-large]-file, not block-in[-large]-device?) I think that's
what it was introduced for, although it's not in wide use at this point.
I guess the type really isn't used anywhere else; just in the inode's
i_blocks. Hmm.
-Eric
-
To unsubscribe from this list: send the line unsubscribe linux-fsdevel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
