I also have some questions, marked with XXX.
Cc: [email protected]
Cc: Christoph Hellwig <[email protected]>
Signed-off-by: Nick Piggin <[email protected]>
---
fs/ext2/ext2.h | 1
fs/ext2/file.c | 2
fs/ext2/inode.c | 138 +++++++++++++++++++++++++++++++++++++++++++++-----------
3 files changed, 113 insertions(+), 28 deletions(-)
Index: linux-2.6/fs/ext2/inode.c
===================================================================
--- linux-2.6.orig/fs/ext2/inode.c
+++ linux-2.6/fs/ext2/inode.c
@@ -53,6 +53,8 @@ static inline int ext2_inode_is_fast_sym
inode->i_blocks - ea_blocks == 0);
}
+static void ext2_truncate_blocks(struct inode *inode, loff_t offset);
+
/*
* Called at the last iput() if i_nlink is zero.
*/
@@ -66,9 +68,10 @@ void ext2_delete_inode (struct inode * i
mark_inode_dirty(inode);
ext2_write_inode(inode, inode_needs_sync(inode));
+ /* XXX: where is truncate_inode_pages? */
inode->i_size = 0;
if (inode->i_blocks)
- ext2_truncate (inode);
+ ext2_truncate_blocks(inode, 0);
ext2_free_inode (inode);
return;
@@ -761,8 +764,33 @@ ext2_write_begin(struct file *file, stru
loff_t pos, unsigned len, unsigned flags,
struct page **pagep, void **fsdata)
{
+ int ret;
+
*pagep = NULL;
- return __ext2_write_begin(file, mapping, pos, len, flags, pagep,fsdata);
+ ret = __ext2_write_begin(file, mapping, pos, len, flags, pagep,fsdata);
+ if (ret < 0) {
+ struct inode *inode = mapping->host;
+ loff_t isize = inode->i_size;
+ if (pos + len > isize)
+ ext2_truncate_blocks(inode, isize);
+ }
+ return ret;
+}
+
+static int ext2_write_end(struct file *file, struct address_space *mapping,
+ loff_t pos, unsigned len, unsigned copied,
+ struct page *page, void *fsdata)
+{
+ int ret;
+
+ ret = generic_write_end(file, mapping, pos, len, copied, page, fsdata);
+ if (ret < len) {
+ struct inode *inode = mapping->host;
+ loff_t isize = inode->i_size;
+ if (pos + len > isize)
+ ext2_truncate_blocks(inode, isize);
+ }
+ return ret;
}
static int
@@ -770,13 +798,22 @@ ext2_nobh_write_begin(struct file *file,
loff_t pos, unsigned len, unsigned flags,
struct page **pagep, void **fsdata)
{
+ int ret;
+
/*
* Dir-in-pagecache still uses ext2_write_begin. Would have to rework
* directory handling code to pass around offsets rather than struct
* pages in order to make this work easily.
*/
- return nobh_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+ ret = nobh_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
ext2_get_block);
+ if (ret < 0) {
+ struct inode *inode = mapping->host;
+ loff_t isize = inode->i_size;
+ if (pos + len > isize)
+ ext2_truncate_blocks(inode, isize);
+ }
+ return ret;
}
static int ext2_nobh_writepage(struct page *page,
@@ -796,9 +833,15 @@ ext2_direct_IO(int rw, struct kiocb *ioc
{
struct file *file = iocb->ki_filp;
struct inode *inode = file->f_mapping->host;
+ ssize_t ret;
- return blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
+ ret = blockdev_direct_IO(rw, iocb, inode, inode->i_sb->s_bdev, iov,
offset, nr_segs, ext2_get_block, NULL);
+ if (ret < 0 && (rw & WRITE)) {
+ loff_t isize = inode->i_size;
+ ext2_truncate_blocks(inode, isize);
+ }
+ return ret;
}
static int
@@ -813,7 +856,7 @@ const struct address_space_operations ex
.writepage = ext2_writepage,
.sync_page = block_sync_page,
.write_begin = ext2_write_begin,
- .write_end = generic_write_end,
+ .write_end = ext2_write_end,
.bmap = ext2_bmap,
.direct_IO = ext2_direct_IO,
.writepages = ext2_writepages,
@@ -1020,7 +1063,7 @@ static void ext2_free_branches(struct in
ext2_free_data(inode, p, q);
}
-void ext2_truncate(struct inode *inode)
+static void __ext2_truncate_blocks(struct inode *inode, loff_t offset)
{
__le32 *i_data = EXT2_I(inode)->i_data;
struct ext2_inode_info *ei = EXT2_I(inode);
@@ -1032,27 +1075,8 @@ void ext2_truncate(struct inode *inode)
int n;
long iblock;
unsigned blocksize;
-
- if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
- S_ISLNK(inode->i_mode)))
- return;
- if (ext2_inode_is_fast_symlink(inode))
- return;
- if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
- return;
-
blocksize = inode->i_sb->s_blocksize;
- iblock = (inode->i_size + blocksize-1)
- >> EXT2_BLOCK_SIZE_BITS(inode->i_sb);
-
- if (mapping_is_xip(inode->i_mapping))
- xip_truncate_page(inode->i_mapping, inode->i_size);
- else if (test_opt(inode->i_sb, NOBH))
- nobh_truncate_page(inode->i_mapping,
- inode->i_size, ext2_get_block);
- else
- block_truncate_page(inode->i_mapping,
- inode->i_size, ext2_get_block);
+ iblock = (offset + blocksize-1) >> EXT2_BLOCK_SIZE_BITS(inode->i_sb);
n = ext2_block_to_path(inode, iblock, offsets, NULL);
if (n == 0)
@@ -1120,6 +1144,59 @@ do_indirects:
ext2_discard_reservation(inode);
mutex_unlock(&ei->truncate_mutex);
+}
+
+static void ext2_truncate_blocks(struct inode *inode, loff_t offset)
+{
+ /*
+ * XXX: it seems like a bug here that we don't allow
+ * IS_APPEND inode to have blocks-past-i_size trimmed off.
+ * review and fix this.
+ */
+ if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
+ S_ISLNK(inode->i_mode)))
+ return -EINVAL;
+ if (ext2_inode_is_fast_symlink(inode))
+ return -EINVAL;
+ if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
+ return -EPERM;
+ __ext2_truncate_blocks(inode, offset);
+}
+
+int ext2_setsize(struct inode *inode, loff_t newsize)
+{
+ loff_t oldsize;
+ int error;
+
+ error = inode_newsize_ok(inode, newsize);
+ if (error)
+ return error;
+
+ if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
+ S_ISLNK(inode->i_mode)))
+ return -EINVAL;
+ if (ext2_inode_is_fast_symlink(inode))
+ return -EINVAL;
+ if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
+ return -EPERM;
+
+ if (mapping_is_xip(inode->i_mapping))
+ error = xip_truncate_page(inode->i_mapping, newsize);
+ else if (test_opt(inode->i_sb, NOBH))
+ error = nobh_truncate_page(inode->i_mapping,
+ newsize, ext2_get_block);
+ else
+ error = block_truncate_page(inode->i_mapping,
+ newsize, ext2_get_block);
+ if (error)
+ return error;
+
+ oldsize = inode->i_size;
+ i_size_write(inode, newsize);
+ truncate_pagecache(inode, oldsize, newsize);
+
+ __ext2_truncate_blocks(inode, newsize);
+
inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
if (inode_needs_sync(inode)) {
sync_mapping_buffers(inode->i_mapping);
@@ -1127,6 +1204,8 @@ do_indirects:
} else {
mark_inode_dirty(inode);
}
+
+ return 0;
}
static struct ext2_inode *ext2_get_inode(struct super_block *sb, ino_t ino,
@@ -1459,6 +1538,13 @@ int ext2_setattr(struct dentry *dentry,
if (error)
return error;
}
+ if (iattr->ia_valid & ATTR_SIZE) {
+ error = ext2_setsize(inode, iattr->ia_size);
+ if (error)
+ return error;
+ iattr->ia_valid &= ~ATTR_SIZE;
+ /* strip ATTR_SIZE for inode_setattr */
+ }
error = inode_setattr(inode, iattr);
if (!error && (iattr->ia_valid & ATTR_MODE))
error = ext2_acl_chmod(inode);
Index: linux-2.6/fs/ext2/ext2.h
===================================================================
--- linux-2.6.orig/fs/ext2/ext2.h
+++ linux-2.6/fs/ext2/ext2.h
@@ -122,7 +122,6 @@ extern int ext2_write_inode (struct inod
extern void ext2_delete_inode (struct inode *);
extern int ext2_sync_inode (struct inode *);
extern int ext2_get_block(struct inode *, sector_t, struct buffer_head *, int);
-extern void ext2_truncate (struct inode *);
extern int ext2_setattr (struct dentry *, struct iattr *);
extern void ext2_set_inode_flags(struct inode *inode);
extern void ext2_get_inode_flags(struct ext2_inode_info *);
Index: linux-2.6/fs/ext2/file.c
===================================================================
--- linux-2.6.orig/fs/ext2/file.c
+++ linux-2.6/fs/ext2/file.c
@@ -77,13 +77,13 @@ const struct file_operations ext2_xip_fi
#endif
const struct inode_operations ext2_file_inode_operations = {
- .truncate = ext2_truncate,
#ifdef CONFIG_EXT2_FS_XATTR
.setxattr = generic_setxattr,
.getxattr = generic_getxattr,
.listxattr = ext2_listxattr,
.removexattr = generic_removexattr,
#endif
+ .new_truncate = 1,
.setattr = ext2_setattr,
.permission = ext2_permission,
.fiemap = ext2_fiemap,
On Sun, Aug 16, 2009 at 08:25:38PM +1000, [email protected] wrote:
> @@ -66,9 +68,10 @@ void ext2_delete_inode (struct inode * i
> mark_inode_dirty(inode);
> ext2_write_inode(inode, inode_needs_sync(inode));
>
> + /* XXX: where is truncate_inode_pages? */
> inode->i_size = 0;
> if (inode->i_blocks)
> - ext2_truncate (inode);
> + ext2_truncate_blocks(inode, 0);
> ext2_free_inode (inode);
At the beginning of the function, just before the diff window. Because
this is ->delete_inode we truncate away all pages, down to offset 0.
> +static void ext2_truncate_blocks(struct inode *inode, loff_t offset)
> +{
> + /*
> + * XXX: it seems like a bug here that we don't allow
> + * IS_APPEND inode to have blocks-past-i_size trimmed off.
> + * review and fix this.
> + */
> + if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
> + S_ISLNK(inode->i_mode)))
> + return -EINVAL;
> + if (ext2_inode_is_fast_symlink(inode))
> + return -EINVAL;
> + if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
> + return -EPERM;
> + __ext2_truncate_blocks(inode, offset);
Yes, I think the IS_APPEND(inode) || IS_IMMUTABLE(inode) checks should move
into ext2_setsize. But let's leave that for a separate patch.
Btw, the above code gives me warnings like this:
/home/hch/work/linux-2.6/fs/ext2/inode.c: In function
'ext2_truncate_blocks':
/home/hch/work/linux-2.6/fs/ext2/inode.c:1158: warning: 'return' with a
value, in function returning void
/home/hch/work/linux-2.6/fs/ext2/inode.c:1160: warning: 'return' with a
value, in function returning void
/home/hch/work/linux-2.6/fs/ext2/inode.c:1162: warning: 'return' with a
value, in function returning void
because you try to return errors from a function delcared as void.
On Sun, Aug 16, 2009 at 04:16:26PM -0400, Christoph Hellwig wrote:
> On Sun, Aug 16, 2009 at 08:25:38PM +1000, [email protected] wrote:
> > @@ -66,9 +68,10 @@ void ext2_delete_inode (struct inode * i
> > mark_inode_dirty(inode);
> > ext2_write_inode(inode, inode_needs_sync(inode));
> >
> > + /* XXX: where is truncate_inode_pages? */
> > inode->i_size = 0;
> > if (inode->i_blocks)
> > - ext2_truncate (inode);
> > + ext2_truncate_blocks(inode, 0);
> > ext2_free_inode (inode);
>
> At the beginning of the function, just before the diff window. Because
> this is ->delete_inode we truncate away all pages, down to offset 0.
OK, weird. I thought I couldn't see it when I wrote that :) maybe my
tree was corrupted or I'm stupid.
> > +static void ext2_truncate_blocks(struct inode *inode, loff_t offset)
> > +{
> > + /*
> > + * XXX: it seems like a bug here that we don't allow
> > + * IS_APPEND inode to have blocks-past-i_size trimmed off.
> > + * review and fix this.
> > + */
> > + if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
> > + S_ISLNK(inode->i_mode)))
> > + return -EINVAL;
> > + if (ext2_inode_is_fast_symlink(inode))
> > + return -EINVAL;
> > + if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
> > + return -EPERM;
> > + __ext2_truncate_blocks(inode, offset);
>
> Yes, I think the IS_APPEND(inode) || IS_IMMUTABLE(inode) checks should move
> into ext2_setsize. But let's leave that for a separate patch.
Yeah agreed.
> Btw, the above code gives me warnings like this:
>
> /home/hch/work/linux-2.6/fs/ext2/inode.c: In function
> 'ext2_truncate_blocks':
> /home/hch/work/linux-2.6/fs/ext2/inode.c:1158: warning: 'return' with a
> value, in function returning void
> /home/hch/work/linux-2.6/fs/ext2/inode.c:1160: warning: 'return' with a
> value, in function returning void
> /home/hch/work/linux-2.6/fs/ext2/inode.c:1162: warning: 'return' with a
> value, in function returning void
>
> because you try to return errors from a function delcared as void.
Hm, sorry. I thought it was in good shape... I'll recheck that I sent
out the correct versions and resend according to feedback from you
and Hugh.
Thanks,
Nick
On 08/17/2009 09:42 AM, Nick Piggin wrote:
> On Sun, Aug 16, 2009 at 04:16:26PM -0400, Christoph Hellwig wrote:
>> On Sun, Aug 16, 2009 at 08:25:38PM +1000, [email protected] wrote:
>>> @@ -66,9 +68,10 @@ void ext2_delete_inode (struct inode * i
>>> mark_inode_dirty(inode);
>>> ext2_write_inode(inode, inode_needs_sync(inode));
>>>
>>> + /* XXX: where is truncate_inode_pages? */
>>> inode->i_size = 0;
>>> if (inode->i_blocks)
>>> - ext2_truncate (inode);
>>> + ext2_truncate_blocks(inode, 0);
>>> ext2_free_inode (inode);
>>
>> At the beginning of the function, just before the diff window. Because
>> this is ->delete_inode we truncate away all pages, down to offset 0.
>
> OK, weird. I thought I couldn't see it when I wrote that :) maybe my
> tree was corrupted or I'm stupid.
>
>
>>> +static void ext2_truncate_blocks(struct inode *inode, loff_t offset)
>>> +{
>>> + /*
>>> + * XXX: it seems like a bug here that we don't allow
>>> + * IS_APPEND inode to have blocks-past-i_size trimmed off.
>>> + * review and fix this.
>>> + */
>>> + if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
>>> + S_ISLNK(inode->i_mode)))
>>> + return -EINVAL;
>>> + if (ext2_inode_is_fast_symlink(inode))
>>> + return -EINVAL;
>>> + if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
>>> + return -EPERM;
>>> + __ext2_truncate_blocks(inode, offset);
>>
>> Yes, I think the IS_APPEND(inode) || IS_IMMUTABLE(inode) checks should move
>> into ext2_setsize. But let's leave that for a separate patch.
>
> Yeah agreed.
>
>
>> Btw, the above code gives me warnings like this:
>>
>> /home/hch/work/linux-2.6/fs/ext2/inode.c: In function
>> 'ext2_truncate_blocks':
>> /home/hch/work/linux-2.6/fs/ext2/inode.c:1158: warning: 'return' with a
>> value, in function returning void
>> /home/hch/work/linux-2.6/fs/ext2/inode.c:1160: warning: 'return' with a
>> value, in function returning void
>> /home/hch/work/linux-2.6/fs/ext2/inode.c:1162: warning: 'return' with a
>> value, in function returning void
>>
>> because you try to return errors from a function delcared as void.
>
> Hm, sorry. I thought it was in good shape... I'll recheck that I sent
> out the correct versions and resend according to feedback from you
> and Hugh.
>
> Thanks,
> Nick
>
Nick do you have a public tree with these latest patches? I would like to
try out an exofs conversion and testing.
Thanks
Boaz
On Mon, Aug 17, 2009 at 02:09:54PM +0300, Boaz Harrosh wrote:
> On 08/17/2009 09:42 AM, Nick Piggin wrote:
> > On Sun, Aug 16, 2009 at 04:16:26PM -0400, Christoph Hellwig wrote:
> >> On Sun, Aug 16, 2009 at 08:25:38PM +1000, [email protected] wrote:
> >>> @@ -66,9 +68,10 @@ void ext2_delete_inode (struct inode * i
> >>> mark_inode_dirty(inode);
> >>> ext2_write_inode(inode, inode_needs_sync(inode));
> >>>
> >>> + /* XXX: where is truncate_inode_pages? */
> >>> inode->i_size = 0;
> >>> if (inode->i_blocks)
> >>> - ext2_truncate (inode);
> >>> + ext2_truncate_blocks(inode, 0);
> >>> ext2_free_inode (inode);
> >>
> >> At the beginning of the function, just before the diff window. Because
> >> this is ->delete_inode we truncate away all pages, down to offset 0.
> >
> > OK, weird. I thought I couldn't see it when I wrote that :) maybe my
> > tree was corrupted or I'm stupid.
> >
> >
> >>> +static void ext2_truncate_blocks(struct inode *inode, loff_t offset)
> >>> +{
> >>> + /*
> >>> + * XXX: it seems like a bug here that we don't allow
> >>> + * IS_APPEND inode to have blocks-past-i_size trimmed off.
> >>> + * review and fix this.
> >>> + */
> >>> + if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
> >>> + S_ISLNK(inode->i_mode)))
> >>> + return -EINVAL;
> >>> + if (ext2_inode_is_fast_symlink(inode))
> >>> + return -EINVAL;
> >>> + if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
> >>> + return -EPERM;
> >>> + __ext2_truncate_blocks(inode, offset);
> >>
> >> Yes, I think the IS_APPEND(inode) || IS_IMMUTABLE(inode) checks should move
> >> into ext2_setsize. But let's leave that for a separate patch.
> >
> > Yeah agreed.
> >
> >
> >> Btw, the above code gives me warnings like this:
> >>
> >> /home/hch/work/linux-2.6/fs/ext2/inode.c: In function
> >> 'ext2_truncate_blocks':
> >> /home/hch/work/linux-2.6/fs/ext2/inode.c:1158: warning: 'return' with a
> >> value, in function returning void
> >> /home/hch/work/linux-2.6/fs/ext2/inode.c:1160: warning: 'return' with a
> >> value, in function returning void
> >> /home/hch/work/linux-2.6/fs/ext2/inode.c:1162: warning: 'return' with a
> >> value, in function returning void
> >>
> >> because you try to return errors from a function delcared as void.
> >
> > Hm, sorry. I thought it was in good shape... I'll recheck that I sent
> > out the correct versions and resend according to feedback from you
> > and Hugh.
> >
> > Thanks,
> > Nick
> >
>
> Nick do you have a public tree with these latest patches? I would like to
> try out an exofs conversion and testing.
Hi Boaz,
I don't have a public tree. I can send you a rollup if you ping me or
just take the patches from the list. I will send out a new set shortly
(with very minor differences -- a couple of new helper functions to
use).
That will be great if you can convert your fs. I will really appreciate
it.
Thanks,
Nick