* [patch 00/11] new truncate sequence
@ 2009-08-20 16:35 npiggin
2009-08-20 16:35 ` [patch 01/11] fs: new truncate helpers npiggin
` (16 more replies)
0 siblings, 17 replies; 35+ messages in thread
From: npiggin @ 2009-08-20 16:35 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, Christoph Hellwig
Hi,
I have made a few small changes that were suggested, and converted
a few more filesystems over. I didn't spend so much time on converting
them this week so I think it should be best to just get it out now.
Thanks,
Nick
^ permalink raw reply [flat|nested] 35+ messages in thread
* [patch 01/11] fs: new truncate helpers
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
@ 2009-08-20 16:35 ` npiggin
2009-08-26 7:38 ` Artem Bityutskiy
2009-08-20 16:35 ` [patch 02/11] fs: use " npiggin-l3A5Bk7waGM
` (15 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: npiggin @ 2009-08-20 16:35 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, Christoph Hellwig
[-- Attachment #1: fs-new-truncate-helpers.patch --]
[-- Type: text/plain, Size: 12087 bytes --]
Introduce new truncate helpers truncate_pagecache and inode_newsize_ok.
vmtruncate is also consolidated from mm/memory.c and mm/nommu.c and
into mm/truncate.c.
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nick Piggin <npiggin@suse.de>
---
Documentation/vm/locking | 2 -
fs/attr.c | 44 +++++++++++++++++++++++++++++++-
include/linux/fs.h | 1
include/linux/mm.h | 5 ++-
mm/filemap.c | 2 -
mm/memory.c | 62 ++-------------------------------------------
mm/mremap.c | 4 +-
mm/nommu.c | 40 -----------------------------
mm/truncate.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++
9 files changed, 118 insertions(+), 106 deletions(-)
Index: linux-2.6/fs/attr.c
===================================================================
--- linux-2.6.orig/fs/attr.c
+++ linux-2.6/fs/attr.c
@@ -60,9 +60,51 @@ fine:
error:
return retval;
}
-
EXPORT_SYMBOL(inode_change_ok);
+/**
+ * inode_newsize_ok - may this inode be truncated to a given size
+ * @inode: the inode to be truncated
+ * @offset: the new size to assign to the inode
+ * @Returns: 0 on success, -ve errno on failure
+ *
+ * inode_newsize_ok will check filesystem limits and ulimits to check that the
+ * new inode size is within limits. inode_newsize_ok will also send SIGXFSZ
+ * when necessary. Caller must not proceed with inode size change if failure is
+ * returned. @inode must be a file (not directory), with appropriate
+ * permissions to allow truncate (inode_newsize_ok does NOT check these
+ * conditions).
+ *
+ * inode_newsize_ok must be called with i_mutex held.
+ */
+int inode_newsize_ok(struct inode *inode, loff_t offset)
+{
+ if (inode->i_size < offset) {
+ unsigned long limit;
+
+ limit = current->signal->rlim[RLIMIT_FSIZE].rlim_cur;
+ if (limit != RLIM_INFINITY && offset > limit)
+ goto out_sig;
+ if (offset > inode->i_sb->s_maxbytes)
+ goto out_big;
+ } else {
+ /*
+ * truncation of in-use swapfiles is disallowed - it would
+ * cause subsequent swapout to scribble on the now-freed
+ * blocks.
+ */
+ if (IS_SWAPFILE(inode))
+ return -ETXTBSY;
+ }
+
+ return 0;
+out_sig:
+ send_sig(SIGXFSZ, current, 0);
+out_big:
+ return -EFBIG;
+}
+EXPORT_SYMBOL(inode_newsize_ok);
+
int inode_setattr(struct inode * inode, struct iattr * attr)
{
unsigned int ia_valid = attr->ia_valid;
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h
+++ linux-2.6/include/linux/fs.h
@@ -2369,6 +2369,7 @@ extern int buffer_migrate_page(struct ad
#endif
extern int inode_change_ok(struct inode *, struct iattr *);
+extern int inode_newsize_ok(struct inode *, loff_t offset);
extern int __must_check inode_setattr(struct inode *, struct iattr *);
extern void file_update_time(struct file *file);
Index: linux-2.6/include/linux/mm.h
===================================================================
--- linux-2.6.orig/include/linux/mm.h
+++ linux-2.6/include/linux/mm.h
@@ -805,8 +805,9 @@ static inline void unmap_shared_mapping_
unmap_mapping_range(mapping, holebegin, holelen, 0);
}
-extern int vmtruncate(struct inode * inode, loff_t offset);
-extern int vmtruncate_range(struct inode * inode, loff_t offset, loff_t end);
+extern void truncate_pagecache(struct inode *inode, loff_t old, loff_t new);
+extern int vmtruncate(struct inode *inode, loff_t offset);
+extern int vmtruncate_range(struct inode *inode, loff_t offset, loff_t end);
#ifdef CONFIG_MMU
extern int handle_mm_fault(struct mm_struct *mm, struct vm_area_struct *vma,
Index: linux-2.6/mm/memory.c
===================================================================
--- linux-2.6.orig/mm/memory.c
+++ linux-2.6/mm/memory.c
@@ -283,7 +283,8 @@ void free_pgtables(struct mmu_gather *tl
unsigned long addr = vma->vm_start;
/*
- * Hide vma from rmap and vmtruncate before freeing pgtables
+ * Hide vma from rmap and truncate_pagecache before freeing
+ * pgtables
*/
anon_vma_unlink(vma);
unlink_file_vma(vma);
@@ -2359,7 +2360,7 @@ restart:
* @mapping: the address space containing mmaps to be unmapped.
* @holebegin: byte in first page to unmap, relative to the start of
* the underlying file. This will be rounded down to a PAGE_SIZE
- * boundary. Note that this is different from vmtruncate(), which
+ * boundary. Note that this is different from truncate_pagecache(), which
* must keep the partial page. In contrast, we must get rid of
* partial pages.
* @holelen: size of prospective hole in bytes. This will be rounded
@@ -2410,63 +2411,6 @@ void unmap_mapping_range(struct address_
}
EXPORT_SYMBOL(unmap_mapping_range);
-/**
- * vmtruncate - unmap mappings "freed" by truncate() syscall
- * @inode: inode of the file used
- * @offset: file offset to start truncating
- *
- * NOTE! We have to be ready to update the memory sharing
- * between the file and the memory map for a potential last
- * incomplete page. Ugly, but necessary.
- */
-int vmtruncate(struct inode * inode, loff_t offset)
-{
- if (inode->i_size < offset) {
- unsigned long limit;
-
- limit = current->signal->rlim[RLIMIT_FSIZE].rlim_cur;
- if (limit != RLIM_INFINITY && offset > limit)
- goto out_sig;
- if (offset > inode->i_sb->s_maxbytes)
- goto out_big;
- i_size_write(inode, offset);
- } else {
- struct address_space *mapping = inode->i_mapping;
-
- /*
- * truncation of in-use swapfiles is disallowed - it would
- * cause subsequent swapout to scribble on the now-freed
- * blocks.
- */
- if (IS_SWAPFILE(inode))
- return -ETXTBSY;
- i_size_write(inode, offset);
-
- /*
- * unmap_mapping_range is called twice, first simply for
- * efficiency so that truncate_inode_pages does fewer
- * single-page unmaps. However after this first call, and
- * before truncate_inode_pages finishes, it is possible for
- * private pages to be COWed, which remain after
- * truncate_inode_pages finishes, hence the second
- * unmap_mapping_range call must be made for correctness.
- */
- unmap_mapping_range(mapping, offset + PAGE_SIZE - 1, 0, 1);
- truncate_inode_pages(mapping, offset);
- unmap_mapping_range(mapping, offset + PAGE_SIZE - 1, 0, 1);
- }
-
- if (inode->i_op->truncate)
- inode->i_op->truncate(inode);
- return 0;
-
-out_sig:
- send_sig(SIGXFSZ, current, 0);
-out_big:
- return -EFBIG;
-}
-EXPORT_SYMBOL(vmtruncate);
-
int vmtruncate_range(struct inode *inode, loff_t offset, loff_t end)
{
struct address_space *mapping = inode->i_mapping;
Index: linux-2.6/mm/nommu.c
===================================================================
--- linux-2.6.orig/mm/nommu.c
+++ linux-2.6/mm/nommu.c
@@ -86,46 +86,6 @@ struct vm_operations_struct generic_file
};
/*
- * Handle all mappings that got truncated by a "truncate()"
- * system call.
- *
- * NOTE! We have to be ready to update the memory sharing
- * between the file and the memory map for a potential last
- * incomplete page. Ugly, but necessary.
- */
-int vmtruncate(struct inode *inode, loff_t offset)
-{
- struct address_space *mapping = inode->i_mapping;
- unsigned long limit;
-
- if (inode->i_size < offset)
- goto do_expand;
- i_size_write(inode, offset);
-
- truncate_inode_pages(mapping, offset);
- goto out_truncate;
-
-do_expand:
- limit = current->signal->rlim[RLIMIT_FSIZE].rlim_cur;
- if (limit != RLIM_INFINITY && offset > limit)
- goto out_sig;
- if (offset > inode->i_sb->s_maxbytes)
- goto out;
- i_size_write(inode, offset);
-
-out_truncate:
- if (inode->i_op->truncate)
- inode->i_op->truncate(inode);
- return 0;
-out_sig:
- send_sig(SIGXFSZ, current, 0);
-out:
- return -EFBIG;
-}
-
-EXPORT_SYMBOL(vmtruncate);
-
-/*
* Return the total memory allocated for this pointer, not
* just what the caller asked for.
*
Index: linux-2.6/Documentation/vm/locking
===================================================================
--- linux-2.6.orig/Documentation/vm/locking
+++ linux-2.6/Documentation/vm/locking
@@ -80,7 +80,7 @@ Note: PTL can also be used to guarantee
mm start up ... this is a loose form of stability on mm_users. For
example, it is used in copy_mm to protect against a racing tlb_gather_mmu
single address space optimization, so that the zap_page_range (from
-vmtruncate) does not lose sending ipi's to cloned threads that might
+truncate) does not lose sending ipi's to cloned threads that might
be spawned underneath it and go to user mode to drag in pte's into tlbs.
swap_lock
Index: linux-2.6/mm/filemap.c
===================================================================
--- linux-2.6.orig/mm/filemap.c
+++ linux-2.6/mm/filemap.c
@@ -59,7 +59,7 @@
/*
* Lock ordering:
*
- * ->i_mmap_lock (vmtruncate)
+ * ->i_mmap_lock (truncate_pagecache)
* ->private_lock (__free_pte->__set_page_dirty_buffers)
* ->swap_lock (exclusive_swap_page, others)
* ->mapping->tree_lock
Index: linux-2.6/mm/mremap.c
===================================================================
--- linux-2.6.orig/mm/mremap.c
+++ linux-2.6/mm/mremap.c
@@ -85,8 +85,8 @@ static void move_ptes(struct vm_area_str
if (vma->vm_file) {
/*
* Subtle point from Rajesh Venkatasubramanian: before
- * moving file-based ptes, we must lock vmtruncate out,
- * since it might clean the dst vma before the src vma,
+ * moving file-based ptes, we must lock truncate_pagecache
+ * out, since it might clean the dst vma before the src vma,
* and we propagate stale pages into the dst afterward.
*/
mapping = vma->vm_file->f_mapping;
Index: linux-2.6/mm/truncate.c
===================================================================
--- linux-2.6.orig/mm/truncate.c
+++ linux-2.6/mm/truncate.c
@@ -465,3 +465,67 @@ int invalidate_inode_pages2(struct addre
return invalidate_inode_pages2_range(mapping, 0, -1);
}
EXPORT_SYMBOL_GPL(invalidate_inode_pages2);
+
+/**
+ * truncate_pagecache - unmap and remove pagecache that has been truncated
+ * @inode: inode
+ * @old: old file offset
+ * @new: new file offset
+ *
+ * inode's new i_size must already be written before truncate_pagecache
+ * is called.
+ *
+ * This function should typically be called before the filesystem
+ * releases resources associated with the freed range (eg. deallocates
+ * blocks). This way, pagecache will always stay logically coherent
+ * with on-disk format, and the filesystem would not have to deal with
+ * situations such as writepage being called for a page that has already
+ * had its underlying blocks deallocated.
+ */
+void truncate_pagecache(struct inode *inode, loff_t old, loff_t new)
+{
+ if (new < old) {
+ struct address_space *mapping = inode->i_mapping;
+
+ /*
+ * unmap_mapping_range is called twice, first simply for
+ * efficiency so that truncate_inode_pages does fewer
+ * single-page unmaps. However after this first call, and
+ * before truncate_inode_pages finishes, it is possible for
+ * private pages to be COWed, which remain after
+ * truncate_inode_pages finishes, hence the second
+ * unmap_mapping_range call must be made for correctness.
+ */
+ unmap_mapping_range(mapping, new + PAGE_SIZE - 1, 0, 1);
+ truncate_inode_pages(mapping, new);
+ unmap_mapping_range(mapping, new + PAGE_SIZE - 1, 0, 1);
+ }
+}
+EXPORT_SYMBOL(truncate_pagecache);
+
+/**
+ * vmtruncate - unmap mappings "freed" by truncate() syscall
+ * @inode: inode of the file used
+ * @offset: file offset to start truncating
+ *
+ * NOTE! We have to be ready to update the memory sharing
+ * between the file and the memory map for a potential last
+ * incomplete page. Ugly, but necessary.
+ */
+int vmtruncate(struct inode *inode, loff_t offset)
+{
+ loff_t oldsize;
+ int error;
+
+ error = inode_newsize_ok(inode, offset);
+ if (error)
+ return error;
+ oldsize = inode->i_size;
+ i_size_write(inode, offset);
+ truncate_pagecache(inode, oldsize, offset);
+ if (inode->i_op->truncate)
+ inode->i_op->truncate(inode);
+
+ return error;
+}
+EXPORT_SYMBOL(vmtruncate);
^ permalink raw reply [flat|nested] 35+ messages in thread
* [patch 02/11] fs: use new truncate helpers
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
2009-08-20 16:35 ` [patch 01/11] fs: new truncate helpers npiggin
@ 2009-08-20 16:35 ` npiggin-l3A5Bk7waGM
2009-08-20 16:35 ` [patch 03/11] fs: introduce new truncate sequence npiggin
` (14 subsequent siblings)
16 siblings, 0 replies; 35+ messages in thread
From: npiggin-l3A5Bk7waGM @ 2009-08-20 16:35 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-fsdevel-u79uwXL29TY76Z2rM5mHXA, Christoph Hellwig,
Miklos Szeredi, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
linux-cifs-client-w/Ol4Ecudpl8XjKLYN78aQ,
sfrench-eUNUBHrolfbYtjvyW6yDsg
[-- Attachment #1: fs-new-truncate-convert.patch --]
[-- Type: text/plain, Size: 9336 bytes --]
Update some fs code to make use of new helper functions introduced
in the previous patch. Should be no significant change in behaviour
(except CIFS now calls send_sig under i_lock, via inode_newsize_ok).
Reviewed-by: Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>
Acked-by: Miklos Szeredi <miklos-sUDqSbJrdHQHWmgEVkV9KA@public.gmane.org>
Cc: linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org
Cc: linux-cifs-client-w/Ol4Ecudpl8XjKLYN78aQ@public.gmane.org
Cc: sfrench-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org
Signed-off-by: Nick Piggin <npiggin-l3A5Bk7waGM@public.gmane.org>
---
fs/buffer.c | 10 +--------
fs/cifs/inode.c | 51 ++++++++-----------------------------------------
fs/fuse/dir.c | 14 +++----------
fs/fuse/fuse_i.h | 2 -
fs/fuse/inode.c | 11 ----------
fs/nfs/inode.c | 52 +++++++++++---------------------------------------
fs/ramfs/file-nommu.c | 18 ++++-------------
7 files changed, 33 insertions(+), 125 deletions(-)
Index: linux-2.6/fs/buffer.c
===================================================================
--- linux-2.6.orig/fs/buffer.c
+++ linux-2.6/fs/buffer.c
@@ -2225,16 +2225,10 @@ int generic_cont_expand_simple(struct in
struct address_space *mapping = inode->i_mapping;
struct page *page;
void *fsdata;
- unsigned long limit;
int err;
- err = -EFBIG;
- limit = current->signal->rlim[RLIMIT_FSIZE].rlim_cur;
- if (limit != RLIM_INFINITY && size > (loff_t)limit) {
- send_sig(SIGXFSZ, current, 0);
- goto out;
- }
- if (size > inode->i_sb->s_maxbytes)
+ err = inode_newsize_ok(inode, size);
+ if (err)
goto out;
err = pagecache_write_begin(NULL, mapping, size, 0,
Index: linux-2.6/fs/cifs/inode.c
===================================================================
--- linux-2.6.orig/fs/cifs/inode.c
+++ linux-2.6/fs/cifs/inode.c
@@ -1557,57 +1557,24 @@ static int cifs_truncate_page(struct add
static int cifs_vmtruncate(struct inode *inode, loff_t offset)
{
- struct address_space *mapping = inode->i_mapping;
- unsigned long limit;
+ loff_t oldsize;
+ int err;
spin_lock(&inode->i_lock);
- if (inode->i_size < offset)
- goto do_expand;
- /*
- * truncation of in-use swapfiles is disallowed - it would cause
- * subsequent swapout to scribble on the now-freed blocks.
- */
- if (IS_SWAPFILE(inode)) {
+ err = inode_newsize_ok(inode, offset);
+ if (err) {
spin_unlock(&inode->i_lock);
- goto out_busy;
+ goto out;
}
- i_size_write(inode, offset);
- spin_unlock(&inode->i_lock);
- /*
- * unmap_mapping_range is called twice, first simply for efficiency
- * so that truncate_inode_pages does fewer single-page unmaps. However
- * after this first call, and before truncate_inode_pages finishes,
- * it is possible for private pages to be COWed, which remain after
- * truncate_inode_pages finishes, hence the second unmap_mapping_range
- * call must be made for correctness.
- */
- unmap_mapping_range(mapping, offset + PAGE_SIZE - 1, 0, 1);
- truncate_inode_pages(mapping, offset);
- unmap_mapping_range(mapping, offset + PAGE_SIZE - 1, 0, 1);
- goto out_truncate;
-do_expand:
- limit = current->signal->rlim[RLIMIT_FSIZE].rlim_cur;
- if (limit != RLIM_INFINITY && offset > limit) {
- spin_unlock(&inode->i_lock);
- goto out_sig;
- }
- if (offset > inode->i_sb->s_maxbytes) {
- spin_unlock(&inode->i_lock);
- goto out_big;
- }
+ oldsize = inode->i_size;
i_size_write(inode, offset);
spin_unlock(&inode->i_lock);
-out_truncate:
+ truncate_pagecache(inode, oldsize, offset);
if (inode->i_op->truncate)
inode->i_op->truncate(inode);
- return 0;
-out_sig:
- send_sig(SIGXFSZ, current, 0);
-out_big:
- return -EFBIG;
-out_busy:
- return -ETXTBSY;
+out:
+ return err;
}
static int
Index: linux-2.6/fs/fuse/dir.c
===================================================================
--- linux-2.6.orig/fs/fuse/dir.c
+++ linux-2.6/fs/fuse/dir.c
@@ -1276,14 +1276,9 @@ static int fuse_do_setattr(struct dentry
return 0;
if (attr->ia_valid & ATTR_SIZE) {
- unsigned long limit;
- if (IS_SWAPFILE(inode))
- return -ETXTBSY;
- limit = current->signal->rlim[RLIMIT_FSIZE].rlim_cur;
- if (limit != RLIM_INFINITY && attr->ia_size > (loff_t) limit) {
- send_sig(SIGXFSZ, current, 0);
- return -EFBIG;
- }
+ err = inode_newsize_ok(inode, attr->ia_size);
+ if (err)
+ return err;
is_truncate = true;
}
@@ -1350,8 +1345,7 @@ static int fuse_do_setattr(struct dentry
* FUSE_NOWRITE, otherwise fuse_launder_page() would deadlock.
*/
if (S_ISREG(inode->i_mode) && oldsize != outarg.attr.size) {
- if (outarg.attr.size < oldsize)
- fuse_truncate(inode->i_mapping, outarg.attr.size);
+ truncate_pagecache(inode, oldsize, outarg.attr.size);
invalidate_inode_pages2(inode->i_mapping);
}
Index: linux-2.6/fs/nfs/inode.c
===================================================================
--- linux-2.6.orig/fs/nfs/inode.c
+++ linux-2.6/fs/nfs/inode.c
@@ -426,49 +426,21 @@ nfs_setattr(struct dentry *dentry, struc
*/
static int nfs_vmtruncate(struct inode * inode, loff_t offset)
{
- if (i_size_read(inode) < offset) {
- unsigned long limit;
+ loff_t oldsize;
+ int err;
- limit = current->signal->rlim[RLIMIT_FSIZE].rlim_cur;
- if (limit != RLIM_INFINITY && offset > limit)
- goto out_sig;
- if (offset > inode->i_sb->s_maxbytes)
- goto out_big;
- spin_lock(&inode->i_lock);
- i_size_write(inode, offset);
- spin_unlock(&inode->i_lock);
- } else {
- struct address_space *mapping = inode->i_mapping;
+ err = inode_newsize_ok(inode, offset);
+ if (err)
+ goto out;
- /*
- * truncation of in-use swapfiles is disallowed - it would
- * cause subsequent swapout to scribble on the now-freed
- * blocks.
- */
- if (IS_SWAPFILE(inode))
- return -ETXTBSY;
- spin_lock(&inode->i_lock);
- i_size_write(inode, offset);
- spin_unlock(&inode->i_lock);
+ spin_lock(&inode->i_lock);
+ oldsize = inode->i_size;
+ i_size_write(inode, offset);
+ spin_unlock(&inode->i_lock);
- /*
- * unmap_mapping_range is called twice, first simply for
- * efficiency so that truncate_inode_pages does fewer
- * single-page unmaps. However after this first call, and
- * before truncate_inode_pages finishes, it is possible for
- * private pages to be COWed, which remain after
- * truncate_inode_pages finishes, hence the second
- * unmap_mapping_range call must be made for correctness.
- */
- unmap_mapping_range(mapping, offset + PAGE_SIZE - 1, 0, 1);
- truncate_inode_pages(mapping, offset);
- unmap_mapping_range(mapping, offset + PAGE_SIZE - 1, 0, 1);
- }
- return 0;
-out_sig:
- send_sig(SIGXFSZ, current, 0);
-out_big:
- return -EFBIG;
+ truncate_pagecache(inode, oldsize, offset);
+out:
+ return err;
}
/**
Index: linux-2.6/fs/ramfs/file-nommu.c
===================================================================
--- linux-2.6.orig/fs/ramfs/file-nommu.c
+++ linux-2.6/fs/ramfs/file-nommu.c
@@ -69,14 +69,11 @@ int ramfs_nommu_expand_for_mapping(struc
/* make various checks */
order = get_order(newsize);
if (unlikely(order >= MAX_ORDER))
- goto too_big;
+ return -EFBIG;
- limit = current->signal->rlim[RLIMIT_FSIZE].rlim_cur;
- if (limit != RLIM_INFINITY && newsize > limit)
- goto fsize_exceeded;
-
- if (newsize > inode->i_sb->s_maxbytes)
- goto too_big;
+ ret = inode_newsize_ok(inode, newsize);
+ if (ret)
+ return ret;
i_size_write(inode, newsize);
@@ -118,12 +115,7 @@ int ramfs_nommu_expand_for_mapping(struc
return 0;
- fsize_exceeded:
- send_sig(SIGXFSZ, current, 0);
- too_big:
- return -EFBIG;
-
- add_error:
+add_error:
while (loop < npages)
__free_page(pages + loop++);
return ret;
Index: linux-2.6/fs/fuse/fuse_i.h
===================================================================
--- linux-2.6.orig/fs/fuse/fuse_i.h
+++ linux-2.6/fs/fuse/fuse_i.h
@@ -602,8 +602,6 @@ void fuse_change_attributes(struct inode
void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
u64 attr_valid);
-void fuse_truncate(struct address_space *mapping, loff_t offset);
-
/**
* Initialize the client device
*/
Index: linux-2.6/fs/fuse/inode.c
===================================================================
--- linux-2.6.orig/fs/fuse/inode.c
+++ linux-2.6/fs/fuse/inode.c
@@ -115,14 +115,6 @@ static int fuse_remount_fs(struct super_
return 0;
}
-void fuse_truncate(struct address_space *mapping, loff_t offset)
-{
- /* See vmtruncate() */
- unmap_mapping_range(mapping, offset + PAGE_SIZE - 1, 0, 1);
- truncate_inode_pages(mapping, offset);
- unmap_mapping_range(mapping, offset + PAGE_SIZE - 1, 0, 1);
-}
-
void fuse_change_attributes_common(struct inode *inode, struct fuse_attr *attr,
u64 attr_valid)
{
@@ -180,8 +172,7 @@ void fuse_change_attributes(struct inode
spin_unlock(&fc->lock);
if (S_ISREG(inode->i_mode) && oldsize != attr->size) {
- if (attr->size < oldsize)
- fuse_truncate(inode->i_mapping, attr->size);
+ truncate_pagecache(inode, oldsize, attr->size);
invalidate_inode_pages2(inode->i_mapping);
}
}
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 35+ messages in thread
* [patch 03/11] fs: introduce new truncate sequence
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
2009-08-20 16:35 ` [patch 01/11] fs: new truncate helpers npiggin
2009-08-20 16:35 ` [patch 02/11] fs: use " npiggin-l3A5Bk7waGM
@ 2009-08-20 16:35 ` npiggin
2009-08-26 7:40 ` Artem Bityutskiy
2009-08-20 16:35 ` [patch 04/11] fs: convert simple fs to new truncate npiggin
` (13 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: npiggin @ 2009-08-20 16:35 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, Christoph Hellwig
[-- Attachment #1: fs-new-truncate.patch --]
[-- Type: text/plain, Size: 14246 bytes --]
Introduce a new truncate calling sequence into fs/mm subsystems. Rather than
setattr > vmtruncate > truncate, have filesystems call their truncate sequence
from ->setattr if filesystem specific operations are required. vmtruncate is
deprecated, and truncate_pagecache and inode_newsize_ok helpers introduced
previously should be used.
simple_setattr is introduced for simple in-ram filesystems to implement
the new truncate sequence. Eventually all filesystems should be converted
to implement a setattr, and the default code in notify_change should go
away.
simple_setsize is also introduced to perform just the ATTR_SIZE portion
of simple_setattr (ie. changing i_size and trimming pagecache).
A new attribute is introduced into inode_operations structure; .new_truncate
is a temporary hack to distinguish filesystems that implement the new
truncate system.
To implement the new truncate sequence:
- set .new_truncate = 1
- filesystem specific manipulations (eg freeing blocks) must be done in
the setattr method rather than ->truncate.
- vmtruncate can not be used by core code to trim blocks past i_size in
the event of write failure after allocation, so this must be performed
in the fs code.
- make use of the better opportunity to catch errors with the above 2 changes.
- inode_setattr should not be used. generic_setattr is a new function
to be used to copy simple attributes into the generic inode.
Big problem with the previous calling sequence: the filesystem is not called
until i_size has already changed. This means it is not allowed to fail the
call, and also it does not know what the previous i_size was. Also, generic
code calling vmtruncate to truncate allocated blocks in case of error had
no good way to return a meaningful error (or, for example, atomically handle
block deallocation).
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nick Piggin <npiggin@suse.de>
---
Documentation/filesystems/vfs.txt | 7 ++-
fs/attr.c | 51 ++++++++++++++++++-----
fs/buffer.c | 12 ++++-
fs/direct-io.c | 7 +--
fs/libfs.c | 83 ++++++++++++++++++++++++++++++++++++++
include/linux/fs.h | 4 +
mm/truncate.c | 10 ++--
7 files changed, 152 insertions(+), 22 deletions(-)
Index: linux-2.6/fs/libfs.c
===================================================================
--- linux-2.6.orig/fs/libfs.c
+++ linux-2.6/fs/libfs.c
@@ -7,6 +7,7 @@
#include <linux/pagemap.h>
#include <linux/mount.h>
#include <linux/vfs.h>
+#include <linux/quotaops.h>
#include <linux/mutex.h>
#include <linux/exportfs.h>
#include <linux/writeback.h>
@@ -329,6 +330,88 @@ int simple_rename(struct inode *old_dir,
return 0;
}
+/**
+ * simple_setsize - handle core mm and vfs requirements for file size change
+ * @inode: inode
+ * @newsize: new file size
+ *
+ * Returns 0 on success, -error on failure.
+ *
+ * simple_setsize must be called with inode_mutex held.
+ *
+ * simple_setsize will check that the requested new size is OK (see
+ * inode_newsize_ok), and then will perform the necessary i_size update
+ * and pagecache truncation (if necessary). It will be typically be called
+ * from the filesystem's setattr function when ATTR_SIZE is passed in.
+ *
+ * The inode itself must have correct permissions and attributes to allow
+ * i_size to be changed, this function then just checks that the new size
+ * requested is valid.
+ *
+ * In the case of simple in-memory filesystems with inodes stored solely
+ * in the inode cache, and file data in the pagecache, nothing more needs
+ * to be done to satisfy a truncate request. Filesystems with on-disk
+ * blocks for example will need to free them in the case of truncate, in
+ * that case it may be easier not to use simple_setsize (but each of its
+ * components will likely be required at some point to update pagecache
+ * and inode etc).
+ */
+int simple_setsize(struct inode *inode, loff_t newsize)
+{
+ loff_t oldsize;
+ int error;
+
+ error = inode_newsize_ok(inode, newsize);
+ if (error)
+ return error;
+
+ oldsize = inode->i_size;
+ i_size_write(inode, newsize);
+ truncate_pagecache(inode, oldsize, newsize);
+
+ return error;
+}
+EXPORT_SYMBOL(simple_setsize);
+
+/**
+ * simple_setattr - setattr for simple in-memory filesystem
+ * @dentry: dentry
+ * @iattr: iattr structure
+ *
+ * Returns 0 on success, -error on failure.
+ *
+ * simple_setattr implements setattr for an in-memory filesystem which
+ * does not store its own file data or metadata (eg. uses the page cache
+ * and inode cache as its data store).
+ */
+int simple_setattr(struct dentry *dentry, struct iattr *iattr)
+{
+ struct inode *inode = dentry->d_inode;
+ int error;
+
+ error = inode_change_ok(inode, iattr);
+ if (error)
+ return error;
+
+ if ((iattr->ia_valid & ATTR_UID && iattr->ia_uid != inode->i_uid) ||
+ (iattr->ia_valid & ATTR_GID && iattr->ia_gid != inode->i_gid)) {
+ error = vfs_dq_transfer(inode, iattr) ? -EDQUOT : 0;
+ if (error)
+ return error;
+ }
+
+ if (iattr->ia_valid & ATTR_SIZE) {
+ error = simple_setsize(inode, iattr->ia_size);
+ if (error)
+ return error;
+ }
+
+ generic_setattr(inode, iattr);
+
+ return error;
+}
+EXPORT_SYMBOL(simple_setattr);
+
int simple_readpage(struct file *file, struct page *page)
{
clear_highpage(page);
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h
+++ linux-2.6/include/linux/fs.h
@@ -1527,6 +1527,7 @@ struct inode_operations {
void * (*follow_link) (struct dentry *, struct nameidata *);
void (*put_link) (struct dentry *, struct nameidata *, void *);
void (*truncate) (struct inode *);
+ int new_truncate; /* nasty hack to transition to new truncate code */
int (*permission) (struct inode *, int);
int (*setattr) (struct dentry *, struct iattr *);
int (*getattr) (struct vfsmount *mnt, struct dentry *, struct kstat *);
@@ -2328,12 +2329,14 @@ extern int dcache_dir_open(struct inode
extern int dcache_dir_close(struct inode *, struct file *);
extern loff_t dcache_dir_lseek(struct file *, loff_t, int);
extern int dcache_readdir(struct file *, void *, filldir_t);
+extern int simple_setattr(struct dentry *dentry, struct iattr *attr);
extern int simple_getattr(struct vfsmount *, struct dentry *, struct kstat *);
extern int simple_statfs(struct dentry *, struct kstatfs *);
extern int simple_link(struct dentry *, struct inode *, struct dentry *);
extern int simple_unlink(struct inode *, struct dentry *);
extern int simple_rmdir(struct inode *, struct dentry *);
extern int simple_rename(struct inode *, struct dentry *, struct inode *, struct dentry *);
+extern int simple_setsize(struct inode *inode, loff_t newsize);
extern int simple_sync_file(struct file *, struct dentry *, int);
extern int simple_empty(struct dentry *);
extern int simple_readpage(struct file *file, struct page *page);
@@ -2371,6 +2374,7 @@ extern int buffer_migrate_page(struct ad
extern int inode_change_ok(struct inode *, struct iattr *);
extern int inode_newsize_ok(struct inode *, loff_t offset);
extern int __must_check inode_setattr(struct inode *, struct iattr *);
+extern void generic_setattr(struct inode *inode, struct iattr *attr);
extern void file_update_time(struct file *file);
Index: linux-2.6/fs/buffer.c
===================================================================
--- linux-2.6.orig/fs/buffer.c
+++ linux-2.6/fs/buffer.c
@@ -1992,9 +1992,14 @@ int block_write_begin(struct file *file,
* prepare_write() may have instantiated a few blocks
* outside i_size. Trim these off again. Don't need
* i_size_read because we hold i_mutex.
+ *
+ * Filesystems which set i_op->new_truncate must
+ * handle this themselves. Eventually this will go
+ * away because everyone will be converted.
*/
if (pos + len > inode->i_size)
- vmtruncate(inode, inode->i_size);
+ if (!inode->i_op->new_truncate)
+ vmtruncate(inode, inode->i_size);
}
}
@@ -2371,7 +2376,7 @@ int block_commit_write(struct page *page
*
* We are not allowed to take the i_mutex here so we have to play games to
* protect against truncate races as the page could now be beyond EOF. Because
- * vmtruncate() writes the inode size before removing pages, once we have the
+ * truncate writes the inode size before removing pages, once we have the
* page lock we can determine safely if the page is beyond EOF. If it is not
* beyond EOF, then the page is guaranteed safe against truncation until we
* unlock the page.
@@ -2595,7 +2600,8 @@ out_release:
*pagep = NULL;
if (pos + len > inode->i_size)
- vmtruncate(inode, inode->i_size);
+ if (!inode->i_op->new_truncate)
+ vmtruncate(inode, inode->i_size);
return ret;
}
Index: linux-2.6/fs/direct-io.c
===================================================================
--- linux-2.6.orig/fs/direct-io.c
+++ linux-2.6/fs/direct-io.c
@@ -1210,14 +1210,15 @@ __blockdev_direct_IO(int rw, struct kioc
/*
* In case of error extending write may have instantiated a few
* blocks outside i_size. Trim these off again for DIO_LOCKING.
- * NOTE: DIO_NO_LOCK/DIO_OWN_LOCK callers have to handle this by
- * it's own meaner.
+ * NOTE: DIO_NO_LOCK/DIO_OWN_LOCK callers have to handle this in
+ * their own manner.
*/
if (unlikely(retval < 0 && (rw & WRITE))) {
loff_t isize = i_size_read(inode);
if (end > isize && dio_lock_type == DIO_LOCKING)
- vmtruncate(inode, isize);
+ if (!inode->i_op->new_truncate)
+ vmtruncate(inode, isize);
}
if (rw == READ && dio_lock_type == DIO_LOCKING)
Index: linux-2.6/fs/attr.c
===================================================================
--- linux-2.6.orig/fs/attr.c
+++ linux-2.6/fs/attr.c
@@ -68,14 +68,14 @@ EXPORT_SYMBOL(inode_change_ok);
* @offset: the new size to assign to the inode
* @Returns: 0 on success, -ve errno on failure
*
+ * inode_newsize_ok must be called with i_mutex held.
+ *
* inode_newsize_ok will check filesystem limits and ulimits to check that the
* new inode size is within limits. inode_newsize_ok will also send SIGXFSZ
* when necessary. Caller must not proceed with inode size change if failure is
* returned. @inode must be a file (not directory), with appropriate
* permissions to allow truncate (inode_newsize_ok does NOT check these
* conditions).
- *
- * inode_newsize_ok must be called with i_mutex held.
*/
int inode_newsize_ok(struct inode *inode, loff_t offset)
{
@@ -105,17 +105,25 @@ out_big:
}
EXPORT_SYMBOL(inode_newsize_ok);
-int inode_setattr(struct inode * inode, struct iattr * attr)
+/**
+ * generic_setattr - copy simple metadata updates into the generic inode
+ * @inode: the inode to be updated
+ * @attr: the new attributes
+ *
+ * generic_setattr must be called with i_mutex held.
+ *
+ * generic_setattr updates the inode's metadata with that specified
+ * in attr. Noticably missing is inode size update, which is more complex
+ * as it requires pagecache updates. See simple_setsize.
+ *
+ * The inode is not marked as dirty after this operation. The rationale is
+ * that for "simple" filesystems, the struct inode is the inode storage.
+ * The caller is free to mark the inode dirty afterwards if needed.
+ */
+void generic_setattr(struct inode *inode, struct iattr *attr)
{
unsigned int ia_valid = attr->ia_valid;
- if (ia_valid & ATTR_SIZE &&
- attr->ia_size != i_size_read(inode)) {
- int error = vmtruncate(inode, attr->ia_size);
- if (error)
- return error;
- }
-
if (ia_valid & ATTR_UID)
inode->i_uid = attr->ia_uid;
if (ia_valid & ATTR_GID)
@@ -136,6 +144,29 @@ int inode_setattr(struct inode * inode,
mode &= ~S_ISGID;
inode->i_mode = mode;
}
+}
+EXPORT_SYMBOL(generic_setattr);
+
+/*
+ * note this function is deprecated, the new truncate sequence should be
+ * used instead -- see eg. simple_setsize, generic_setattr.
+ */
+int inode_setattr(struct inode *inode, struct iattr * attr)
+{
+ unsigned int ia_valid = attr->ia_valid;
+
+ if (ia_valid & ATTR_SIZE &&
+ attr->ia_size != i_size_read(inode)) {
+ int error;
+
+ BUG_ON(inode->i_op->new_truncate);
+ error = vmtruncate(inode, attr->ia_size);
+ if (error)
+ return error;
+ }
+
+ generic_setattr(inode, attr);
+
mark_inode_dirty(inode);
return 0;
Index: linux-2.6/Documentation/filesystems/vfs.txt
===================================================================
--- linux-2.6.orig/Documentation/filesystems/vfs.txt
+++ linux-2.6/Documentation/filesystems/vfs.txt
@@ -401,11 +401,16 @@ otherwise noted.
started might not be in the page cache at the end of the
walk).
- truncate: called by the VFS to change the size of a file. The
+ truncate: Deprecated. This will not be called if ->setsize is defined.
+ Called by the VFS to change the size of a file. The
i_size field of the inode is set to the desired size by the
VFS before this method is called. This method is called by
the truncate(2) system call and related functionality.
+ Note: ->truncate and vmtruncate are deprecated. Do not add new
+ instances/calls of these. Filesystems shoud be converted to do their
+ truncate sequence via ->setattr().
+
permission: called by the VFS to check for access rights on a POSIX-like
filesystem.
Index: linux-2.6/mm/truncate.c
===================================================================
--- linux-2.6.orig/mm/truncate.c
+++ linux-2.6/mm/truncate.c
@@ -511,18 +511,18 @@ EXPORT_SYMBOL(truncate_pagecache);
* NOTE! We have to be ready to update the memory sharing
* between the file and the memory map for a potential last
* incomplete page. Ugly, but necessary.
+ *
+ * This function is deprecated and simple_setsize or truncate_pagecache
+ * should be used instead.
*/
int vmtruncate(struct inode *inode, loff_t offset)
{
- loff_t oldsize;
int error;
- error = inode_newsize_ok(inode, offset);
+ error = simple_setsize(inode, offset);
if (error)
return error;
- oldsize = inode->i_size;
- i_size_write(inode, offset);
- truncate_pagecache(inode, oldsize, offset);
+
if (inode->i_op->truncate)
inode->i_op->truncate(inode);
^ permalink raw reply [flat|nested] 35+ messages in thread
* [patch 04/11] fs: convert simple fs to new truncate
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
` (2 preceding siblings ...)
2009-08-20 16:35 ` [patch 03/11] fs: introduce new truncate sequence npiggin
@ 2009-08-20 16:35 ` npiggin
2009-08-20 16:35 ` [patch 05/11] tmpfs: convert to use the new truncate convention npiggin
` (12 subsequent siblings)
16 siblings, 0 replies; 35+ messages in thread
From: npiggin @ 2009-08-20 16:35 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, Christoph Hellwig
[-- Attachment #1: fs-simple-new-truncate.patch --]
[-- Type: text/plain, Size: 3289 bytes --]
Convert simple filesystems: ramfs, configfs, sysfs to new truncate sequence.
Signed-off-by: Nick Piggin <npiggin@suse.de>
---
fs/configfs/inode.c | 11 ++++++++---
fs/ramfs/file-mmu.c | 2 ++
fs/ramfs/file-nommu.c | 8 +++++---
fs/sysfs/inode.c | 7 ++-----
4 files changed, 17 insertions(+), 11 deletions(-)
Index: linux-2.6/fs/ramfs/file-mmu.c
===================================================================
--- linux-2.6.orig/fs/ramfs/file-mmu.c
+++ linux-2.6/fs/ramfs/file-mmu.c
@@ -50,5 +50,7 @@ const struct file_operations ramfs_file_
};
const struct inode_operations ramfs_file_inode_operations = {
+ .new_truncate = 1,
+ .setattr = simple_setattr,
.getattr = simple_getattr,
};
Index: linux-2.6/fs/ramfs/file-nommu.c
===================================================================
--- linux-2.6.orig/fs/ramfs/file-nommu.c
+++ linux-2.6/fs/ramfs/file-nommu.c
@@ -48,6 +48,7 @@ const struct file_operations ramfs_file_
};
const struct inode_operations ramfs_file_inode_operations = {
+ .new_truncate = 1,
.setattr = ramfs_nommu_setattr,
.getattr = simple_getattr,
};
@@ -169,7 +170,7 @@ static int ramfs_nommu_resize(struct ino
return ret;
}
- ret = vmtruncate(inode, newsize);
+ ret = simple_setsize(inode, newsize);
return ret;
}
@@ -192,7 +193,8 @@ static int ramfs_nommu_setattr(struct de
/* pick out size-changing events */
if (ia->ia_valid & ATTR_SIZE) {
- loff_t size = i_size_read(inode);
+ loff_t size = inode->i_size;
+
if (ia->ia_size != size) {
ret = ramfs_nommu_resize(inode, ia->ia_size, size);
if (ret < 0 || ia->ia_valid == ATTR_SIZE)
@@ -205,7 +207,7 @@ static int ramfs_nommu_setattr(struct de
}
}
- ret = inode_setattr(inode, ia);
+ generic_setattr(inode, ia);
out:
ia->ia_valid = old_ia_valid;
return ret;
Index: linux-2.6/fs/configfs/inode.c
===================================================================
--- linux-2.6.orig/fs/configfs/inode.c
+++ linux-2.6/fs/configfs/inode.c
@@ -56,6 +56,7 @@ static struct backing_dev_info configfs_
};
static const struct inode_operations configfs_inode_operations ={
+ .new_truncate = 1,
.setattr = configfs_setattr,
};
@@ -76,9 +77,13 @@ int configfs_setattr(struct dentry * den
if (error)
return error;
- error = inode_setattr(inode, iattr);
- if (error)
- return error;
+ if (iattr->ia_valid & ATTR_SIZE) {
+ error = simple_setsize(inode, iattr->ia_size);
+ if (error)
+ return error;
+ }
+
+ generic_setattr(inode, iattr);
if (!sd_iattr) {
/* setting attributes for the first time, allocate now */
Index: linux-2.6/fs/sysfs/inode.c
===================================================================
--- linux-2.6.orig/fs/sysfs/inode.c
+++ linux-2.6/fs/sysfs/inode.c
@@ -34,6 +34,7 @@ static struct backing_dev_info sysfs_bac
};
static const struct inode_operations sysfs_inode_operations ={
+ .new_truncate = 1,
.setattr = sysfs_setattr,
};
@@ -59,11 +60,7 @@ int sysfs_setattr(struct dentry * dentry
if (error)
return error;
- iattr->ia_valid &= ~ATTR_SIZE; /* ignore size changes */
-
- error = inode_setattr(inode, iattr);
- if (error)
- return error;
+ generic_setattr(inode, iattr);
if (!sd_iattr) {
/* setting attributes for the first time, allocate now */
^ permalink raw reply [flat|nested] 35+ messages in thread
* [patch 05/11] tmpfs: convert to use the new truncate convention
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
` (3 preceding siblings ...)
2009-08-20 16:35 ` [patch 04/11] fs: convert simple fs to new truncate npiggin
@ 2009-08-20 16:35 ` npiggin
2009-08-20 16:35 ` [patch 06/11] ext2: " npiggin
` (11 subsequent siblings)
16 siblings, 0 replies; 35+ messages in thread
From: npiggin @ 2009-08-20 16:35 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, Christoph Hellwig, Hugh Dickins
[-- Attachment #1: tmpfs-new-truncate.patch --]
[-- Type: text/plain, Size: 4258 bytes --]
Cc: Christoph Hellwig <hch@lst.de>
Acked-by: Hugh Dickins <hugh.dickins@tiscali.co.uk>
Signed-off-by: Nick Piggin <npiggin@suse.de>
---
mm/shmem.c | 44 +++++++++++++++++++++++---------------------
1 file changed, 23 insertions(+), 21 deletions(-)
Index: linux-2.6/mm/shmem.c
===================================================================
--- linux-2.6.orig/mm/shmem.c
+++ linux-2.6/mm/shmem.c
@@ -730,10 +730,11 @@ done2:
if (inode->i_mapping->nrpages && (info->flags & SHMEM_PAGEIN)) {
/*
* Call truncate_inode_pages again: racing shmem_unuse_inode
- * may have swizzled a page in from swap since vmtruncate or
- * generic_delete_inode did it, before we lowered next_index.
- * Also, though shmem_getpage checks i_size before adding to
- * cache, no recheck after: so fix the narrow window there too.
+ * may have swizzled a page in from swap since
+ * truncate_pagecache or generic_delete_inode did it, before we
+ * lowered next_index. Also, though shmem_getpage checks
+ * i_size before adding to cache, no recheck after: so fix the
+ * narrow window there too.
*
* Recalling truncate_inode_pages_range and unmap_mapping_range
* every time for punch_hole (which never got a chance to clear
@@ -763,19 +764,16 @@ done2:
}
}
-static void shmem_truncate(struct inode *inode)
-{
- shmem_truncate_range(inode, inode->i_size, (loff_t)-1);
-}
-
static int shmem_notify_change(struct dentry *dentry, struct iattr *attr)
{
struct inode *inode = dentry->d_inode;
- struct page *page = NULL;
int error;
if (S_ISREG(inode->i_mode) && (attr->ia_valid & ATTR_SIZE)) {
- if (attr->ia_size < inode->i_size) {
+ loff_t newsize = attr->ia_size;
+ struct page *page = NULL;
+
+ if (newsize < inode->i_size) {
/*
* If truncating down to a partial page, then
* if that page is already allocated, hold it
@@ -783,9 +781,9 @@ static int shmem_notify_change(struct de
* truncate_partial_page cannnot miss it were
* it assigned to swap.
*/
- if (attr->ia_size & (PAGE_CACHE_SIZE-1)) {
+ if (newsize & (PAGE_CACHE_SIZE-1)) {
(void) shmem_getpage(inode,
- attr->ia_size>>PAGE_CACHE_SHIFT,
+ newsize >> PAGE_CACHE_SHIFT,
&page, SGP_READ, NULL);
if (page)
unlock_page(page);
@@ -797,24 +795,29 @@ static int shmem_notify_change(struct de
* if it's being fully truncated to zero-length: the
* nrpages check is efficient enough in that case.
*/
- if (attr->ia_size) {
+ if (newsize) {
struct shmem_inode_info *info = SHMEM_I(inode);
spin_lock(&info->lock);
info->flags &= ~SHMEM_PAGEIN;
spin_unlock(&info->lock);
}
}
+
+ error = simple_setsize(inode, newsize);
+ if (page)
+ page_cache_release(page);
+ if (error)
+ return error;
+ shmem_truncate_range(inode, newsize, (loff_t)-1);
}
error = inode_change_ok(inode, attr);
if (!error)
- error = inode_setattr(inode, attr);
+ generic_setattr(inode, attr);
#ifdef CONFIG_TMPFS_POSIX_ACL
if (!error && (attr->ia_valid & ATTR_MODE))
error = generic_acl_chmod(inode, &shmem_acl_ops);
#endif
- if (page)
- page_cache_release(page);
return error;
}
@@ -822,11 +825,11 @@ static void shmem_delete_inode(struct in
{
struct shmem_inode_info *info = SHMEM_I(inode);
- if (inode->i_op->truncate == shmem_truncate) {
+ if (inode->i_mapping->a_ops == &shmem_aops) {
truncate_inode_pages(inode->i_mapping, 0);
shmem_unacct_size(info->flags, inode->i_size);
inode->i_size = 0;
- shmem_truncate(inode);
+ shmem_truncate_range(inode, 0, (loff_t)-1);
if (!list_empty(&info->swaplist)) {
mutex_lock(&shmem_swaplist_mutex);
list_del_init(&info->swaplist);
@@ -2018,7 +2021,6 @@ static const struct inode_operations shm
};
static const struct inode_operations shmem_symlink_inode_operations = {
- .truncate = shmem_truncate,
.readlink = generic_readlink,
.follow_link = shmem_follow_link,
.put_link = shmem_put_link,
@@ -2438,7 +2440,7 @@ static const struct file_operations shme
};
static const struct inode_operations shmem_inode_operations = {
- .truncate = shmem_truncate,
+ .new_truncate = 1,
.setattr = shmem_notify_change,
.truncate_range = shmem_truncate_range,
#ifdef CONFIG_TMPFS_POSIX_ACL
^ permalink raw reply [flat|nested] 35+ messages in thread
* [patch 06/11] ext2: convert to use the new truncate convention.
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
` (4 preceding siblings ...)
2009-08-20 16:35 ` [patch 05/11] tmpfs: convert to use the new truncate convention npiggin
@ 2009-08-20 16:35 ` npiggin
2009-08-21 13:42 ` Jan Kara
2009-08-20 16:35 ` [patch 07/11] fat: convert to use the new truncate convention npiggin
` (10 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: npiggin @ 2009-08-20 16:35 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, Christoph Hellwig, linux-ext4
[-- Attachment #1: ext2-new-truncate.patch --]
[-- Type: text/plain, Size: 8260 bytes --]
I also have commented a possible bug in existing ext2 code, marked with XXX.
Cc: linux-ext4@vger.kernel.org
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nick Piggin <npiggin@suse.de>
---
fs/ext2/ext2.h | 1
fs/ext2/file.c | 2
fs/ext2/inode.c | 142 +++++++++++++++++++++++++++++++++++++++++++++-----------
3 files changed, 116 insertions(+), 29 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.
*/
@@ -68,7 +70,7 @@ void ext2_delete_inode (struct inode * i
inode->i_size = 0;
if (inode->i_blocks)
- ext2_truncate (inode);
+ ext2_truncate_blocks(inode, 0);
ext2_free_inode (inode);
return;
@@ -761,8 +763,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 +797,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 +832,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 +855,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 +1062,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 +1074,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 +1143,62 @@ 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.
+ *
+ * Also would be nice to be able to handle IO errors and such,
+ * but that's probably too much to ask.
+ */
+ 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;
+ __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 +1206,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,8 +1540,15 @@ int ext2_setattr(struct dentry *dentry,
if (error)
return error;
}
- error = inode_setattr(inode, iattr);
+ if (iattr->ia_valid & ATTR_SIZE) {
+ error = ext2_setsize(inode, iattr->ia_size);
+ if (error)
+ return error;
+ }
+ generic_setattr(inode, iattr);
if (!error && (iattr->ia_valid & ATTR_MODE))
error = ext2_acl_chmod(inode);
+ mark_inode_dirty(inode);
+
return error;
}
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,
^ permalink raw reply [flat|nested] 35+ messages in thread
* [patch 07/11] fat: convert to use the new truncate convention.
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
` (5 preceding siblings ...)
2009-08-20 16:35 ` [patch 06/11] ext2: " npiggin
@ 2009-08-20 16:35 ` npiggin
2009-08-20 16:35 ` [patch 08/11] btrfs: " npiggin
` (9 subsequent siblings)
16 siblings, 0 replies; 35+ messages in thread
From: npiggin @ 2009-08-20 16:35 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, Christoph Hellwig, OGAWA Hirofumi
[-- Attachment #1: fat-new-truncate.patch --]
[-- Type: text/plain, Size: 5693 bytes --]
Cc: OGAWA Hirofumi <hirofumi@mail.parknet.co.jp>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nick Piggin <npiggin@suse.de>
---
fs/fat/fat.h | 2 +-
fs/fat/file.c | 35 +++++++++++++++++++++++++++--------
fs/fat/inode.c | 25 ++++++++++++++++++++++---
3 files changed, 50 insertions(+), 12 deletions(-)
Index: linux-2.6/fs/fat/fat.h
===================================================================
--- linux-2.6.orig/fs/fat/fat.h
+++ linux-2.6/fs/fat/fat.h
@@ -302,7 +302,7 @@ extern int fat_generic_ioctl(struct inod
extern const struct file_operations fat_file_operations;
extern const struct inode_operations fat_file_inode_operations;
extern int fat_setattr(struct dentry * dentry, struct iattr * attr);
-extern void fat_truncate(struct inode *inode);
+extern void fat_truncate_blocks(struct inode *inode, loff_t offset);
extern int fat_getattr(struct vfsmount *mnt, struct dentry *dentry,
struct kstat *stat);
extern int fat_file_fsync(struct file *file, struct dentry *dentry,
Index: linux-2.6/fs/fat/file.c
===================================================================
--- linux-2.6.orig/fs/fat/file.c
+++ linux-2.6/fs/fat/file.c
@@ -252,7 +252,7 @@ static int fat_free(struct inode *inode,
return fat_free_clusters(inode, free_start);
}
-void fat_truncate(struct inode *inode)
+void fat_truncate_blocks(struct inode *inode, loff_t offset)
{
struct msdos_sb_info *sbi = MSDOS_SB(inode->i_sb);
const unsigned int cluster_size = sbi->cluster_size;
@@ -262,10 +262,10 @@ void fat_truncate(struct inode *inode)
* This protects against truncating a file bigger than it was then
* trying to write into the hole.
*/
- if (MSDOS_I(inode)->mmu_private > inode->i_size)
- MSDOS_I(inode)->mmu_private = inode->i_size;
+ if (MSDOS_I(inode)->mmu_private > offset)
+ MSDOS_I(inode)->mmu_private = offset;
- nr_clusters = (inode->i_size + (cluster_size - 1)) >> sbi->cluster_bits;
+ nr_clusters = (offset + (cluster_size - 1)) >> sbi->cluster_bits;
fat_free(inode, nr_clusters);
fat_flush_inodes(inode->i_sb, inode, NULL);
@@ -333,6 +333,18 @@ static int fat_allow_set_time(struct msd
return 0;
}
+static int fat_setsize(struct inode *inode, loff_t offset)
+{
+ int error;
+
+ error = simple_setsize(inode, offset);
+ if (error)
+ return error;
+ fat_truncate_blocks(inode, offset);
+
+ return error;
+}
+
#define TIMES_SET_FLAGS (ATTR_MTIME_SET | ATTR_ATIME_SET | ATTR_TIMES_SET)
/* valid file mode bits */
#define FAT_VALID_MODE (S_IFREG | S_IFDIR | S_IRWXUGO)
@@ -347,7 +359,8 @@ int fat_setattr(struct dentry *dentry, s
/*
* Expand the file. Since inode_setattr() updates ->i_size
* before calling the ->truncate(), but FAT needs to fill the
- * hole before it.
+ * hole before it. XXX: this is no longer true with new truncate
+ * sequence.
*/
if (attr->ia_valid & ATTR_SIZE) {
if (attr->ia_size > inode->i_size) {
@@ -396,15 +409,21 @@ int fat_setattr(struct dentry *dentry, s
attr->ia_valid &= ~ATTR_MODE;
}
- if (attr->ia_valid)
- error = inode_setattr(inode, attr);
+ if (attr->ia_valid & ATTR_SIZE) {
+ error = fat_setsize(inode, attr->ia_size);
+ if (error)
+ goto out;
+ }
+
+ generic_setattr(inode, attr);
+ mark_inode_dirty(inode);
out:
return error;
}
EXPORT_SYMBOL_GPL(fat_setattr);
const struct inode_operations fat_file_inode_operations = {
- .truncate = fat_truncate,
+ .new_truncate = 1,
.setattr = fat_setattr,
.getattr = fat_getattr,
};
Index: linux-2.6/fs/fat/inode.c
===================================================================
--- linux-2.6.orig/fs/fat/inode.c
+++ linux-2.6/fs/fat/inode.c
@@ -146,10 +146,19 @@ static int fat_write_begin(struct file *
loff_t pos, unsigned len, unsigned flags,
struct page **pagep, void **fsdata)
{
+ int err;
+
*pagep = NULL;
- return cont_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+ err = cont_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
fat_get_block,
&MSDOS_I(mapping->host)->mmu_private);
+ if (err < 0) {
+ struct inode *inode = mapping->host;
+ loff_t isize = inode->i_size;
+ if (pos + len > isize)
+ fat_truncate_blocks(inode, isize);
+ }
+ return err;
}
static int fat_write_end(struct file *file, struct address_space *mapping,
@@ -159,6 +168,11 @@ static int fat_write_end(struct file *fi
struct inode *inode = mapping->host;
int err;
err = generic_write_end(file, mapping, pos, len, copied, pagep, fsdata);
+ if (err < len) {
+ loff_t isize = inode->i_size;
+ if (pos + len > isize)
+ fat_truncate_blocks(inode, isize);
+ }
if (!(err < 0) && !(MSDOS_I(inode)->i_attrs & ATTR_ARCH)) {
inode->i_mtime = inode->i_ctime = CURRENT_TIME_SEC;
MSDOS_I(inode)->i_attrs |= ATTR_ARCH;
@@ -173,6 +187,7 @@ static ssize_t fat_direct_IO(int rw, str
{
struct file *file = iocb->ki_filp;
struct inode *inode = file->f_mapping->host;
+ ssize_t ret;
if (rw == WRITE) {
/*
@@ -193,8 +208,12 @@ static ssize_t fat_direct_IO(int rw, str
* FAT need to use the DIO_LOCKING for avoiding the race
* condition of fat_get_block() and ->truncate().
*/
- 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, fat_get_block, NULL);
+ if (ret < 0 && (rw & WRITE))
+ fat_truncate_blocks(inode, inode->i_size);
+
+ return ret;
}
static sector_t _fat_bmap(struct address_space *mapping, sector_t block)
@@ -429,7 +448,7 @@ static void fat_delete_inode(struct inod
{
truncate_inode_pages(&inode->i_data, 0);
inode->i_size = 0;
- fat_truncate(inode);
+ fat_truncate_blocks(inode, 0);
clear_inode(inode);
}
^ permalink raw reply [flat|nested] 35+ messages in thread
* [patch 08/11] btrfs: convert to use the new truncate convention.
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
` (6 preceding siblings ...)
2009-08-20 16:35 ` [patch 07/11] fat: convert to use the new truncate convention npiggin
@ 2009-08-20 16:35 ` npiggin
2009-08-20 16:35 ` [patch 09/11] jfs: " npiggin
` (8 subsequent siblings)
16 siblings, 0 replies; 35+ messages in thread
From: npiggin @ 2009-08-20 16:35 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, Christoph Hellwig, Chris Mason, linux-btrfs
[-- Attachment #1: btrfs-new-truncate.patch --]
[-- Type: text/plain, Size: 7395 bytes --]
Cc: Chris Mason <chris.mason@oracle.com>
Cc: linux-btrfs@vger.kernel.org
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nick Piggin <npiggin@suse.de>
---
fs/btrfs/ctree.h | 1
fs/btrfs/inode.c | 132 +++++++++++++++++++++++++++++++++++--------------------
fs/btrfs/ioctl.c | 2
3 files changed, 86 insertions(+), 49 deletions(-)
Index: linux-2.6/fs/btrfs/inode.c
===================================================================
--- linux-2.6.orig/fs/btrfs/inode.c
+++ linux-2.6/fs/btrfs/inode.c
@@ -81,7 +81,6 @@ static unsigned char btrfs_type_by_mode[
[S_IFLNK >> S_SHIFT] = BTRFS_FT_SYMLINK,
};
-static void btrfs_truncate(struct inode *inode);
static int btrfs_finish_ordered_io(struct inode *inode, u64 start, u64 end);
static noinline int cow_file_range(struct inode *inode,
struct page *locked_page,
@@ -1990,7 +1989,7 @@ void btrfs_orphan_cleanup(struct btrfs_r
/* if we have links, this was a truncate, lets do that */
if (inode->i_nlink) {
nr_truncate++;
- btrfs_truncate(inode);
+ btrfs_truncate_blocks(inode, inode->i_size);
} else {
nr_unlink++;
}
@@ -2956,39 +2955,6 @@ int btrfs_cont_expand(struct inode *inod
return err;
}
-static int btrfs_setattr(struct dentry *dentry, struct iattr *attr)
-{
- struct inode *inode = dentry->d_inode;
- int err;
-
- err = inode_change_ok(inode, attr);
- if (err)
- return err;
-
- if (S_ISREG(inode->i_mode) && (attr->ia_valid & ATTR_SIZE)) {
- if (attr->ia_size > inode->i_size) {
- err = btrfs_cont_expand(inode, attr->ia_size);
- if (err)
- return err;
- } else if (inode->i_size > 0 &&
- attr->ia_size == 0) {
-
- /* we're truncating a file that used to have good
- * data down to zero. Make sure it gets into
- * the ordered flush list so that any new writes
- * get down to disk quickly.
- */
- BTRFS_I(inode)->ordered_data_close = 1;
- }
- }
-
- err = inode_setattr(inode, attr);
-
- if (!err && ((attr->ia_valid & ATTR_MODE)))
- err = btrfs_acl_chmod(inode);
- return err;
-}
-
void btrfs_delete_inode(struct inode *inode)
{
struct btrfs_trans_handle *trans;
@@ -4438,7 +4404,7 @@ static void btrfs_invalidatepage(struct
*
* We are not allowed to take the i_mutex here so we have to play games to
* protect against truncate races as the page could now be beyond EOF. Because
- * vmtruncate() writes the inode size before removing pages, once we have the
+ * btrfs_setsize() writes the inode size before removing pages, once we have the
* page lock we can determine safely if the page is beyond EOF. If it is not
* beyond EOF, then the page is guaranteed safe against truncation until we
* unlock the page.
@@ -4524,7 +4490,7 @@ out:
return ret;
}
-static void btrfs_truncate(struct inode *inode)
+static void __btrfs_truncate_blocks(struct inode *inode, loff_t offset)
{
struct btrfs_root *root = BTRFS_I(inode)->root;
int ret;
@@ -4532,13 +4498,8 @@ static void btrfs_truncate(struct inode
unsigned long nr;
u64 mask = root->sectorsize - 1;
- if (!S_ISREG(inode->i_mode))
- return;
- if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
- return;
-
- btrfs_truncate_page(inode->i_mapping, inode->i_size);
- btrfs_wait_ordered_range(inode, inode->i_size & (~mask), (u64)-1);
+ btrfs_truncate_page(inode->i_mapping, offset);
+ btrfs_wait_ordered_range(inode, offset & (~mask), (u64)-1);
trans = btrfs_start_transaction(root, 1);
@@ -4559,17 +4520,17 @@ static void btrfs_truncate(struct inode
* using truncate to replace the contents of the file will
* end up with a zero length file after a crash.
*/
- if (inode->i_size == 0 && BTRFS_I(inode)->ordered_data_close)
+ if (offset == 0 && BTRFS_I(inode)->ordered_data_close)
btrfs_add_ordered_operation(trans, root, inode);
btrfs_set_trans_block_group(trans, inode);
- btrfs_i_size_write(inode, inode->i_size);
+ btrfs_i_size_write(inode, offset);
ret = btrfs_orphan_add(trans, inode);
if (ret)
goto out;
/* FIXME, add redo link to tree so we don't leak on crash */
- ret = btrfs_truncate_inode_items(trans, root, inode, inode->i_size,
+ ret = btrfs_truncate_inode_items(trans, root, inode, offset,
BTRFS_EXTENT_DATA_KEY);
btrfs_update_inode(trans, root, inode);
@@ -4583,6 +4544,81 @@ out:
btrfs_btree_balance_dirty(root, nr);
}
+void btrfs_truncate_blocks(struct inode *inode, loff_t offset)
+{
+ if (!S_ISREG(inode->i_mode))
+ return;
+ /* XXX: should allow IS_APPEND and IS_IMMUTABLE files to have
+ * blocks trimmed past i_size */
+ if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
+ return;
+
+ __btrfs_truncate_blocks(inode, offset);
+ mark_inode_dirty(inode);
+}
+
+static int btrfs_setsize(struct inode *inode, loff_t newsize)
+{
+ int error;
+ loff_t oldsize;
+
+ if (!S_ISREG(inode->i_mode))
+ return -EINVAL;
+ if (IS_APPEND(inode) || IS_IMMUTABLE(inode))
+ return -EPERM;
+
+ error = inode_newsize_ok(inode, newsize);
+ if (error)
+ return error;
+
+ oldsize = inode->i_size;
+ __btrfs_truncate_blocks(inode, newsize);
+ truncate_pagecache(inode, oldsize, newsize);
+
+ return error;
+}
+
+static int btrfs_setattr(struct dentry *dentry, struct iattr *attr)
+{
+ struct inode *inode = dentry->d_inode;
+ int err;
+
+ err = inode_change_ok(inode, attr);
+ if (err)
+ return err;
+
+ if (S_ISREG(inode->i_mode) && (attr->ia_valid & ATTR_SIZE)) {
+ loff_t newsize = attr->ia_size;
+
+ if (newsize > inode->i_size) {
+ err = btrfs_cont_expand(inode, newsize);
+ if (err)
+ return err;
+ } else if (inode->i_size > 0 &&
+ newsize == 0) {
+
+ /* we're truncating a file that used to have good
+ * data down to zero. Make sure it gets into
+ * the ordered flush list so that any new writes
+ * get down to disk quickly.
+ */
+ BTRFS_I(inode)->ordered_data_close = 1;
+ }
+
+ err = btrfs_setsize(inode, newsize);
+ if (err)
+ return err;
+ }
+
+ generic_setattr(inode, attr);
+
+ if ((attr->ia_valid & ATTR_MODE))
+ err = btrfs_acl_chmod(inode);
+ mark_inode_dirty(inode);
+
+ return err;
+}
+
/*
* create a new subvolume directory/inode (helper for the ioctl).
*/
@@ -5272,7 +5308,7 @@ static struct address_space_operations b
};
static struct inode_operations btrfs_file_inode_operations = {
- .truncate = btrfs_truncate,
+ .new_truncate = 1,
.getattr = btrfs_getattr,
.setattr = btrfs_setattr,
.setxattr = btrfs_setxattr,
Index: linux-2.6/fs/btrfs/ioctl.c
===================================================================
--- linux-2.6.orig/fs/btrfs/ioctl.c
+++ linux-2.6/fs/btrfs/ioctl.c
@@ -1148,7 +1148,7 @@ out:
btrfs_end_transaction(trans, root);
unlock_extent(&BTRFS_I(src)->io_tree, off, off+len, GFP_NOFS);
if (ret)
- vmtruncate(inode, 0);
+ btrfs_truncate_blocks(inode, 0);
out_unlock:
mutex_unlock(&src->i_mutex);
mutex_unlock(&inode->i_mutex);
Index: linux-2.6/fs/btrfs/ctree.h
===================================================================
--- linux-2.6.orig/fs/btrfs/ctree.h
+++ linux-2.6/fs/btrfs/ctree.h
@@ -2275,6 +2275,7 @@ int btrfs_orphan_add(struct btrfs_trans_
int btrfs_orphan_del(struct btrfs_trans_handle *trans, struct inode *inode);
void btrfs_orphan_cleanup(struct btrfs_root *root);
int btrfs_cont_expand(struct inode *inode, loff_t size);
+void btrfs_truncate_blocks(struct inode *inode, loff_t offset);
/* ioctl.c */
long btrfs_ioctl(struct file *file, unsigned int cmd, unsigned long arg);
^ permalink raw reply [flat|nested] 35+ messages in thread
* [patch 09/11] jfs: convert to use the new truncate convention.
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
` (7 preceding siblings ...)
2009-08-20 16:35 ` [patch 08/11] btrfs: " npiggin
@ 2009-08-20 16:35 ` npiggin
2009-08-20 16:35 ` [patch 10/11] udf: " npiggin
` (7 subsequent siblings)
16 siblings, 0 replies; 35+ messages in thread
From: npiggin @ 2009-08-20 16:35 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, Christoph Hellwig, Dave Kleikamp
[-- Attachment #1: jfs-new-truncate.patch --]
[-- Type: text/plain, Size: 7029 bytes --]
Note I wasn't able to test jfs because the kernel wasn't mounting the
product of my mkfs.jfs for some reason.
Cc: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nick Piggin <npiggin@suse.de>
---
fs/jfs/acl.c | 12 ++---------
fs/jfs/file.c | 2 -
fs/jfs/inode.c | 56 +++++++++++++++++++++++++++++++++++++++++++----------
fs/jfs/jfs_inode.h | 5 ++--
fs/jfs/jfs_xtree.c | 2 -
fs/jfs/namei.c | 6 ++---
6 files changed, 57 insertions(+), 26 deletions(-)
Index: linux-2.6/fs/jfs/acl.c
===================================================================
--- linux-2.6.orig/fs/jfs/acl.c
+++ linux-2.6/fs/jfs/acl.c
@@ -216,20 +216,14 @@ int jfs_setattr(struct dentry *dentry, s
struct inode *inode = dentry->d_inode;
int rc;
- rc = inode_change_ok(inode, iattr);
+ rc = simple_setattr(dentry, iattr);
if (rc)
return rc;
- if ((iattr->ia_valid & ATTR_UID && iattr->ia_uid != inode->i_uid) ||
- (iattr->ia_valid & ATTR_GID && iattr->ia_gid != inode->i_gid)) {
- if (vfs_dq_transfer(inode, iattr))
- return -EDQUOT;
- }
-
- rc = inode_setattr(inode, iattr);
-
if (!rc && (iattr->ia_valid & ATTR_MODE))
rc = jfs_acl_chmod(inode);
+ mark_inode_dirty(inode);
+
return rc;
}
Index: linux-2.6/fs/jfs/file.c
===================================================================
--- linux-2.6.orig/fs/jfs/file.c
+++ linux-2.6/fs/jfs/file.c
@@ -89,7 +89,7 @@ static int jfs_release(struct inode *ino
}
const struct inode_operations jfs_file_inode_operations = {
- .truncate = jfs_truncate,
+ .new_truncate = 1,
.setxattr = jfs_setxattr,
.getxattr = jfs_getxattr,
.listxattr = jfs_listxattr,
Index: linux-2.6/fs/jfs/inode.c
===================================================================
--- linux-2.6.orig/fs/jfs/inode.c
+++ linux-2.6/fs/jfs/inode.c
@@ -297,8 +297,33 @@ static int jfs_write_begin(struct file *
loff_t pos, unsigned len, unsigned flags,
struct page **pagep, void **fsdata)
{
- return nobh_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+ int ret;
+
+ ret = nobh_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
jfs_get_block);
+ if (ret < 0) {
+ struct inode *inode = mapping->host;
+ loff_t isize = inode->i_size;
+ if (pos + len > isize)
+ jfs_truncate_blocks(inode, isize);
+ }
+ return ret;
+}
+
+static int jfs_write_end(struct file *file, struct address_space *mapping,
+ loff_t pos, unsigned len, unsigned copied,
+ struct page *page, void *fsdata)
+{
+ int ret;
+
+ ret = nobh_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)
+ jfs_truncate_blocks(inode, isize);
+ }
+ return ret;
}
static sector_t jfs_bmap(struct address_space *mapping, sector_t block)
@@ -311,9 +336,13 @@ static ssize_t jfs_direct_IO(int rw, str
{
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, jfs_get_block, NULL);
+ if (ret < 0)
+ jfs_truncate_blocks(inode, inode->i_size);
+ return ret;
}
const struct address_space_operations jfs_aops = {
@@ -323,16 +352,16 @@ const struct address_space_operations jf
.writepages = jfs_writepages,
.sync_page = block_sync_page,
.write_begin = jfs_write_begin,
- .write_end = nobh_write_end,
+ .write_end = jfs_write_end,
.bmap = jfs_bmap,
.direct_IO = jfs_direct_IO,
};
/*
- * Guts of jfs_truncate. Called with locks already held. Can be called
+ * Guts of jfs_truncate_blocks. Called with locks already held. Can be called
* with directory for truncating directory index table.
*/
-void jfs_truncate_nolock(struct inode *ip, loff_t length)
+void jfs_truncate_blocks_nolock(struct inode *ip, loff_t length)
{
loff_t newsize;
tid_t tid;
@@ -372,13 +401,20 @@ void jfs_truncate_nolock(struct inode *i
} while (newsize > length); /* Truncate isn't always atomic */
}
-void jfs_truncate(struct inode *ip)
+void jfs_truncate_blocks(struct inode *ip, loff_t offset)
{
- jfs_info("jfs_truncate: size = 0x%lx", (ulong) ip->i_size);
-
- nobh_truncate_page(ip->i_mapping, ip->i_size, jfs_get_block);
+ jfs_info("jfs_truncate: size = 0x%lx", (ulong) offset);
IWRITE_LOCK(ip, RDWRLOCK_NORMAL);
- jfs_truncate_nolock(ip, ip->i_size);
+ jfs_truncate_blocks_nolock(ip, offset);
IWRITE_UNLOCK(ip);
}
+
+void jfs_setsize(struct inode *ip, loff_t offset)
+{
+ simple_setsize(ip, offset);
+
+ nobh_truncate_page(ip->i_mapping, offset, jfs_get_block);
+
+ jfs_truncate_blocks(ip, offset);
+}
Index: linux-2.6/fs/jfs/jfs_inode.h
===================================================================
--- linux-2.6.orig/fs/jfs/jfs_inode.h
+++ linux-2.6/fs/jfs/jfs_inode.h
@@ -29,8 +29,9 @@ extern int jfs_commit_inode(struct inode
extern int jfs_write_inode(struct inode*, int);
extern void jfs_delete_inode(struct inode *);
extern void jfs_dirty_inode(struct inode *);
-extern void jfs_truncate(struct inode *);
-extern void jfs_truncate_nolock(struct inode *, loff_t);
+extern void jfs_truncate_blocks(struct inode *, loff_t);
+extern void jfs_truncate_blocks_nolock(struct inode *, loff_t);
+extern void jfs_setsize(struct inode *, loff_t);
extern void jfs_free_zero_link(struct inode *);
extern struct dentry *jfs_get_parent(struct dentry *dentry);
extern void jfs_get_inode_flags(struct jfs_inode_info *);
Index: linux-2.6/fs/jfs/jfs_xtree.c
===================================================================
--- linux-2.6.orig/fs/jfs/jfs_xtree.c
+++ linux-2.6/fs/jfs/jfs_xtree.c
@@ -3132,7 +3132,7 @@ void xtInitRoot(tid_t tid, struct inode
* note:
* PWMAP:
* 1. truncate (non-COMMIT_NOLINK file)
- * by jfs_truncate() or jfs_open(O_TRUNC):
+ * by jfs_setsize() or jfs_open(O_TRUNC):
* xtree is updated;
* 2. truncate index table of directory when last entry removed
* map update via tlock at commit time;
Index: linux-2.6/fs/jfs/namei.c
===================================================================
--- linux-2.6.orig/fs/jfs/namei.c
+++ linux-2.6/fs/jfs/namei.c
@@ -435,7 +435,7 @@ static int jfs_rmdir(struct inode *dip,
*/
if (test_cflag(COMMIT_Stale, dip)) {
if (dip->i_size > 1)
- jfs_truncate_nolock(dip, 0);
+ jfs_truncate_blocks_nolock(dip, 0);
clear_cflag(COMMIT_Stale, dip);
}
@@ -586,7 +586,7 @@ static int jfs_unlink(struct inode *dip,
*/
if (test_cflag(COMMIT_Stale, dip)) {
if (dip->i_size > 1)
- jfs_truncate_nolock(dip, 0);
+ jfs_truncate_blocks_nolock(dip, 0);
clear_cflag(COMMIT_Stale, dip);
}
@@ -1327,7 +1327,7 @@ static int jfs_rename(struct inode *old_
*/
if (test_cflag(COMMIT_Stale, old_dir)) {
if (old_dir->i_size > 1)
- jfs_truncate_nolock(old_dir, 0);
+ jfs_truncate_blocks_nolock(old_dir, 0);
clear_cflag(COMMIT_Stale, old_dir);
}
^ permalink raw reply [flat|nested] 35+ messages in thread
* [patch 10/11] udf: convert to use the new truncate convention.
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
` (8 preceding siblings ...)
2009-08-20 16:35 ` [patch 09/11] jfs: " npiggin
@ 2009-08-20 16:35 ` npiggin
2009-08-21 14:22 ` Jan Kara
2009-08-20 16:35 ` [patch 11/11] minix: " npiggin
` (6 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: npiggin @ 2009-08-20 16:35 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, Christoph Hellwig, Jan Kara
[-- Attachment #1: udf-new-truncate.patch --]
[-- Type: text/plain, Size: 4419 bytes --]
Cc: Jan Kara <jack@suse.cz>
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nick Piggin <npiggin@suse.de>
---
fs/udf/file.c | 3 +-
fs/udf/inode.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++----
fs/udf/udfdecl.h | 2 -
3 files changed, 71 insertions(+), 7 deletions(-)
Index: linux-2.6/fs/udf/inode.c
===================================================================
--- linux-2.6.orig/fs/udf/inode.c
+++ linux-2.6/fs/udf/inode.c
@@ -67,6 +67,7 @@ static void udf_update_extents(struct in
struct extent_position *);
static int udf_get_block(struct inode *, sector_t, struct buffer_head *, int);
+static void udf_truncate_blocks(struct inode *inode);
void udf_delete_inode(struct inode *inode)
{
@@ -76,7 +77,7 @@ void udf_delete_inode(struct inode *inod
goto no_delete;
inode->i_size = 0;
- udf_truncate(inode);
+ udf_truncate_blocks(inode);
lock_kernel();
udf_update_inode(inode, IS_SYNC(inode));
@@ -127,9 +128,34 @@ static int udf_write_begin(struct file *
loff_t pos, unsigned len, unsigned flags,
struct page **pagep, void **fsdata)
{
+ int ret;
+
*pagep = NULL;
- return block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+ ret = block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
udf_get_block);
+ if (ret < 0) {
+ struct inode *inode = mapping->host;
+ loff_t isize = inode->i_size;
+ if (pos + len > isize)
+ udf_truncate_blocks(inode);
+ }
+ return ret;
+}
+
+static int udf_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)
+ udf_truncate_blocks(inode);
+ }
+ return ret;
}
static sector_t udf_bmap(struct address_space *mapping, sector_t block)
@@ -141,8 +167,8 @@ const struct address_space_operations ud
.readpage = udf_readpage,
.writepage = udf_writepage,
.sync_page = block_sync_page,
- .write_begin = udf_write_begin,
- .write_end = generic_write_end,
+ .write_begin = udf_write_begin,
+ .write_end = udf_write_end,
.bmap = udf_bmap,
};
@@ -1011,7 +1037,7 @@ struct buffer_head *udf_bread(struct ino
return NULL;
}
-void udf_truncate(struct inode *inode)
+static void udf_truncate_blocks(struct inode *inode)
{
int offset;
int err;
@@ -1057,6 +1083,43 @@ void udf_truncate(struct inode *inode)
unlock_kernel();
}
+static int udf_setsize(struct inode *inode, loff_t newsize)
+{
+ loff_t oldsize;
+ int error;
+
+ error = inode_newsize_ok(inode, newsize);
+ if (error)
+ return error;
+
+ oldsize = inode->i_size;
+ i_size_write(inode, newsize);
+ truncate_pagecache(inode, oldsize, newsize);
+
+ udf_truncate_blocks(inode);
+
+ return error;
+}
+
+int udf_setattr(struct dentry *dentry, struct iattr *attr)
+{
+ struct inode *inode = dentry->d_inode;
+ int error;
+
+ if (attr->ia_valid & ATTR_SIZE) {
+ error = udf_setsize(inode, attr->ia_size);
+ if (error)
+ return error;
+ }
+
+ error = simple_setattr(dentry, attr);
+ if (error)
+ return error;
+
+ mark_inode_dirty(inode);
+ return error;
+}
+
static void __udf_read_inode(struct inode *inode)
{
struct buffer_head *bh = NULL;
Index: linux-2.6/fs/udf/udfdecl.h
===================================================================
--- linux-2.6.orig/fs/udf/udfdecl.h
+++ linux-2.6/fs/udf/udfdecl.h
@@ -138,7 +138,7 @@ extern int udf_sync_inode(struct inode *
extern void udf_expand_file_adinicb(struct inode *, int, int *);
extern struct buffer_head *udf_expand_dir_adinicb(struct inode *, int *, int *);
extern struct buffer_head *udf_bread(struct inode *, int, int, int *);
-extern void udf_truncate(struct inode *);
+extern int udf_setattr(struct dentry *, struct iattr *);
extern void udf_read_inode(struct inode *);
extern void udf_delete_inode(struct inode *);
extern void udf_clear_inode(struct inode *);
Index: linux-2.6/fs/udf/file.c
===================================================================
--- linux-2.6.orig/fs/udf/file.c
+++ linux-2.6/fs/udf/file.c
@@ -215,5 +215,6 @@ const struct file_operations udf_file_op
};
const struct inode_operations udf_file_inode_operations = {
- .truncate = udf_truncate,
+ .new_truncate = 1,
+ .setattr = udf_setattr,
};
^ permalink raw reply [flat|nested] 35+ messages in thread
* [patch 11/11] minix: convert to use the new truncate convention.
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
` (9 preceding siblings ...)
2009-08-20 16:35 ` [patch 10/11] udf: " npiggin
@ 2009-08-20 16:35 ` npiggin
2009-09-09 7:11 ` [patch 00/11] new truncate sequence Artem Bityutskiy
` (5 subsequent siblings)
16 siblings, 0 replies; 35+ messages in thread
From: npiggin @ 2009-08-20 16:35 UTC (permalink / raw)
To: Andrew Morton; +Cc: linux-fsdevel, Christoph Hellwig
[-- Attachment #1: minix-new-truncate.patch --]
[-- Type: text/plain, Size: 7538 bytes --]
Cc: Christoph Hellwig <hch@lst.de>
Signed-off-by: Nick Piggin <npiggin@suse.de>
---
fs/minix/file.c | 3 +
fs/minix/inode.c | 82 +++++++++++++++++++++++++++++++++++++++++++-----
fs/minix/itree_common.c | 5 +-
fs/minix/itree_v1.c | 4 +-
fs/minix/itree_v2.c | 4 +-
fs/minix/minix.h | 6 +--
6 files changed, 85 insertions(+), 19 deletions(-)
Index: linux-2.6/fs/minix/file.c
===================================================================
--- linux-2.6.orig/fs/minix/file.c
+++ linux-2.6/fs/minix/file.c
@@ -24,6 +24,7 @@ const struct file_operations minix_file_
};
const struct inode_operations minix_file_inode_operations = {
- .truncate = minix_truncate,
+ .new_truncate = 1,
+ .setattr = minix_setattr,
.getattr = minix_getattr,
};
Index: linux-2.6/fs/minix/inode.c
===================================================================
--- linux-2.6.orig/fs/minix/inode.c
+++ linux-2.6/fs/minix/inode.c
@@ -18,15 +18,16 @@
#include <linux/highuid.h>
#include <linux/vfs.h>
-static int minix_write_inode(struct inode * inode, int wait);
+static int minix_write_inode(struct inode *inode, int wait);
static int minix_statfs(struct dentry *dentry, struct kstatfs *buf);
-static int minix_remount (struct super_block * sb, int * flags, char * data);
+static int minix_remount(struct super_block *sb, int *flags, char *data);
+static void minix_truncate_blocks(struct inode *inode, loff_t offset);
static void minix_delete_inode(struct inode *inode)
{
truncate_inode_pages(&inode->i_data, 0);
inode->i_size = 0;
- minix_truncate(inode);
+ minix_truncate_blocks(inode, 0);
minix_free_inode(inode);
}
@@ -367,8 +368,33 @@ static int minix_write_begin(struct file
loff_t pos, unsigned len, unsigned flags,
struct page **pagep, void **fsdata)
{
+ int ret;
+
*pagep = NULL;
- return __minix_write_begin(file, mapping, pos, len, flags, pagep, fsdata);
+ ret = __minix_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)
+ minix_truncate_blocks(inode, isize);
+ }
+ return ret;
+}
+
+static int minix_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)
+ minix_truncate_blocks(inode, isize);
+ }
+ return ret;
}
static sector_t minix_bmap(struct address_space *mapping, sector_t block)
@@ -381,7 +407,7 @@ static const struct address_space_operat
.writepage = minix_writepage,
.sync_page = block_sync_page,
.write_begin = minix_write_begin,
- .write_end = generic_write_end,
+ .write_end = minix_write_end,
.bmap = minix_bmap
};
@@ -591,14 +617,54 @@ int minix_getattr(struct vfsmount *mnt,
/*
* The function that is called for file truncation.
*/
-void minix_truncate(struct inode * inode)
+static void minix_truncate_blocks(struct inode *inode, loff_t offset)
{
if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode)))
return;
if (INODE_VERSION(inode) == MINIX_V1)
- V1_minix_truncate(inode);
+ V1_minix_truncate_blocks(inode, offset);
else
- V2_minix_truncate(inode);
+ V2_minix_truncate_blocks(inode, offset);
+}
+
+static int minix_setsize(struct inode *inode, loff_t newsize)
+{
+ loff_t oldsize;
+ int error;
+
+ error = inode_newsize_ok(inode, newsize);
+ if (error)
+ return error;
+
+ oldsize = inode->i_size;
+ i_size_write(inode, newsize);
+ truncate_pagecache(inode, oldsize, newsize);
+
+ error = block_truncate_page(inode->i_mapping, newsize, minix_get_block);
+ if (error)
+ return error;
+ minix_truncate_blocks(inode, newsize);
+
+ return error;
+}
+
+int minix_setattr(struct dentry *dentry, struct iattr *iattr)
+{
+ struct inode *inode = dentry->d_inode;
+ int error;
+
+ error = inode_change_ok(inode, iattr);
+ if (error)
+ return error;
+ if (iattr->ia_valid & ATTR_SIZE) {
+ error = minix_setsize(inode, iattr->ia_size);
+ if (error)
+ return error;
+ }
+ generic_setattr(inode, iattr);
+ mark_inode_dirty(inode);
+
+ return error;
}
static int minix_get_sb(struct file_system_type *fs_type,
Index: linux-2.6/fs/minix/itree_common.c
===================================================================
--- linux-2.6.orig/fs/minix/itree_common.c
+++ linux-2.6/fs/minix/itree_common.c
@@ -290,7 +290,7 @@ static void free_branches(struct inode *
free_data(inode, p, q);
}
-static inline void truncate (struct inode * inode)
+static inline void truncate_blocks(struct inode *inode, loff_t offset)
{
struct super_block *sb = inode->i_sb;
block_t *idata = i_data(inode);
@@ -302,8 +302,7 @@ static inline void truncate (struct inod
int first_whole;
long iblock;
- iblock = (inode->i_size + sb->s_blocksize -1) >> sb->s_blocksize_bits;
- block_truncate_page(inode->i_mapping, inode->i_size, get_block);
+ iblock = (offset + sb->s_blocksize - 1) >> sb->s_blocksize_bits;
n = block_to_path(inode, iblock, offsets);
if (!n)
Index: linux-2.6/fs/minix/itree_v1.c
===================================================================
--- linux-2.6.orig/fs/minix/itree_v1.c
+++ linux-2.6/fs/minix/itree_v1.c
@@ -55,9 +55,9 @@ int V1_minix_get_block(struct inode * in
return get_block(inode, block, bh_result, create);
}
-void V1_minix_truncate(struct inode * inode)
+void V1_minix_truncate_blocks(struct inode *inode, loff_t offset)
{
- truncate(inode);
+ truncate_blocks(inode, offset);
}
unsigned V1_minix_blocks(loff_t size, struct super_block *sb)
Index: linux-2.6/fs/minix/itree_v2.c
===================================================================
--- linux-2.6.orig/fs/minix/itree_v2.c
+++ linux-2.6/fs/minix/itree_v2.c
@@ -61,9 +61,9 @@ int V2_minix_get_block(struct inode * in
return get_block(inode, block, bh_result, create);
}
-void V2_minix_truncate(struct inode * inode)
+void V2_minix_truncate_blocks(struct inode *inode, loff_t offset)
{
- truncate(inode);
+ truncate_blocks(inode, offset);
}
unsigned V2_minix_blocks(loff_t size, struct super_block *sb)
Index: linux-2.6/fs/minix/minix.h
===================================================================
--- linux-2.6.orig/fs/minix/minix.h
+++ linux-2.6/fs/minix/minix.h
@@ -52,14 +52,14 @@ extern unsigned long minix_count_free_in
extern int minix_new_block(struct inode * inode);
extern void minix_free_block(struct inode *inode, unsigned long block);
extern unsigned long minix_count_free_blocks(struct minix_sb_info *sbi);
+extern int minix_setattr(struct dentry *dentry, struct iattr *iattr);
extern int minix_getattr(struct vfsmount *, struct dentry *, struct kstat *);
extern int __minix_write_begin(struct file *file, struct address_space *mapping,
loff_t pos, unsigned len, unsigned flags,
struct page **pagep, void **fsdata);
-extern void V1_minix_truncate(struct inode *);
-extern void V2_minix_truncate(struct inode *);
-extern void minix_truncate(struct inode *);
+extern void V1_minix_truncate_blocks(struct inode *, loff_t);
+extern void V2_minix_truncate_blocks(struct inode *, loff_t);
extern void minix_set_inode(struct inode *, dev_t);
extern int V1_minix_get_block(struct inode *, long, struct buffer_head *, int);
extern int V2_minix_get_block(struct inode *, long, struct buffer_head *, int);
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 06/11] ext2: convert to use the new truncate convention.
2009-08-20 16:35 ` [patch 06/11] ext2: " npiggin
@ 2009-08-21 13:42 ` Jan Kara
2009-08-21 14:06 ` Jan Kara
0 siblings, 1 reply; 35+ messages in thread
From: Jan Kara @ 2009-08-21 13:42 UTC (permalink / raw)
To: npiggin; +Cc: Andrew Morton, linux-fsdevel, Christoph Hellwig, linux-ext4
Hi,
> I also have commented a possible bug in existing ext2 code, marked with XXX.
Looks good, except:
> +int ext2_setsize(struct inode *inode, loff_t newsize)
This could be static.
> @@ -1459,8 +1540,15 @@ int ext2_setattr(struct dentry *dentry,
> if (error)
> return error;
> }
> - error = inode_setattr(inode, iattr);
> + if (iattr->ia_valid & ATTR_SIZE) {
> + error = ext2_setsize(inode, iattr->ia_size);
> + if (error)
> + return error;
> + }
> + generic_setattr(inode, iattr);
Here, we should store the error code I suppose...
Honza
--
Jan Kara <jack@suse.cz>
SuSE CR Labs
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 06/11] ext2: convert to use the new truncate convention.
2009-08-21 13:42 ` Jan Kara
@ 2009-08-21 14:06 ` Jan Kara
2009-08-24 5:30 ` [patch] ext2: convert to use the new truncate convention fix Nick Piggin
0 siblings, 1 reply; 35+ messages in thread
From: Jan Kara @ 2009-08-21 14:06 UTC (permalink / raw)
To: npiggin; +Cc: Andrew Morton, linux-fsdevel, Christoph Hellwig, linux-ext4
> Hi,
>
> > I also have commented a possible bug in existing ext2 code, marked with XXX.
> Looks good, except:
>
> > +int ext2_setsize(struct inode *inode, loff_t newsize)
> This could be static.
>
> > @@ -1459,8 +1540,15 @@ int ext2_setattr(struct dentry *dentry,
> > if (error)
> > return error;
> > }
> > - error = inode_setattr(inode, iattr);
> > + if (iattr->ia_valid & ATTR_SIZE) {
> > + error = ext2_setsize(inode, iattr->ia_size);
> > + if (error)
> > + return error;
> > + }
> > + generic_setattr(inode, iattr);
> Here, we should store the error code I suppose...
Ah, I was confused. generic_setattr() returns void. But then remove
the check !error from:
if (!error && (iattr->ia_valid & ATTR_MODE))
which just follows the generic_setattr(). That's what made me think
generic_setattr() returns something :)
Honza
--
Jan Kara <jack@suse.cz>
SuSE CR Labs
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 10/11] udf: convert to use the new truncate convention.
2009-08-20 16:35 ` [patch 10/11] udf: " npiggin
@ 2009-08-21 14:22 ` Jan Kara
2009-08-24 5:33 ` Nick Piggin
0 siblings, 1 reply; 35+ messages in thread
From: Jan Kara @ 2009-08-21 14:22 UTC (permalink / raw)
To: npiggin; +Cc: Andrew Morton, linux-fsdevel, Christoph Hellwig, Jan Kara
On Fri 21-08-09 02:35:14, npiggin@suse.de wrote:
> Cc: Jan Kara <jack@suse.cz>
> Cc: Christoph Hellwig <hch@lst.de>
> Signed-off-by: Nick Piggin <npiggin@suse.de>
The patch is fine.
Acked-by: Jan Kara <jack@suse.cz>
I won't pull the patch to my UDF tree for now since it depends on the
generic patches so I guess it's better if Andrew carries all the patches
in one bunch. Once we decide it's time to merge with Linus, feel free to
either merge it directly with my Ack or via my tree.
Honza
> ---
> fs/udf/file.c | 3 +-
> fs/udf/inode.c | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++----
> fs/udf/udfdecl.h | 2 -
> 3 files changed, 71 insertions(+), 7 deletions(-)
>
> Index: linux-2.6/fs/udf/inode.c
> ===================================================================
> --- linux-2.6.orig/fs/udf/inode.c
> +++ linux-2.6/fs/udf/inode.c
> @@ -67,6 +67,7 @@ static void udf_update_extents(struct in
> struct extent_position *);
> static int udf_get_block(struct inode *, sector_t, struct buffer_head *, int);
>
> +static void udf_truncate_blocks(struct inode *inode);
>
> void udf_delete_inode(struct inode *inode)
> {
> @@ -76,7 +77,7 @@ void udf_delete_inode(struct inode *inod
> goto no_delete;
>
> inode->i_size = 0;
> - udf_truncate(inode);
> + udf_truncate_blocks(inode);
> lock_kernel();
>
> udf_update_inode(inode, IS_SYNC(inode));
> @@ -127,9 +128,34 @@ static int udf_write_begin(struct file *
> loff_t pos, unsigned len, unsigned flags,
> struct page **pagep, void **fsdata)
> {
> + int ret;
> +
> *pagep = NULL;
> - return block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
> + ret = block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
> udf_get_block);
> + if (ret < 0) {
> + struct inode *inode = mapping->host;
> + loff_t isize = inode->i_size;
> + if (pos + len > isize)
> + udf_truncate_blocks(inode);
> + }
> + return ret;
> +}
> +
> +static int udf_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)
> + udf_truncate_blocks(inode);
> + }
> + return ret;
> }
>
> static sector_t udf_bmap(struct address_space *mapping, sector_t block)
> @@ -141,8 +167,8 @@ const struct address_space_operations ud
> .readpage = udf_readpage,
> .writepage = udf_writepage,
> .sync_page = block_sync_page,
> - .write_begin = udf_write_begin,
> - .write_end = generic_write_end,
> + .write_begin = udf_write_begin,
> + .write_end = udf_write_end,
> .bmap = udf_bmap,
> };
>
> @@ -1011,7 +1037,7 @@ struct buffer_head *udf_bread(struct ino
> return NULL;
> }
>
> -void udf_truncate(struct inode *inode)
> +static void udf_truncate_blocks(struct inode *inode)
> {
> int offset;
> int err;
> @@ -1057,6 +1083,43 @@ void udf_truncate(struct inode *inode)
> unlock_kernel();
> }
>
> +static int udf_setsize(struct inode *inode, loff_t newsize)
> +{
> + loff_t oldsize;
> + int error;
> +
> + error = inode_newsize_ok(inode, newsize);
> + if (error)
> + return error;
> +
> + oldsize = inode->i_size;
> + i_size_write(inode, newsize);
> + truncate_pagecache(inode, oldsize, newsize);
> +
> + udf_truncate_blocks(inode);
> +
> + return error;
> +}
> +
> +int udf_setattr(struct dentry *dentry, struct iattr *attr)
> +{
> + struct inode *inode = dentry->d_inode;
> + int error;
> +
> + if (attr->ia_valid & ATTR_SIZE) {
> + error = udf_setsize(inode, attr->ia_size);
> + if (error)
> + return error;
> + }
> +
> + error = simple_setattr(dentry, attr);
> + if (error)
> + return error;
> +
> + mark_inode_dirty(inode);
> + return error;
> +}
> +
> static void __udf_read_inode(struct inode *inode)
> {
> struct buffer_head *bh = NULL;
> Index: linux-2.6/fs/udf/udfdecl.h
> ===================================================================
> --- linux-2.6.orig/fs/udf/udfdecl.h
> +++ linux-2.6/fs/udf/udfdecl.h
> @@ -138,7 +138,7 @@ extern int udf_sync_inode(struct inode *
> extern void udf_expand_file_adinicb(struct inode *, int, int *);
> extern struct buffer_head *udf_expand_dir_adinicb(struct inode *, int *, int *);
> extern struct buffer_head *udf_bread(struct inode *, int, int, int *);
> -extern void udf_truncate(struct inode *);
> +extern int udf_setattr(struct dentry *, struct iattr *);
> extern void udf_read_inode(struct inode *);
> extern void udf_delete_inode(struct inode *);
> extern void udf_clear_inode(struct inode *);
> Index: linux-2.6/fs/udf/file.c
> ===================================================================
> --- linux-2.6.orig/fs/udf/file.c
> +++ linux-2.6/fs/udf/file.c
> @@ -215,5 +215,6 @@ const struct file_operations udf_file_op
> };
>
> const struct inode_operations udf_file_inode_operations = {
> - .truncate = udf_truncate,
> + .new_truncate = 1,
> + .setattr = udf_setattr,
> };
>
>
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 35+ messages in thread
* [patch] ext2: convert to use the new truncate convention fix
2009-08-21 14:06 ` Jan Kara
@ 2009-08-24 5:30 ` Nick Piggin
0 siblings, 0 replies; 35+ messages in thread
From: Nick Piggin @ 2009-08-24 5:30 UTC (permalink / raw)
To: Jan Kara; +Cc: Andrew Morton, linux-fsdevel, Christoph Hellwig, linux-ext4
On Fri, Aug 21, 2009 at 04:06:59PM +0200, Jan Kara wrote:
> > Hi,
> >
> > > I also have commented a possible bug in existing ext2 code, marked with XXX.
> > Looks good, except:
> >
> > > +int ext2_setsize(struct inode *inode, loff_t newsize)
> > This could be static.
> >
> > > @@ -1459,8 +1540,15 @@ int ext2_setattr(struct dentry *dentry,
> > > if (error)
> > > return error;
> > > }
> > > - error = inode_setattr(inode, iattr);
> > > + if (iattr->ia_valid & ATTR_SIZE) {
> > > + error = ext2_setsize(inode, iattr->ia_size);
> > > + if (error)
> > > + return error;
> > > + }
> > > + generic_setattr(inode, iattr);
> > Here, we should store the error code I suppose...
> Ah, I was confused. generic_setattr() returns void. But then remove
> the check !error from:
> if (!error && (iattr->ia_valid & ATTR_MODE))
> which just follows the generic_setattr(). That's what made me think
> generic_setattr() returns something :)
Yep, good suggestion. Andrew please add this incremental patch
---
fs/ext2/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6/fs/ext2/inode.c
===================================================================
--- linux-2.6.orig/fs/ext2/inode.c
+++ linux-2.6/fs/ext2/inode.c
@@ -1546,7 +1546,7 @@ int ext2_setattr(struct dentry *dentry,
return error;
}
generic_setattr(inode, iattr);
- if (!error && (iattr->ia_valid & ATTR_MODE))
+ if (iattr->ia_valid & ATTR_MODE)
error = ext2_acl_chmod(inode);
mark_inode_dirty(inode);
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 10/11] udf: convert to use the new truncate convention.
2009-08-21 14:22 ` Jan Kara
@ 2009-08-24 5:33 ` Nick Piggin
0 siblings, 0 replies; 35+ messages in thread
From: Nick Piggin @ 2009-08-24 5:33 UTC (permalink / raw)
To: Jan Kara; +Cc: Andrew Morton, linux-fsdevel, Christoph Hellwig
On Fri, Aug 21, 2009 at 04:22:31PM +0200, Jan Kara wrote:
> On Fri 21-08-09 02:35:14, npiggin@suse.de wrote:
> > Cc: Jan Kara <jack@suse.cz>
> > Cc: Christoph Hellwig <hch@lst.de>
> > Signed-off-by: Nick Piggin <npiggin@suse.de>
> The patch is fine.
> Acked-by: Jan Kara <jack@suse.cz>
>
> I won't pull the patch to my UDF tree for now since it depends on the
> generic patches so I guess it's better if Andrew carries all the patches
> in one bunch. Once we decide it's time to merge with Linus, feel free to
> either merge it directly with my Ack or via my tree.
Thanks. I'm not sure how Andrew will merge -- send the core patches to
either Linus or Al, at which point the fs patches can go to maintainers.
Hopefully we can get at least the core patches early in next merge window.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 01/11] fs: new truncate helpers
2009-08-20 16:35 ` [patch 01/11] fs: new truncate helpers npiggin
@ 2009-08-26 7:38 ` Artem Bityutskiy
2009-09-07 7:33 ` Nick Piggin
0 siblings, 1 reply; 35+ messages in thread
From: Artem Bityutskiy @ 2009-08-26 7:38 UTC (permalink / raw)
To: npiggin; +Cc: Andrew Morton, linux-fsdevel, Christoph Hellwig
Hi,
On 08/20/2009 07:35 PM, npiggin@suse.de wrote:
> +/**
> + * inode_newsize_ok - may this inode be truncated to a given size
> + * @inode: the inode to be truncated
> + * @offset: the new size to assign to the inode
> + * @Returns: 0 on success, -ve errno on failure
> + *
> + * inode_newsize_ok will check filesystem limits and ulimits to check that the
> + * new inode size is within limits. inode_newsize_ok will also send SIGXFSZ
> + * when necessary. Caller must not proceed with inode size change if failure is
> + * returned. @inode must be a file (not directory), with appropriate
> + * permissions to allow truncate (inode_newsize_ok does NOT check these
> + * conditions).
> + *
> + * inode_newsize_ok must be called with i_mutex held.
> + */
> +int inode_newsize_ok(struct inode *inode, loff_t offset)
> +{
Could you use 'const' for inode here?
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 03/11] fs: introduce new truncate sequence
2009-08-20 16:35 ` [patch 03/11] fs: introduce new truncate sequence npiggin
@ 2009-08-26 7:40 ` Artem Bityutskiy
0 siblings, 0 replies; 35+ messages in thread
From: Artem Bityutskiy @ 2009-08-26 7:40 UTC (permalink / raw)
To: npiggin; +Cc: Andrew Morton, linux-fsdevel, Christoph Hellwig
Hi,
just minor suggestions.
On 08/20/2009 07:35 PM, npiggin@suse.de wrote:
> +/**
> + * generic_setattr - copy simple metadata updates into the generic inode
> + * @inode: the inode to be updated
> + * @attr: the new attributes
> + *
> + * generic_setattr must be called with i_mutex held.
> + *
> + * generic_setattr updates the inode's metadata with that specified
> + * in attr. Noticably missing is inode size update, which is more complex
> + * as it requires pagecache updates. See simple_setsize.
> + *
> + * The inode is not marked as dirty after this operation. The rationale is
> + * that for "simple" filesystems, the struct inode is the inode storage.
> + * The caller is free to mark the inode dirty afterwards if needed.
> + */
> +void generic_setattr(struct inode *inode, struct iattr *attr)
> {
Could you use 'const' for attr here?
> +/*
> + * note this function is deprecated, the new truncate sequence should be
> + * used instead -- see eg. simple_setsize, generic_setattr.
> + */
> +int inode_setattr(struct inode *inode, struct iattr * attr)
> +{
And for 'attr' here, as well as do a tiny clean-up and us '*attr', not '* attr'.
:-)
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 01/11] fs: new truncate helpers
2009-08-26 7:38 ` Artem Bityutskiy
@ 2009-09-07 7:33 ` Nick Piggin
2009-09-07 7:48 ` Artem Bityutskiy
0 siblings, 1 reply; 35+ messages in thread
From: Nick Piggin @ 2009-09-07 7:33 UTC (permalink / raw)
To: Artem Bityutskiy; +Cc: Andrew Morton, linux-fsdevel, Christoph Hellwig
On Wed, Aug 26, 2009 at 10:38:11AM +0300, Artem Bityutskiy wrote:
> Hi,
>
> On 08/20/2009 07:35 PM, npiggin@suse.de wrote:
> >+/**
> >+ * inode_newsize_ok - may this inode be truncated to a given size
> >+ * @inode: the inode to be truncated
> >+ * @offset: the new size to assign to the inode
> >+ * @Returns: 0 on success, -ve errno on failure
> >+ *
> >+ * inode_newsize_ok will check filesystem limits and ulimits to check
> >that the
> >+ * new inode size is within limits. inode_newsize_ok will also send
> >SIGXFSZ
> >+ * when necessary. Caller must not proceed with inode size change if
> >failure is
> >+ * returned. @inode must be a file (not directory), with appropriate
> >+ * permissions to allow truncate (inode_newsize_ok does NOT check these
> >+ * conditions).
> >+ *
> >+ * inode_newsize_ok must be called with i_mutex held.
> >+ */
> >+int inode_newsize_ok(struct inode *inode, loff_t offset)
> >+{
>
> Could you use 'const' for inode here?
Sorry, I don't think I replied to you. Thanks for the suggestions, yes
const wouldn't hurt I guess. I will be sending a couple of incremental
updates to Andrew shortly so I'll include that.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 01/11] fs: new truncate helpers
2009-09-07 7:33 ` Nick Piggin
@ 2009-09-07 7:48 ` Artem Bityutskiy
0 siblings, 0 replies; 35+ messages in thread
From: Artem Bityutskiy @ 2009-09-07 7:48 UTC (permalink / raw)
To: Nick Piggin; +Cc: Andrew Morton, linux-fsdevel, Christoph Hellwig
On Mon, 2009-09-07 at 09:33 +0200, Nick Piggin wrote:
> > >+int inode_newsize_ok(struct inode *inode, loff_t offset)
> > >+{
> >
> > Could you use 'const' for inode here?
>
> Sorry, I don't think I replied to you. Thanks for the suggestions, yes
> const wouldn't hurt I guess. I will be sending a couple of incremental
> updates to Andrew shortly so I'll include that.
Thanks. I have done 'new_truncate' modifications for UBIFS and got gcc
warnings because my calling functions use 'const'.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 00/11] new truncate sequence
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
` (10 preceding siblings ...)
2009-08-20 16:35 ` [patch 11/11] minix: " npiggin
@ 2009-09-09 7:11 ` Artem Bityutskiy
2009-09-22 15:04 ` Al Viro
` (4 subsequent siblings)
16 siblings, 0 replies; 35+ messages in thread
From: Artem Bityutskiy @ 2009-09-09 7:11 UTC (permalink / raw)
To: npiggin; +Cc: Andrew Morton, linux-fsdevel, Christoph Hellwig
On Fri, 2009-08-21 at 02:35 +1000, npiggin@suse.de wrote:
> Hi,
>
> I have made a few small changes that were suggested, and converted
> a few more filesystems over. I didn't spend so much time on converting
> them this week so I think it should be best to just get it out now.
Just FYI,
I have UBIFS conversion ready as well and can merge it as soon
as your changes have gone in.
--
Best Regards,
Artem Bityutskiy (Артём Битюцкий)
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 00/11] new truncate sequence
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
` (11 preceding siblings ...)
2009-09-09 7:11 ` [patch 00/11] new truncate sequence Artem Bityutskiy
@ 2009-09-22 15:04 ` Al Viro
2009-09-22 20:00 ` Christoph Hellwig
2009-09-22 20:53 ` [PATCH 12/n] kill spurious reference to vmtruncate Christoph Hellwig
` (3 subsequent siblings)
16 siblings, 1 reply; 35+ messages in thread
From: Al Viro @ 2009-09-22 15:04 UTC (permalink / raw)
To: npiggin; +Cc: Andrew Morton, linux-fsdevel, Christoph Hellwig
On Fri, Aug 21, 2009 at 02:35:04AM +1000, npiggin@suse.de wrote:
> Hi,
>
> I have made a few small changes that were suggested, and converted
> a few more filesystems over. I didn't spend so much time on converting
> them this week so I think it should be best to just get it out now.
Umm... who calls jfs_setsize()?
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 00/11] new truncate sequence
2009-09-22 15:04 ` Al Viro
@ 2009-09-22 20:00 ` Christoph Hellwig
2009-09-22 21:51 ` Al Viro
0 siblings, 1 reply; 35+ messages in thread
From: Christoph Hellwig @ 2009-09-22 20:00 UTC (permalink / raw)
To: Al Viro; +Cc: npiggin, Andrew Morton, linux-fsdevel, Christoph Hellwig
On Tue, Sep 22, 2009 at 04:04:44PM +0100, Al Viro wrote:
> On Fri, Aug 21, 2009 at 02:35:04AM +1000, npiggin@suse.de wrote:
> > Hi,
> >
> > I have made a few small changes that were suggested, and converted
> > a few more filesystems over. I didn't spend so much time on converting
> > them this week so I think it should be best to just get it out now.
>
> Umm... who calls jfs_setsize()?
Yeah, noticed this while looking over the series. The JFS patch seems
to be botched.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 12/n] kill spurious reference to vmtruncate
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
` (12 preceding siblings ...)
2009-09-22 15:04 ` Al Viro
@ 2009-09-22 20:53 ` Christoph Hellwig
2009-09-22 20:54 ` [PATCH 13/n] xfs: new truncate sequence Christoph Hellwig
` (2 subsequent siblings)
16 siblings, 0 replies; 35+ messages in thread
From: Christoph Hellwig @ 2009-09-22 20:53 UTC (permalink / raw)
To: npiggin; +Cc: Andrew Morton, linux-fsdevel, Christoph Hellwig
Lots of filesystems calls vmtruncate despite not implementing the old
->truncate method. Switch them to use simple_setsize and add some
comments about the truncate code where it seems fitting.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: vfs-2.6.git/fs/ufs/truncate.c
===================================================================
--- vfs-2.6.git.orig/fs/ufs/truncate.c 2009-09-22 13:08:36.842264538 -0300
+++ vfs-2.6.git/fs/ufs/truncate.c 2009-09-22 13:12:13.926305949 -0300
@@ -500,12 +500,10 @@ out:
return err;
}
-
/*
- * We don't define our `inode->i_op->truncate', and call it here,
- * because of:
- * - there is no way to know old size
- * - there is no way inform user about error, if it happens in `truncate'
+ * TODO:
+ * - truncate case should use proper ordering instead of using
+ * simple_setsize
*/
static int ufs_setattr(struct dentry *dentry, struct iattr *attr)
{
@@ -520,7 +518,7 @@ static int ufs_setattr(struct dentry *de
if (ia_valid & ATTR_SIZE &&
attr->ia_size != i_size_read(inode)) {
loff_t old_i_size = inode->i_size;
- error = vmtruncate(inode, attr->ia_size);
+ error = simple_setsize(inode, attr->ia_size);
if (error)
return error;
error = ufs_truncate(inode, old_i_size);
Index: vfs-2.6.git/fs/adfs/inode.c
===================================================================
--- vfs-2.6.git.orig/fs/adfs/inode.c 2009-09-22 13:18:00.242265034 -0300
+++ vfs-2.6.git/fs/adfs/inode.c 2009-09-22 13:18:33.437781151 -0300
@@ -328,8 +328,9 @@ adfs_notify_change(struct dentry *dentry
if (error)
goto out;
+ /* XXX: this is missing some actual on-disk truncation.. */
if (ia_valid & ATTR_SIZE)
- error = vmtruncate(inode, attr->ia_size);
+ error = simple_setsize(inode, attr->ia_size);
if (error)
goto out;
Index: vfs-2.6.git/fs/ecryptfs/inode.c
===================================================================
--- vfs-2.6.git.orig/fs/ecryptfs/inode.c 2009-09-22 13:20:16.726261535 -0300
+++ vfs-2.6.git/fs/ecryptfs/inode.c 2009-09-22 13:21:30.893762356 -0300
@@ -831,9 +831,15 @@ int ecryptfs_truncate(struct dentry *den
- (new_length & ~PAGE_CACHE_MASK));
if (!(crypt_stat->flags & ECRYPTFS_ENCRYPTED)) {
- rc = vmtruncate(inode, new_length);
+ rc = simple_setsize(inode, new_length);
if (rc)
goto out_free;
+
+ /*
+ * XXX: this is horribly buggy as we can't and never
+ * could rely on the lower inode having a ->truncate
+ * method to actually release blocks.
+ */
rc = vmtruncate(lower_dentry->d_inode, new_length);
goto out_free;
}
@@ -855,7 +861,7 @@ int ecryptfs_truncate(struct dentry *den
goto out_free;
}
}
- vmtruncate(inode, new_length);
+ simple_setsize(inode, new_length);
rc = ecryptfs_write_inode_size_to_metadata(inode);
if (rc) {
printk(KERN_ERR "Problem with "
@@ -870,6 +876,11 @@ int ecryptfs_truncate(struct dentry *den
lower_size_after_truncate =
upper_size_to_lower_size(crypt_stat, new_length);
if (lower_size_after_truncate < lower_size_before_truncate)
+ /*
+ * XXX: this is horribly buggy as we can't and never
+ * could rely on the lower inode having a ->truncate
+ * method to actually release blocks.
+ */
vmtruncate(lower_dentry->d_inode,
lower_size_after_truncate);
}
Index: vfs-2.6.git/fs/gfs2/aops.c
===================================================================
--- vfs-2.6.git.orig/fs/gfs2/aops.c 2009-09-22 13:15:40.718262195 -0300
+++ vfs-2.6.git/fs/gfs2/aops.c 2009-09-22 13:16:45.265763898 -0300
@@ -710,8 +710,14 @@ out:
return 0;
page_cache_release(page);
+
+ /*
+ * XXX(hch): the call below should probably be replaced with
+ * a call to the gfs2-specific truncate blocks helper to actually
+ * release disk blocks..
+ */
if (pos + len > ip->i_inode.i_size)
- vmtruncate(&ip->i_inode, ip->i_inode.i_size);
+ simple_setsize(&ip->i_inode, ip->i_inode.i_size);
out_endtrans:
gfs2_trans_end(sdp);
out_trans_fail:
Index: vfs-2.6.git/fs/gfs2/ops_inode.c
===================================================================
--- vfs-2.6.git.orig/fs/gfs2/ops_inode.c 2009-09-22 13:16:48.901764352 -0300
+++ vfs-2.6.git/fs/gfs2/ops_inode.c 2009-09-22 13:17:30.256367942 -0300
@@ -1129,6 +1129,9 @@ int gfs2_permission(struct inode *inode,
return error;
}
+/*
+ * XXX: should be changed to have proper ordering by opencoding simple_setsize
+ */
static int setattr_size(struct inode *inode, struct iattr *attr)
{
struct gfs2_inode *ip = GFS2_I(inode);
@@ -1139,7 +1142,7 @@ static int setattr_size(struct inode *in
error = gfs2_trans_begin(sdp, 0, sdp->sd_jdesc->jd_blocks);
if (error)
return error;
- error = vmtruncate(inode, attr->ia_size);
+ error = simple_setsize(inode, attr->ia_size);
gfs2_trans_end(sdp);
if (error)
return error;
Index: vfs-2.6.git/fs/jffs2/fs.c
===================================================================
--- vfs-2.6.git.orig/fs/jffs2/fs.c 2009-09-22 13:18:47.962264848 -0300
+++ vfs-2.6.git/fs/jffs2/fs.c 2009-09-22 13:21:42.981761532 -0300
@@ -169,13 +169,13 @@ int jffs2_do_setattr (struct inode *inod
mutex_unlock(&f->sem);
jffs2_complete_reservation(c);
- /* We have to do the vmtruncate() without f->sem held, since
+ /* We have to do the simple_setsize() without f->sem held, since
some pages may be locked and waiting for it in readpage().
We are protected from a simultaneous write() extending i_size
back past iattr->ia_size, because do_truncate() holds the
generic inode semaphore. */
if (ivalid & ATTR_SIZE && inode->i_size > iattr->ia_size) {
- vmtruncate(inode, iattr->ia_size);
+ simple_setsize(inode, iattr->ia_size);
inode->i_blocks = (inode->i_size + 511) >> 9;
}
Index: vfs-2.6.git/fs/ocfs2/file.c
===================================================================
--- vfs-2.6.git.orig/fs/ocfs2/file.c 2009-09-22 13:14:22.849791263 -0300
+++ vfs-2.6.git/fs/ocfs2/file.c 2009-09-22 13:15:26.746262469 -0300
@@ -1021,7 +1021,7 @@ int ocfs2_setattr(struct dentry *dentry,
}
/*
- * This will intentionally not wind up calling vmtruncate(),
+ * This will intentionally not wind up calling simple_setsize(),
* since all the work for a size change has been done above.
* Otherwise, we could get into problems with truncate as
* ip_alloc_sem is used there to protect against i_size
@@ -1864,9 +1864,13 @@ relock:
* direct write may have instantiated a few
* blocks outside i_size. Trim these off again.
* Don't need i_size_read because we hold i_mutex.
+ *
+ * XXX(hch): this looks buggy because ocfs2 did not
+ * actually implement ->truncate. Take a look at
+ * the new truncate sequence and update this accordingly
*/
if (*ppos + count > inode->i_size)
- vmtruncate(inode, inode->i_size);
+ simple_setsize(inode, inode->i_size);
ret = written;
goto out_dio;
}
Index: vfs-2.6.git/fs/smbfs/inode.c
===================================================================
--- vfs-2.6.git.orig/fs/smbfs/inode.c 2009-09-22 13:12:18.913764201 -0300
+++ vfs-2.6.git/fs/smbfs/inode.c 2009-09-22 13:12:48.649797002 -0300
@@ -712,7 +712,7 @@ smb_notify_change(struct dentry *dentry,
error = server->ops->truncate(inode, attr->ia_size);
if (error)
goto out;
- error = vmtruncate(inode, attr->ia_size);
+ error = simple_setsize(inode, attr->ia_size);
if (error)
goto out;
refresh = 1;
Index: vfs-2.6.git/fs/ubifs/file.c
===================================================================
--- vfs-2.6.git.orig/fs/ubifs/file.c 2009-09-22 13:12:56.969768178 -0300
+++ vfs-2.6.git/fs/ubifs/file.c 2009-09-22 13:13:59.161870394 -0300
@@ -966,12 +966,15 @@ static int do_writepage(struct page *pag
* the page locked, and it locks @ui_mutex. However, write-back does take inode
* @i_mutex, which means other VFS operations may be run on this inode at the
* same time. And the problematic one is truncation to smaller size, from where
- * we have to call 'vmtruncate()', which first changes @inode->i_size, then
+ * we have to call 'simple_setsize()', which first changes @inode->i_size, then
* drops the truncated pages. And while dropping the pages, it takes the page
- * lock. This means that 'do_truncation()' cannot call 'vmtruncate()' with
+ * lock. This means that 'do_truncation()' cannot call 'simple_setsize()' with
* @ui_mutex locked, because it would deadlock with 'ubifs_writepage()'. This
* means that @inode->i_size is changed while @ui_mutex is unlocked.
*
+ * XXX: with the new truncate the above is not true anymore, the simple_setsize
+ * calls can be replaced with the individual components.
+ *
* But in 'ubifs_writepage()' we have to guarantee that we do not write beyond
* inode size. How do we do this if @inode->i_size may became smaller while we
* are in the middle of 'ubifs_writepage()'? The UBIFS solution is the
@@ -1124,7 +1127,7 @@ static int do_truncation(struct ubifs_in
budgeted = 0;
}
- err = vmtruncate(inode, new_size);
+ err = simple_setsize(inode, new_size);
if (err)
goto out_budg;
@@ -1213,7 +1216,7 @@ static int do_setattr(struct ubifs_info
if (attr->ia_valid & ATTR_SIZE) {
dbg_gen("size %lld -> %lld", inode->i_size, new_size);
- err = vmtruncate(inode, new_size);
+ err = simple_setsize(inode, new_size);
if (err)
goto out;
}
@@ -1222,7 +1225,7 @@ static int do_setattr(struct ubifs_info
if (attr->ia_valid & ATTR_SIZE) {
/* Truncation changes inode [mc]time */
inode->i_mtime = inode->i_ctime = ubifs_current_time(inode);
- /* 'vmtruncate()' changed @i_size, update @ui_size */
+ /* 'simple_setsize()' changed @i_size, update @ui_size */
ui->ui_size = inode->i_size;
}
Index: vfs-2.6.git/fs/ubifs/ubifs.h
===================================================================
--- vfs-2.6.git.orig/fs/ubifs/ubifs.h 2009-09-22 13:14:03.945800885 -0300
+++ vfs-2.6.git/fs/ubifs/ubifs.h 2009-09-22 13:14:13.014300584 -0300
@@ -378,7 +378,7 @@ struct ubifs_gced_idx_leb {
* The @ui_size is a "shadow" variable for @inode->i_size and UBIFS uses
* @ui_size instead of @inode->i_size. The reason for this is that UBIFS cannot
* make sure @inode->i_size is always changed under @ui_mutex, because it
- * cannot call 'vmtruncate()' with @ui_mutex locked, because it would deadlock
+ * cannot call 'simple_setsize()' with @ui_mutex locked, because it would deadlock
* with 'ubifs_writepage()' (see file.c). All the other inode fields are
* changed under @ui_mutex, so they do not need "shadow" fields. Note, one
* could consider to rework locking and base it on "shadow" fields.
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 13/n] xfs: new truncate sequence
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
` (13 preceding siblings ...)
2009-09-22 20:53 ` [PATCH 12/n] kill spurious reference to vmtruncate Christoph Hellwig
@ 2009-09-22 20:54 ` Christoph Hellwig
2009-09-22 20:55 ` [PATCH 14/n] sysv: " Christoph Hellwig
2009-09-22 20:56 ` [PATCH 15/n] ntfs: " Christoph Hellwig
16 siblings, 0 replies; 35+ messages in thread
From: Christoph Hellwig @ 2009-09-22 20:54 UTC (permalink / raw)
To: npiggin; +Cc: Andrew Morton, linux-fsdevel, xfs
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: vfs-2.6.git/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- vfs-2.6.git.orig/fs/xfs/linux-2.6/xfs_aops.c 2009-09-22 11:28:48.581004129 -0300
+++ vfs-2.6.git/fs/xfs/linux-2.6/xfs_aops.c 2009-09-22 12:11:08.625765641 -0300
@@ -1563,6 +1563,45 @@ xfs_vm_direct_IO(
return ret;
}
+STATIC void
+xfs_truncate_excess_blocks(
+ struct address_space *mapping,
+ loff_t pos,
+ unsigned len)
+{
+ struct inode *inode = mapping->host;
+ loff_t isize = inode->i_size;
+
+ if (pos + len > isize) {
+ struct iattr ia;
+ int error;
+
+ ia.ia_valid = ATTR_SIZE | ATTR_FORCE,
+ ia.ia_size = isize,
+
+ error = xfs_setattr(XFS_I(inode), &ia, XFS_ATTR_NOLOCK);
+ WARN_ON(error); /* not much we can do here. */
+ }
+}
+
+STATIC int
+xfs_vm_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)
+ xfs_truncate_excess_blocks(mapping, pos, len);
+ return ret;
+}
+
STATIC int
xfs_vm_write_begin(
struct file *file,
@@ -1573,9 +1612,14 @@ xfs_vm_write_begin(
struct page **pagep,
void **fsdata)
{
+ int ret;
+
*pagep = NULL;
- return block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
+ ret = block_write_begin(file, mapping, pos, len, flags, pagep, fsdata,
xfs_get_blocks);
+ if (ret < 0)
+ xfs_truncate_excess_blocks(mapping, pos, len);
+ return ret;
}
STATIC sector_t
@@ -1630,7 +1674,7 @@ const struct address_space_operations xf
.releasepage = xfs_vm_releasepage,
.invalidatepage = xfs_vm_invalidatepage,
.write_begin = xfs_vm_write_begin,
- .write_end = generic_write_end,
+ .write_end = xfs_vm_write_end,
.bmap = xfs_vm_bmap,
.direct_IO = xfs_vm_direct_IO,
.migratepage = buffer_migrate_page,
Index: vfs-2.6.git/fs/xfs/linux-2.6/xfs_iops.c
===================================================================
--- vfs-2.6.git.orig/fs/xfs/linux-2.6/xfs_iops.c 2009-09-22 11:28:48.585004290 -0300
+++ vfs-2.6.git/fs/xfs/linux-2.6/xfs_iops.c 2009-09-22 12:11:47.725937790 -0300
@@ -547,21 +547,6 @@ xfs_vn_setattr(
return -xfs_setattr(XFS_I(dentry->d_inode), iattr, 0);
}
-/*
- * block_truncate_page can return an error, but we can't propagate it
- * at all here. Leave a complaint + stack trace in the syslog because
- * this could be bad. If it is bad, we need to propagate the error further.
- */
-STATIC void
-xfs_vn_truncate(
- struct inode *inode)
-{
- int error;
- error = block_truncate_page(inode->i_mapping, inode->i_size,
- xfs_get_blocks);
- WARN_ON(error);
-}
-
STATIC long
xfs_vn_fallocate(
struct inode *inode,
@@ -688,7 +673,7 @@ xfs_vn_fiemap(
static const struct inode_operations xfs_inode_operations = {
.check_acl = xfs_check_acl,
- .truncate = xfs_vn_truncate,
+ .truncate_kludge_to_be_killed = 1,
.getattr = xfs_vn_getattr,
.setattr = xfs_vn_setattr,
.setxattr = generic_setxattr,
Index: vfs-2.6.git/fs/xfs/linux-2.6/xfs_linux.h
===================================================================
--- vfs-2.6.git.orig/fs/xfs/linux-2.6/xfs_linux.h 2009-09-22 11:28:48.585004290 -0300
+++ vfs-2.6.git/fs/xfs/linux-2.6/xfs_linux.h 2009-09-22 12:11:08.637764444 -0300
@@ -159,9 +159,6 @@
*/
#define xfs_sort(a,n,s,fn) sort(a,n,s,fn,NULL)
#define xfs_stack_trace() dump_stack()
-#define xfs_itruncate_data(ip, off) \
- (-vmtruncate(VFS_I(ip), (off)))
-
/* Move the kernel do_div definition off to one side */
Index: vfs-2.6.git/fs/xfs/xfs_vnodeops.c
===================================================================
--- vfs-2.6.git.orig/fs/xfs/xfs_vnodeops.c 2009-09-22 12:10:45.977765610 -0300
+++ vfs-2.6.git/fs/xfs/xfs_vnodeops.c 2009-09-22 12:12:54.586307127 -0300
@@ -197,9 +197,11 @@ xfs_setattr(
* Truncate file. Must have write permission and not be a directory.
*/
if (mask & ATTR_SIZE) {
+ loff_t oldsize = ip->i_size;
+ loff_t newsize = iattr->ia_size;
+
/* Short circuit the truncate case for zero length files */
- if (iattr->ia_size == 0 &&
- ip->i_size == 0 && ip->i_d.di_nextents == 0) {
+ if (newsize == 0 && oldsize == 0 && ip->i_d.di_nextents == 0) {
xfs_iunlock(ip, XFS_ILOCK_EXCL);
lock_flags &= ~XFS_ILOCK_EXCL;
if (mask & ATTR_CTIME)
@@ -231,16 +233,19 @@ xfs_setattr(
* to the transaction, because the inode cannot be unlocked
* once it is a part of the transaction.
*/
- if (iattr->ia_size > ip->i_size) {
+ if (newsize > oldsize) {
/*
* Do the first part of growing a file: zero any data
* in the last block that is beyond the old EOF. We
* need to do this before the inode is joined to the
* transaction to modify the i_size.
*/
- code = xfs_zero_eof(ip, iattr->ia_size, ip->i_size);
+ code = xfs_zero_eof(ip, newsize, oldsize);
+ if (code)
+ goto error_return;
}
xfs_iunlock(ip, XFS_ILOCK_EXCL);
+ lock_flags &= ~XFS_ILOCK_EXCL;
/*
* We are going to log the inode size change in this
@@ -254,25 +259,28 @@ xfs_setattr(
* really care about here and prevents waiting for other data
* not within the range we care about here.
*/
- if (!code &&
- ip->i_size != ip->i_d.di_size &&
- iattr->ia_size > ip->i_d.di_size) {
- code = xfs_flush_pages(ip,
- ip->i_d.di_size, iattr->ia_size,
- XFS_B_ASYNC, FI_NONE);
+ if (oldsize != ip->i_d.di_size && newsize > ip->i_d.di_size) {
+ code = xfs_flush_pages(ip, ip->i_d.di_size, newsize,
+ XFS_B_ASYNC, FI_NONE);
+ if (code)
+ goto error_return;
}
/* wait for all I/O to complete */
xfs_ioend_wait(ip);
- if (!code)
- code = xfs_itruncate_data(ip, iattr->ia_size);
- if (code) {
- ASSERT(tp == NULL);
- lock_flags &= ~XFS_ILOCK_EXCL;
- ASSERT(lock_flags == XFS_IOLOCK_EXCL);
+ code = -inode_newsize_ok(inode, newsize);
+ if (code)
goto error_return;
- }
+
+ code = -block_truncate_page(inode->i_mapping, newsize,
+ xfs_get_blocks);
+ if (code)
+ goto error_return;
+
+ i_size_write(inode, newsize);
+ truncate_pagecache(inode, oldsize, newsize);
+
tp = xfs_trans_alloc(mp, XFS_TRANS_SETATTR_SIZE);
if ((code = xfs_trans_reserve(tp, 0,
XFS_ITRUNCATE_LOG_RES(mp), 0,
@@ -284,6 +292,7 @@ xfs_setattr(
return code;
}
commit_flags = XFS_TRANS_RELEASE_LOG_RES;
+ lock_flags |= XFS_ILOCK_EXCL;
xfs_ilock(ip, XFS_ILOCK_EXCL);
xfs_trans_ijoin(tp, ip, lock_flags);
@@ -295,23 +304,23 @@ xfs_setattr(
* the semantic difference between truncate() and ftruncate()
* as implemented in the VFS.
*/
- if (iattr->ia_size != ip->i_size || (mask & ATTR_CTIME))
+ if (newsize != ip->i_size || (mask & ATTR_CTIME))
timeflags |= XFS_ICHGTIME_MOD | XFS_ICHGTIME_CHG;
- if (iattr->ia_size > ip->i_size) {
- ip->i_d.di_size = iattr->ia_size;
- ip->i_size = iattr->ia_size;
+ if (newsize > ip->i_size) {
+ ip->i_d.di_size = newsize;
+ ip->i_size = newsize;
if (!(flags & XFS_ATTR_DMI))
xfs_ichgtime(ip, XFS_ICHGTIME_CHG);
xfs_trans_log_inode(tp, ip, XFS_ILOG_CORE);
- } else if (iattr->ia_size <= ip->i_size ||
- (iattr->ia_size == 0 && ip->i_d.di_nextents)) {
+ } else if (newsize <= ip->i_size ||
+ (newsize == 0 && ip->i_d.di_nextents)) {
/*
* signal a sync transaction unless
* we're truncating an already unlinked
* file on a wsync filesystem
*/
- code = xfs_itruncate_finish(&tp, ip, iattr->ia_size,
+ code = xfs_itruncate_finish(&tp, ip, newsize,
XFS_DATA_FORK,
((ip->i_d.di_nlink != 0 ||
!(mp->m_flags & XFS_MOUNT_WSYNC))
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 14/n] sysv: new truncate sequence
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
` (14 preceding siblings ...)
2009-09-22 20:54 ` [PATCH 13/n] xfs: new truncate sequence Christoph Hellwig
@ 2009-09-22 20:55 ` Christoph Hellwig
2009-09-22 20:56 ` [PATCH 15/n] ntfs: " Christoph Hellwig
16 siblings, 0 replies; 35+ messages in thread
From: Christoph Hellwig @ 2009-09-22 20:55 UTC (permalink / raw)
To: npiggin; +Cc: Andrew Morton, linux-fsdevel, Christoph Hellwig
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: vfs-2.6.git/fs/sysv/file.c
===================================================================
--- vfs-2.6.git.orig/fs/sysv/file.c 2009-09-22 15:23:55.049764156 -0300
+++ vfs-2.6.git/fs/sysv/file.c 2009-09-22 15:35:52.993764871 -0300
@@ -31,6 +31,7 @@ const struct file_operations sysv_file_o
};
const struct inode_operations sysv_file_inode_operations = {
- .truncate = sysv_truncate,
+ .truncate_kludge_to_be_killed = 1,
+ .setattr = sysv_setattr,
.getattr = sysv_getattr,
};
Index: vfs-2.6.git/fs/sysv/inode.c
===================================================================
--- vfs-2.6.git.orig/fs/sysv/inode.c 2009-09-22 15:23:55.077764435 -0300
+++ vfs-2.6.git/fs/sysv/inode.c 2009-09-22 15:31:45.210264810 -0300
@@ -305,7 +305,8 @@ static void sysv_delete_inode(struct ino
{
truncate_inode_pages(&inode->i_data, 0);
inode->i_size = 0;
- sysv_truncate(inode);
+ if (S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) || S_ISLNK(inode->i_mode))
+ sysv_truncate_blocks(inode, 0);
sysv_free_inode(inode);
}
Index: vfs-2.6.git/fs/sysv/itree.c
===================================================================
--- vfs-2.6.git.orig/fs/sysv/itree.c 2009-09-22 15:23:55.121765981 -0300
+++ vfs-2.6.git/fs/sysv/itree.c 2009-09-22 15:39:17.898295684 -0300
@@ -360,7 +360,7 @@ static void free_branches(struct inode *
free_data(inode, p, q);
}
-void sysv_truncate (struct inode * inode)
+void sysv_truncate_blocks(struct inode *inode, loff_t offset)
{
sysv_zone_t *i_data = SYSV_I(inode)->i_data;
int offsets[DEPTH];
@@ -371,15 +371,8 @@ void sysv_truncate (struct inode * inode
long iblock;
unsigned blocksize;
- if (!(S_ISREG(inode->i_mode) || S_ISDIR(inode->i_mode) ||
- S_ISLNK(inode->i_mode)))
- return;
-
blocksize = inode->i_sb->s_blocksize;
- iblock = (inode->i_size + blocksize-1)
- >> inode->i_sb->s_blocksize_bits;
-
- block_truncate_page(inode->i_mapping, inode->i_size, get_block);
+ iblock = (offset + blocksize - 1) >> inode->i_sb->s_blocksize_bits;
n = block_to_path(inode, iblock, offsets);
if (n == 0)
@@ -449,6 +442,47 @@ int sysv_getattr(struct vfsmount *mnt, s
return 0;
}
+static int sysv_setsize(struct inode *inode, loff_t newsize)
+{
+ loff_t oldsize;
+ int error;
+
+ error = inode_newsize_ok(inode, newsize);
+ if (error)
+ return error;
+
+ oldsize = inode->i_size;
+ i_size_write(inode, newsize);
+ truncate_pagecache(inode, oldsize, newsize);
+
+ error = block_truncate_page(inode->i_mapping, newsize, get_block);
+ if (error)
+ return error;
+
+ sysv_truncate_blocks(inode, newsize);
+ return 0;
+}
+
+int sysv_setattr(struct dentry *dentry, struct iattr *iattr)
+{
+ struct inode *inode = dentry->d_inode;
+ int error;
+
+ error = inode_change_ok(inode, iattr);
+ if (error)
+ return error;
+
+ if (iattr->ia_valid & ATTR_SIZE) {
+ error = sysv_setsize(inode, iattr->ia_size);
+ if (error)
+ return error;
+ }
+
+ generic_setattr(inode, iattr);
+ mark_inode_dirty(inode);
+ return 0;
+}
+
static int sysv_writepage(struct page *page, struct writeback_control *wbc)
{
return block_write_full_page(page,get_block,wbc);
@@ -471,8 +505,33 @@ static int sysv_write_begin(struct file
loff_t pos, unsigned len, unsigned flags,
struct page **pagep, void **fsdata)
{
+ int ret;
+
*pagep = NULL;
- return __sysv_write_begin(file, mapping, pos, len, flags, pagep, fsdata);
+ ret = __sysv_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)
+ sysv_truncate_blocks(inode, isize);
+ }
+ return ret;
+}
+
+static int sysv_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)
+ sysv_truncate_blocks(inode, isize);
+ }
+ return ret;
}
static sector_t sysv_bmap(struct address_space *mapping, sector_t block)
@@ -485,6 +544,6 @@ const struct address_space_operations sy
.writepage = sysv_writepage,
.sync_page = block_sync_page,
.write_begin = sysv_write_begin,
- .write_end = generic_write_end,
+ .write_end = sysv_write_end,
.bmap = sysv_bmap
};
Index: vfs-2.6.git/fs/sysv/sysv.h
===================================================================
--- vfs-2.6.git.orig/fs/sysv/sysv.h 2009-09-22 15:23:55.097764604 -0300
+++ vfs-2.6.git/fs/sysv/sysv.h 2009-09-22 15:32:51.722266473 -0300
@@ -135,7 +135,7 @@ extern void sysv_free_block(struct super
extern unsigned long sysv_count_free_blocks(struct super_block *);
/* itree.c */
-extern void sysv_truncate(struct inode *);
+extern void sysv_truncate_blocks(struct inode *, loff_t);
extern int __sysv_write_begin(struct file *file, struct address_space *mapping,
loff_t pos, unsigned len, unsigned flags,
struct page **pagep, void **fsdata);
@@ -146,6 +146,7 @@ extern int sysv_write_inode(struct inode
extern int sysv_sync_inode(struct inode *);
extern void sysv_set_inode(struct inode *, dev_t);
extern int sysv_getattr(struct vfsmount *, struct dentry *, struct kstat *);
+extern int sysv_setattr(struct dentry *dentry, struct iattr *iattr);
extern int sysv_init_icache(void);
extern void sysv_destroy_icache(void);
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 15/n] ntfs: new truncate sequence
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
` (15 preceding siblings ...)
2009-09-22 20:55 ` [PATCH 14/n] sysv: " Christoph Hellwig
@ 2009-09-22 20:56 ` Christoph Hellwig
16 siblings, 0 replies; 35+ messages in thread
From: Christoph Hellwig @ 2009-09-22 20:56 UTC (permalink / raw)
To: npiggin; +Cc: Andrew Morton, linux-fsdevel, Christoph Hellwig
The ntfs code isa a bit weird in it's truncate code, so just convert it
to use simple_setsize and leave the rest to someone who knows it better.
Signed-off-by: Christoph Hellwig <hch@lst.de>
Index: vfs-2.6.git/fs/ntfs/aops.c
===================================================================
--- vfs-2.6.git.orig/fs/ntfs/aops.c 2009-09-22 15:42:33.666298178 -0300
+++ vfs-2.6.git/fs/ntfs/aops.c 2009-09-22 15:42:44.701761705 -0300
@@ -642,7 +642,7 @@ static int ntfs_write_block(struct page
* FIXME: What about the small race window where
* ntfs_writepage() has not done any clearing because
* the page was within i_size but before we get here,
- * vmtruncate() modifies i_size?
+ * truncate modifies i_size?
*/
clear_buffer_dirty(bh);
set_buffer_uptodate(bh);
Index: vfs-2.6.git/fs/ntfs/file.c
===================================================================
--- vfs-2.6.git.orig/fs/ntfs/file.c 2009-09-22 15:41:58.526264375 -0300
+++ vfs-2.6.git/fs/ntfs/file.c 2009-09-22 15:46:05.890266938 -0300
@@ -2039,7 +2039,7 @@ static ssize_t ntfs_file_buffered_write(
*/
i_size = i_size_read(vi);
if (pos + bytes > i_size)
- vmtruncate(vi, i_size);
+ ntfs_truncate(inode);
break;
}
}
@@ -2278,7 +2278,7 @@ const struct file_operations ntfs_file_o
const struct inode_operations ntfs_file_inode_ops = {
#ifdef NTFS_RW
- .truncate = ntfs_truncate_vfs,
+ .truncate_kludge_to_be_killed = 1,
.setattr = ntfs_setattr,
#endif /* NTFS_RW */
};
Index: vfs-2.6.git/fs/ntfs/inode.c
===================================================================
--- vfs-2.6.git.orig/fs/ntfs/inode.c 2009-09-22 15:39:44.429764137 -0300
+++ vfs-2.6.git/fs/ntfs/inode.c 2009-09-22 15:47:54.481767325 -0300
@@ -2348,11 +2348,7 @@ static const char *es = " Leaving incon
* Returns 0 on success or -errno on error.
*
* Called with ->i_mutex held. In all but one case ->i_alloc_sem is held for
- * writing. The only case in the kernel where ->i_alloc_sem is not held is
- * mm/filemap.c::generic_file_buffered_write() where vmtruncate() is called
- * with the current i_size as the offset. The analogous place in NTFS is in
- * fs/ntfs/file.c::ntfs_file_buffered_write() where we call vmtruncate() again
- * without holding ->i_alloc_sem.
+ * writing. That case is in fs/ntfs/file.c::ntfs_file_buffered_write().
*/
int ntfs_truncate(struct inode *vi)
{
@@ -2506,7 +2502,7 @@ retry_truncate:
/*
* Note ntfs_resident_attr_value_resize() has already done any
* necessary data clearing in the attribute record. When the
- * file is being shrunk vmtruncate() will already have cleared
+ * file is being shrunk truncate will already have cleared
* the top part of the last partial page, i.e. since this is
* the resident case this is the page with index 0. However,
* when the file is being expanded, the page cache page data
@@ -2854,18 +2850,6 @@ conv_err_out:
}
/**
- * ntfs_truncate_vfs - wrapper for ntfs_truncate() that has no return value
- * @vi: inode for which the i_size was changed
- *
- * Wrapper for ntfs_truncate() that has no return value.
- *
- * See ntfs_truncate() description above for details.
- */
-void ntfs_truncate_vfs(struct inode *vi) {
- ntfs_truncate(vi);
-}
-
-/**
* ntfs_setattr - called from notify_change() when an attribute is being changed
* @dentry: dentry whose attributes to change
* @attr: structure describing the attributes and the changes
@@ -2913,8 +2897,13 @@ int ntfs_setattr(struct dentry *dentry,
NInoCompressed(ni) ?
"compressed" : "encrypted");
err = -EOPNOTSUPP;
- } else
- err = vmtruncate(vi, attr->ia_size);
+ goto out;
+ }
+
+ err = simple_setsize(inode, attr->ia_size);
+ if (err)
+ goto out;
+ err = ntfs_truncate(inode);
if (err || ia_valid == ATTR_SIZE)
goto out;
} else {
Index: vfs-2.6.git/fs/ntfs/inode.h
===================================================================
--- vfs-2.6.git.orig/fs/ntfs/inode.h 2009-09-22 15:43:39.293762634 -0300
+++ vfs-2.6.git/fs/ntfs/inode.h 2009-09-22 15:43:48.061766513 -0300
@@ -303,7 +303,6 @@ extern int ntfs_show_options(struct seq_
#ifdef NTFS_RW
extern int ntfs_truncate(struct inode *vi);
-extern void ntfs_truncate_vfs(struct inode *vi);
extern int ntfs_setattr(struct dentry *dentry, struct iattr *attr);
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 00/11] new truncate sequence
2009-09-22 20:00 ` Christoph Hellwig
@ 2009-09-22 21:51 ` Al Viro
2009-09-22 23:27 ` Al Viro
0 siblings, 1 reply; 35+ messages in thread
From: Al Viro @ 2009-09-22 21:51 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: npiggin, Andrew Morton, linux-fsdevel
On Tue, Sep 22, 2009 at 10:00:42PM +0200, Christoph Hellwig wrote:
> On Tue, Sep 22, 2009 at 04:04:44PM +0100, Al Viro wrote:
> > On Fri, Aug 21, 2009 at 02:35:04AM +1000, npiggin@suse.de wrote:
> > > Hi,
> > >
> > > I have made a few small changes that were suggested, and converted
> > > a few more filesystems over. I didn't spend so much time on converting
> > > them this week so I think it should be best to just get it out now.
> >
> > Umm... who calls jfs_setsize()?
>
> Yeah, noticed this while looking over the series. The JFS patch seems
> to be botched.
It is... I really don't like some parts of that:
* failing on foo_truncate() is all nice and proper, but
what to do about the quota transfer that has already happened?
Note that e.g. ext2_setattr() simply doesn't change uid/gid in
case of ext2_setsize() failure. It does transfer quota, though.
And yes, nfsd can produce calls with both ATTR_SIZE and ATTR_UID
set.
* way, _way_ too much boilerplate around the aforementioned
quota transfers.
* if we don't do quota handling for minix and sysv (fair enough,
no quota support implemented for those), why the devil are we doing it
in simple_setattr()?
Anyway, #new-truncate in the usual VFS tree contains all "new truncate
scheme" stuff I've collected from the lists so far. Additions are welcome...
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 00/11] new truncate sequence
2009-09-22 21:51 ` Al Viro
@ 2009-09-22 23:27 ` Al Viro
2009-09-22 23:58 ` Al Viro
0 siblings, 1 reply; 35+ messages in thread
From: Al Viro @ 2009-09-22 23:27 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: npiggin, Andrew Morton, linux-fsdevel
On Tue, Sep 22, 2009 at 10:51:38PM +0100, Al Viro wrote:
> It is... I really don't like some parts of that:
> * failing on foo_truncate() is all nice and proper, but
> what to do about the quota transfer that has already happened?
> Note that e.g. ext2_setattr() simply doesn't change uid/gid in
> case of ext2_setsize() failure. It does transfer quota, though.
> And yes, nfsd can produce calls with both ATTR_SIZE and ATTR_UID
> set.
> * way, _way_ too much boilerplate around the aforementioned
> quota transfers.
> * if we don't do quota handling for minix and sysv (fair enough,
> no quota support implemented for those), why the devil are we doing it
> in simple_setattr()?
>
> Anyway, #new-truncate in the usual VFS tree contains all "new truncate
> scheme" stuff I've collected from the lists so far. Additions are welcome...
BTW, why are we messing with truncation in ->write_end() e.g. for ext2?
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 00/11] new truncate sequence
2009-09-22 23:27 ` Al Viro
@ 2009-09-22 23:58 ` Al Viro
2009-09-23 2:29 ` Al Viro
0 siblings, 1 reply; 35+ messages in thread
From: Al Viro @ 2009-09-22 23:58 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: npiggin, Andrew Morton, linux-fsdevel
On Wed, Sep 23, 2009 at 12:27:42AM +0100, Al Viro wrote:
> On Tue, Sep 22, 2009 at 10:51:38PM +0100, Al Viro wrote:
> > It is... I really don't like some parts of that:
> > * failing on foo_truncate() is all nice and proper, but
> > what to do about the quota transfer that has already happened?
> > Note that e.g. ext2_setattr() simply doesn't change uid/gid in
> > case of ext2_setsize() failure. It does transfer quota, though.
> > And yes, nfsd can produce calls with both ATTR_SIZE and ATTR_UID
> > set.
> > * way, _way_ too much boilerplate around the aforementioned
> > quota transfers.
> > * if we don't do quota handling for minix and sysv (fair enough,
> > no quota support implemented for those), why the devil are we doing it
> > in simple_setattr()?
> >
> > Anyway, #new-truncate in the usual VFS tree contains all "new truncate
> > scheme" stuff I've collected from the lists so far. Additions are welcome...
>
> BTW, why are we messing with truncation in ->write_end() e.g. for ext2?
BTW^2, what's going to unmap the stale buffers in the page if
ext2_write_begin() runs into an error? ext2_truncate() (and thus vmtruncate()
that used to be called from block_write_begin()) used to do that, but
ext2_truncate_blocks() won't...
Frankly, I'm almost 100% convinced to postpone new-truncate merge until
.33-rc1; the first couple of patches (vmtruncate() unification and cleanups)
can go right now, but the rest obviously hadn't been beaten up enough
to seriously consider it for .32-rc1.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 00/11] new truncate sequence
2009-09-22 23:58 ` Al Viro
@ 2009-09-23 2:29 ` Al Viro
2009-09-27 19:50 ` Nick Piggin
2009-12-07 12:49 ` Nick Piggin
0 siblings, 2 replies; 35+ messages in thread
From: Al Viro @ 2009-09-23 2:29 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: npiggin, Andrew Morton, linux-fsdevel
On Wed, Sep 23, 2009 at 12:58:45AM +0100, Al Viro wrote:
> On Wed, Sep 23, 2009 at 12:27:42AM +0100, Al Viro wrote:
> > On Tue, Sep 22, 2009 at 10:51:38PM +0100, Al Viro wrote:
> > > It is... I really don't like some parts of that:
> > > * failing on foo_truncate() is all nice and proper, but
> > > what to do about the quota transfer that has already happened?
> > > Note that e.g. ext2_setattr() simply doesn't change uid/gid in
> > > case of ext2_setsize() failure. It does transfer quota, though.
> > > And yes, nfsd can produce calls with both ATTR_SIZE and ATTR_UID
> > > set.
> > > * way, _way_ too much boilerplate around the aforementioned
> > > quota transfers.
> > > * if we don't do quota handling for minix and sysv (fair enough,
> > > no quota support implemented for those), why the devil are we doing it
> > > in simple_setattr()?
> > >
> > > Anyway, #new-truncate in the usual VFS tree contains all "new truncate
> > > scheme" stuff I've collected from the lists so far. Additions are welcome...
> >
> > BTW, why are we messing with truncation in ->write_end() e.g. for ext2?
>
> BTW^2, what's going to unmap the stale buffers in the page if
> ext2_write_begin() runs into an error? ext2_truncate() (and thus vmtruncate()
> that used to be called from block_write_begin()) used to do that, but
> ext2_truncate_blocks() won't...
>
> Frankly, I'm almost 100% convinced to postpone new-truncate merge until
> .33-rc1; the first couple of patches (vmtruncate() unification and cleanups)
> can go right now, but the rest obviously hadn't been beaten up enough
> to seriously consider it for .32-rc1.
FWIW, I think that a reasonable battle plan for that series would look so:
* current first two patches
* vmtruncate()-less analogs of block_write_begin(), nobh_write_begin()
and blockdev_direct_IO(); original 3 functions themselves become wrappers.
* default_setattr()
* sort out the use of vmtruncate() in ecryptfs.
* at that point work on individual filesystems becomes independent;
we can convert them one by one at leisure, killing vmtruncate() uses in
each as we go. Basically, take an fs, shift vmtruncate() calls into the
methods (we will have full set of needed helpers) and lambda-expand each
(remember that vmtruncate() becomes a straightforward short sequence of helper
calls after the first stage). And replace that ->truncate() call with
explicit foofs_truncate(), switching .truncate to NULL while we are at it.
All equivalent transformations, all independent. Massage foo_setattr()
as we wish.
* once we are done, remove vmtruncate() and ->truncate() - no more
callers for them. Ditto for leftover 3 wrappers.
AFAICS, that gives a bisectable series with no "new_truncate" flags, with
no flagday interface changes at all and mergable just fine piecewise.
Comments?
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 00/11] new truncate sequence
2009-09-23 2:29 ` Al Viro
@ 2009-09-27 19:50 ` Nick Piggin
2009-12-07 12:49 ` Nick Piggin
1 sibling, 0 replies; 35+ messages in thread
From: Nick Piggin @ 2009-09-27 19:50 UTC (permalink / raw)
To: Al Viro; +Cc: Christoph Hellwig, Andrew Morton, linux-fsdevel
On Wed, Sep 23, 2009 at 03:29:21AM +0100, Al Viro wrote:
> On Wed, Sep 23, 2009 at 12:58:45AM +0100, Al Viro wrote:
> > On Wed, Sep 23, 2009 at 12:27:42AM +0100, Al Viro wrote:
> > > On Tue, Sep 22, 2009 at 10:51:38PM +0100, Al Viro wrote:
> > > > It is... I really don't like some parts of that:
> > > > * failing on foo_truncate() is all nice and proper, but
> > > > what to do about the quota transfer that has already happened?
> > > > Note that e.g. ext2_setattr() simply doesn't change uid/gid in
> > > > case of ext2_setsize() failure. It does transfer quota, though.
> > > > And yes, nfsd can produce calls with both ATTR_SIZE and ATTR_UID
> > > > set.
> > > > * way, _way_ too much boilerplate around the aforementioned
> > > > quota transfers.
> > > > * if we don't do quota handling for minix and sysv (fair enough,
> > > > no quota support implemented for those), why the devil are we doing it
> > > > in simple_setattr()?
> > > >
> > > > Anyway, #new-truncate in the usual VFS tree contains all "new truncate
> > > > scheme" stuff I've collected from the lists so far. Additions are welcome...
> > >
> > > BTW, why are we messing with truncation in ->write_end() e.g. for ext2?
> >
> > BTW^2, what's going to unmap the stale buffers in the page if
> > ext2_write_begin() runs into an error? ext2_truncate() (and thus vmtruncate()
> > that used to be called from block_write_begin()) used to do that, but
> > ext2_truncate_blocks() won't...
> >
> > Frankly, I'm almost 100% convinced to postpone new-truncate merge until
> > .33-rc1; the first couple of patches (vmtruncate() unification and cleanups)
> > can go right now, but the rest obviously hadn't been beaten up enough
> > to seriously consider it for .32-rc1.
>
> FWIW, I think that a reasonable battle plan for that series would look so:
> * current first two patches
> * vmtruncate()-less analogs of block_write_begin(), nobh_write_begin()
> and blockdev_direct_IO(); original 3 functions themselves become wrappers.
> * default_setattr()
> * sort out the use of vmtruncate() in ecryptfs.
> * at that point work on individual filesystems becomes independent;
> we can convert them one by one at leisure, killing vmtruncate() uses in
> each as we go. Basically, take an fs, shift vmtruncate() calls into the
> methods (we will have full set of needed helpers) and lambda-expand each
> (remember that vmtruncate() becomes a straightforward short sequence of helper
> calls after the first stage). And replace that ->truncate() call with
> explicit foofs_truncate(), switching .truncate to NULL while we are at it.
> All equivalent transformations, all independent. Massage foo_setattr()
> as we wish.
> * once we are done, remove vmtruncate() and ->truncate() - no more
> callers for them. Ditto for leftover 3 wrappers.
>
> AFAICS, that gives a bisectable series with no "new_truncate" flags, with
> no flagday interface changes at all and mergable just fine piecewise.
>
> Comments?
Seems probably reasonable... I've been at a conference and travelling
last week (sitting in an airport now), so apologies for intermittent
communication.
I'll take a look at it when I get home.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 00/11] new truncate sequence
2009-09-23 2:29 ` Al Viro
2009-09-27 19:50 ` Nick Piggin
@ 2009-12-07 12:49 ` Nick Piggin
2009-12-07 22:46 ` Tyler Hicks
1 sibling, 1 reply; 35+ messages in thread
From: Nick Piggin @ 2009-12-07 12:49 UTC (permalink / raw)
To: Al Viro, Jan Kara
Cc: Christoph Hellwig, Andrew Morton, linux-fsdevel, ecryptfs-devel,
tyhicks, kirkland
On Wed, Sep 23, 2009 at 03:29:21AM +0100, Al Viro wrote:
> On Wed, Sep 23, 2009 at 12:58:45AM +0100, Al Viro wrote:
> > Frankly, I'm almost 100% convinced to postpone new-truncate merge until
> > .33-rc1; the first couple of patches (vmtruncate() unification and cleanups)
> > can go right now, but the rest obviously hadn't been beaten up enough
> > to seriously consider it for .32-rc1.
>
> FWIW, I think that a reasonable battle plan for that series would look so:
> * current first two patches
> * vmtruncate()-less analogs of block_write_begin(), nobh_write_begin()
> and blockdev_direct_IO(); original 3 functions themselves become wrappers.
> * default_setattr()
> * sort out the use of vmtruncate() in ecryptfs.
> * at that point work on individual filesystems becomes independent;
> we can convert them one by one at leisure, killing vmtruncate() uses in
> each as we go. Basically, take an fs, shift vmtruncate() calls into the
> methods (we will have full set of needed helpers) and lambda-expand each
> (remember that vmtruncate() becomes a straightforward short sequence of helper
> calls after the first stage). And replace that ->truncate() call with
> explicit foofs_truncate(), switching .truncate to NULL while we are at it.
> All equivalent transformations, all independent. Massage foo_setattr()
> as we wish.
> * once we are done, remove vmtruncate() and ->truncate() - no more
> callers for them. Ditto for leftover 3 wrappers.
>
> AFAICS, that gives a bisectable series with no "new_truncate" flags, with
> no flagday interface changes at all and mergable just fine piecewise.
>
> Comments?
I'm having a look at this again, sorry for the delay. I would still
like to merge it in 2.6.33 if we can; at least the core parts so that
we can push fs specific patches to their various maintainers.
Your plan for avoiding .truncate_kludge works, although I still like
that flag so we can put BUG_ONs around the place.... but it is ugly
so maybe we just skip it.
It looks relatively simple, and I'm also updating the series to account
for the writev data loss bug.
Ecryptfs. To start with, it isn't taking i_mutex on the lower inode when
calling vmtruncate so locking is wrong. Secondly, I don't think it is
allowed to assume vmtruncate does the right thing. The only correct
generic entry point to the lower filesystem AFAIKS is notify_change...
which would allow it to work fine with the new truncate sequence. So
any reason why they can't do that?
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch 00/11] new truncate sequence
2009-12-07 12:49 ` Nick Piggin
@ 2009-12-07 22:46 ` Tyler Hicks
0 siblings, 0 replies; 35+ messages in thread
From: Tyler Hicks @ 2009-12-07 22:46 UTC (permalink / raw)
To: Nick Piggin
Cc: Al Viro, Jan Kara, Christoph Hellwig, Andrew Morton,
linux-fsdevel, ecryptfs-devel, kirkland
On Mon Dec 07, 2009 at 01:49:02PM +0100, Nick Piggin (npiggin@suse.de) was quoted:
> On Wed, Sep 23, 2009 at 03:29:21AM +0100, Al Viro wrote:
> > On Wed, Sep 23, 2009 at 12:58:45AM +0100, Al Viro wrote:
> > > Frankly, I'm almost 100% convinced to postpone new-truncate merge until
> > > .33-rc1; the first couple of patches (vmtruncate() unification and cleanups)
> > > can go right now, but the rest obviously hadn't been beaten up enough
> > > to seriously consider it for .32-rc1.
> >
> > FWIW, I think that a reasonable battle plan for that series would look so:
> > * current first two patches
> > * vmtruncate()-less analogs of block_write_begin(), nobh_write_begin()
> > and blockdev_direct_IO(); original 3 functions themselves become wrappers.
> > * default_setattr()
> > * sort out the use of vmtruncate() in ecryptfs.
> > * at that point work on individual filesystems becomes independent;
> > we can convert them one by one at leisure, killing vmtruncate() uses in
> > each as we go. Basically, take an fs, shift vmtruncate() calls into the
> > methods (we will have full set of needed helpers) and lambda-expand each
> > (remember that vmtruncate() becomes a straightforward short sequence of helper
> > calls after the first stage). And replace that ->truncate() call with
> > explicit foofs_truncate(), switching .truncate to NULL while we are at it.
> > All equivalent transformations, all independent. Massage foo_setattr()
> > as we wish.
> > * once we are done, remove vmtruncate() and ->truncate() - no more
> > callers for them. Ditto for leftover 3 wrappers.
> >
> > AFAICS, that gives a bisectable series with no "new_truncate" flags, with
> > no flagday interface changes at all and mergable just fine piecewise.
> >
> > Comments?
>
> I'm having a look at this again, sorry for the delay. I would still
> like to merge it in 2.6.33 if we can; at least the core parts so that
> we can push fs specific patches to their various maintainers.
>
> Your plan for avoiding .truncate_kludge works, although I still like
> that flag so we can put BUG_ONs around the place.... but it is ugly
> so maybe we just skip it.
>
> It looks relatively simple, and I'm also updating the series to account
> for the writev data loss bug.
>
> Ecryptfs. To start with, it isn't taking i_mutex on the lower inode when
> calling vmtruncate so locking is wrong. Secondly, I don't think it is
> allowed to assume vmtruncate does the right thing. The only correct
> generic entry point to the lower filesystem AFAIKS is notify_change...
> which would allow it to work fine with the new truncate sequence. So
> any reason why they can't do that?
Nope. I've rewritten the eCryptfs truncate path to do just that. I
plan on getting the patch into 2.6.33-rc1.
http://git.kernel.org/?p=linux/kernel/git/ecryptfs/ecryptfs-2.6.git;a=commit;h=8137e6500c0b6d627656aa4d9f48dd9349c06051
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2009-12-07 22:46 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-08-20 16:35 [patch 00/11] new truncate sequence npiggin
2009-08-20 16:35 ` [patch 01/11] fs: new truncate helpers npiggin
2009-08-26 7:38 ` Artem Bityutskiy
2009-09-07 7:33 ` Nick Piggin
2009-09-07 7:48 ` Artem Bityutskiy
2009-08-20 16:35 ` [patch 02/11] fs: use " npiggin-l3A5Bk7waGM
2009-08-20 16:35 ` [patch 03/11] fs: introduce new truncate sequence npiggin
2009-08-26 7:40 ` Artem Bityutskiy
2009-08-20 16:35 ` [patch 04/11] fs: convert simple fs to new truncate npiggin
2009-08-20 16:35 ` [patch 05/11] tmpfs: convert to use the new truncate convention npiggin
2009-08-20 16:35 ` [patch 06/11] ext2: " npiggin
2009-08-21 13:42 ` Jan Kara
2009-08-21 14:06 ` Jan Kara
2009-08-24 5:30 ` [patch] ext2: convert to use the new truncate convention fix Nick Piggin
2009-08-20 16:35 ` [patch 07/11] fat: convert to use the new truncate convention npiggin
2009-08-20 16:35 ` [patch 08/11] btrfs: " npiggin
2009-08-20 16:35 ` [patch 09/11] jfs: " npiggin
2009-08-20 16:35 ` [patch 10/11] udf: " npiggin
2009-08-21 14:22 ` Jan Kara
2009-08-24 5:33 ` Nick Piggin
2009-08-20 16:35 ` [patch 11/11] minix: " npiggin
2009-09-09 7:11 ` [patch 00/11] new truncate sequence Artem Bityutskiy
2009-09-22 15:04 ` Al Viro
2009-09-22 20:00 ` Christoph Hellwig
2009-09-22 21:51 ` Al Viro
2009-09-22 23:27 ` Al Viro
2009-09-22 23:58 ` Al Viro
2009-09-23 2:29 ` Al Viro
2009-09-27 19:50 ` Nick Piggin
2009-12-07 12:49 ` Nick Piggin
2009-12-07 22:46 ` Tyler Hicks
2009-09-22 20:53 ` [PATCH 12/n] kill spurious reference to vmtruncate Christoph Hellwig
2009-09-22 20:54 ` [PATCH 13/n] xfs: new truncate sequence Christoph Hellwig
2009-09-22 20:55 ` [PATCH 14/n] sysv: " Christoph Hellwig
2009-09-22 20:56 ` [PATCH 15/n] ntfs: " Christoph Hellwig
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).