Re: [PATCH 3/3] AFS: Implement basic file write support

2007-05-10 Thread Nick Piggin

David Howells wrote:

Nick Piggin <[EMAIL PROTECTED]> wrote:



Why do you call SetPageUptodate when the page is not up to date?
That leaks uninitialised data, AFAIKS.



It only seems that way.  If afs_prepare_write() is called, but doesn't return
an error, then afs_commit_write() will be called, and it seems that the copy
in of the data will be guaranteed not to fail by the caller.


Not only does it seem that way, it is that way :) PG_uptodate is being set
when the page is not uptodate, isn't it?



Furthermore, afs_prepare_page() will have filled in the missing bits.

And whilst all that is going on, the page lock will be help by the caller, so
that no-one else can access the partially complete page.


When a page is uptodate in pagecache, the generic read and nopage functions
do not take the page lock. So how are you excluding those?

--
SUSE Labs, Novell Inc.
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] file capabilities: accomodate >32 bit capabilities

2007-05-10 Thread Suparna Bhattacharya
On Thu, May 10, 2007 at 01:01:27PM -0700, Andreas Dilger wrote:
> On May 08, 2007  16:49 -0500, Serge E. Hallyn wrote:
> > Quoting Andreas Dilger ([EMAIL PROTECTED]):
> > > One of the important use cases I can see today is the ability to
> > > split the heavily-overloaded e.g. CAP_SYS_ADMIN into much more fine
> > > grained attributes.
> > 
> > Sounds plausible, though it suffers from both making capabilities far
> > more cumbersome (i.e. finding the right capability for what you wanted
> > to do) and backward compatibility.  Perhaps at that point we should
> > introduce security.capabilityv2 xattrs.  A binary can then carry
> > security.capability=CAP_SYS_ADMIN=p, and
> > security.capabilityv2=cap_may_clone_mntns=p.
> 
> Well, the overhead of each EA is non-trivial (16 bytes/EA) for storing
> 12 bytes worth of data, so it is probably just better to keep extending
> the original capability fields as was in the proposal.
> 
> > > What we definitely do NOT want to happen is an application that needs
> > > priviledged access (e.g. e2fsck, mount) to stop running because the
> > > new capabilities _would_ have been granted by the new kernel and are
> > > not by the old kernel and STRICTXATTR is used.
> > > 
> > > To me it would seem that having extra capabilities on an old kernel
> > > is relatively harmless if the old kernel doesn't know what they are.
> > > It's like having a key to a door that you don't know where it is.
> > 
> > If we ditch the STRICTXATTR option do the semantics seem sane to you?
> 
> Seems reasonable.

It would simplify the code as well, which is good.

This does mean no sanity checking of fcaps, am not sure if that matters,
I'm guessing it should be similar to the case for other security attributes.

Regards
Suparna

> 
> Cheers, Andreas
> --
> Andreas Dilger
> Principal Software Engineer
> Cluster File Systems, Inc.
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to [EMAIL PROTECTED]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Suparna Bhattacharya ([EMAIL PROTECTED])
Linux Technology Center
IBM Software Lab, India

-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/2] AFS: Fix interminable loop in afs_write_back_from_locked_page()

2007-05-10 Thread Andrew Morton
On Thu, 10 May 2007 15:33:34 +0100
David Howells <[EMAIL PROTECTED]> wrote:

> Following bug was uncovered by compiling with '-W' flag:

gcc -W finds a number of fairly scary bugs.

More than one would expect, given that it is recommended in
Documentation/SubmitChecklist, which everyone reads ;)
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/5] fallocate() implementation in i86, x86_64 and powerpc

2007-05-10 Thread David Chinner
On Thu, May 10, 2007 at 05:26:20PM +0530, Amit K. Arora wrote:
> On Thu, May 10, 2007 at 10:59:26AM +1000, David Chinner wrote:
> > On Wed, May 09, 2007 at 09:31:02PM +0530, Amit K. Arora wrote:
> > > I have the updated patches ready which take care of Andrew's comments.
> > > Will run some tests and post them soon.
> > > 
> > > But, before submitting these patches, I think it will be better to
> > > finalize on certain things which might be worth some discussion here:
> > > 
> > > 1) Should the file size change when preallocation is done beyond EOF ?
> > > - Andreas and Chris Wedgwood are in favor of not changing the file size
> > > in this case. I also tend to agree with them. Does anyone has an
> > > argument in favor of changing the filesize ?  If not, I will remove the
> > > code which changes the filesize, before I resubmit the concerned ext4
> > > patch.
> > 
> > I think there needs to be both. If we don't have a mechanism to atomically
> > change the file size with the preallocation, then applications that use
> > stat() to work out if they need to preallocate more space will end up
> > racing.
> 
> By "both" above, do you mean we should give user the flexibility if it wants
> the filesize changed or not ? It can be done by having *two* modes for
> preallocation in the system call - say FA_PREALLOCATE and FA_ALLOCATE. If we
> use FA_PREALLOCATE mode, fallocate() will allocate blocks, but will not
> change the filesize and [cm]time. If FA_ALLOCATE mode is used, fallocate()
> will change the filesize if required (i.e.  when allocation is beyond EOF)
> and also update [cm]time.  This way, the application can decide what it
> wants.

Yes, that's right.

> This will be helpfull for the partial allocation scenario also. Think of the
> case when we do not change the filesize in fallocate() and expect
> applications/posix_fallocate() to do ftruncate() after fallocate() for this.
> Now if fallocate() results in a partial allocation with -ENOSPC error
> returned, applications/posix_fallocate() will not know for what length
> ftruncate() has to be called.  :(

Well, posix_fallocate() either gets all the space or it fails. If
you truncate to extend the file size after an ENOSPC, then that is
a buggy implementation.

The same could be said for any application, or even the fallocate()
call itself if it changes the filesize without having completely
preallocated the space asked

> Hence it may be a good idea to give user the flexibility if it wants to
> atomically change the file size with preallocation or not. But, with more
> flexibility there comes inconsistency in behavior, which is worth
> considering.

We've got different modes to specify different behaviour. That's
what the mode field was put there for in the first place - the
interface is *designed* to support different preallocation
behaviours

> > > 2) For FA_UNALLOCATE mode, should the file system allow unallocation of
> > > normal (non-preallocated) blocks (blocks allocated via regular
> > > write/truncate operations) also (i.e. work as punch()) ?
> > 
> > Yes. That is the current XFS implementation for XFS_IOC_UNRESVSP, and what
> > i did for FA_UNALLOCATE as well.
> 
> Ok. But, some people may not expect/like this. I think, we can keep it on
> the backburner for a while, till other issues are sorted out.

How can it be a "backburner" issue when it defines the
implementation?  I've already implemented some thing in XFS that
sort of does what I think that the interface is supposed to do, but
I need that interface to be nailed down before proceeding any
further.

All I'm really interested in right now is that the fallocate
_interface_ can be used as a *complete replacement* for the
pre-existing XFS-specific ioctls that are already used by
applications.  What ext4 can or can't do right now is irrelevant to
this discussion - the interface definition needs to take priority
over implementation

Cheers,

Dave,
-- 
Dave Chinner
Principal Engineer
SGI Australian Software Group
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/2] file capabilities: accomodate >32 bit capabilities

2007-05-10 Thread Andreas Dilger
On May 08, 2007  16:49 -0500, Serge E. Hallyn wrote:
> Quoting Andreas Dilger ([EMAIL PROTECTED]):
> > One of the important use cases I can see today is the ability to
> > split the heavily-overloaded e.g. CAP_SYS_ADMIN into much more fine
> > grained attributes.
> 
> Sounds plausible, though it suffers from both making capabilities far
> more cumbersome (i.e. finding the right capability for what you wanted
> to do) and backward compatibility.  Perhaps at that point we should
> introduce security.capabilityv2 xattrs.  A binary can then carry
> security.capability=CAP_SYS_ADMIN=p, and
> security.capabilityv2=cap_may_clone_mntns=p.

Well, the overhead of each EA is non-trivial (16 bytes/EA) for storing
12 bytes worth of data, so it is probably just better to keep extending
the original capability fields as was in the proposal.

> > What we definitely do NOT want to happen is an application that needs
> > priviledged access (e.g. e2fsck, mount) to stop running because the
> > new capabilities _would_ have been granted by the new kernel and are
> > not by the old kernel and STRICTXATTR is used.
> > 
> > To me it would seem that having extra capabilities on an old kernel
> > is relatively harmless if the old kernel doesn't know what they are.
> > It's like having a key to a door that you don't know where it is.
> 
> If we ditch the STRICTXATTR option do the semantics seem sane to you?

Seems reasonable.

Cheers, Andreas
--
Andreas Dilger
Principal Software Engineer
Cluster File Systems, Inc.

-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] AFS: Implement statfs

2007-05-10 Thread David Howells
Implement the statfs() op for AFS.

Signed-off-by: David Howells <[EMAIL PROTECTED]>
---

 fs/afs/afs.h  |   23 
 fs/afs/afs_fs.h   |3 -
 fs/afs/dir.c  |   18 ++-
 fs/afs/fsclient.c |  298 +
 fs/afs/internal.h |6 +
 fs/afs/super.c|   41 ++-
 fs/afs/vnode.c|   52 +
 7 files changed, 426 insertions(+), 15 deletions(-)

diff --git a/fs/afs/afs.h b/fs/afs/afs.h
index 52d0752..2452579 100644
--- a/fs/afs/afs.h
+++ b/fs/afs/afs.h
@@ -16,6 +16,9 @@
 
 #define AFS_MAXCELLNAME64  /* maximum length of a cell 
name */
 #define AFS_MAXVOLNAME 64  /* maximum length of a volume name */
+#define AFSNAMEMAX 256 /* maximum length of a filename plus 
NUL */
+#define AFSPATHMAX 1024/* maximum length of a pathname plus 
NUL */
+#define AFSOPAQUEMAX   1024/* maximum length of an opaque field */
 
 typedef unsigned   afs_volid_t;
 typedef unsigned   afs_vnodeid_t;
@@ -143,4 +146,24 @@ struct afs_volsync {
time_t  creation;   /* volume creation time */
 };
 
+/*
+ * AFS volume status record
+ */
+struct afs_volume_status {
+   u32 vid;/* volume ID */
+   u32 parent_id;  /* parent volume ID */
+   u8  online; /* true if volume currently 
online and available */
+   u8  in_service; /* true if volume currently in 
service */
+   u8  blessed;/* same as in_service */
+   u8  needs_salvage;  /* true if consistency checking 
required */
+   u32 type;   /* volume type (afs_voltype_t) 
*/
+   u32 min_quota;  /* minimum space set aside 
(blocks) */
+   u32 max_quota;  /* maximum space this volume 
may occupy (blocks) */
+   u32 blocks_in_use;  /* space this volume currently 
occupies (blocks) */
+   u32 part_blocks_avail; /* space available in 
volume's partition */
+   u32 part_max_blocks; /* size of volume's partition 
*/
+};
+
+#define AFS_BLOCK_SIZE 1024
+
 #endif /* AFS_H */
diff --git a/fs/afs/afs_fs.h b/fs/afs/afs_fs.h
index d963ef4..a18c374 100644
--- a/fs/afs/afs_fs.h
+++ b/fs/afs/afs_fs.h
@@ -28,7 +28,8 @@ enum AFS_FS_Operations {
FSMAKEDIR   = 141,  /* AFS Create a directory */
FSREMOVEDIR = 142,  /* AFS Remove a directory */
FSGIVEUPCALLBACKS   = 147,  /* AFS Discard callback promises */
-   FSGETVOLUMEINFO = 148,  /* AFS Get root volume information */
+   FSGETVOLUMEINFO = 148,  /* AFS Get information about a volume */
+   FSGETVOLUMESTATUS   = 149,  /* AFS Get volume status information */
FSGETROOTVOLUME = 151,  /* AFS Get root volume name */
FSLOOKUP= 161,  /* AFS lookup file in directory */
FSFETCHDATA64   = 65537, /* AFS Fetch file data */
diff --git a/fs/afs/dir.c b/fs/afs/dir.c
index ac982a8..92b2f1e 100644
--- a/fs/afs/dir.c
+++ b/fs/afs/dir.c
@@ -497,7 +497,7 @@ static struct dentry *afs_lookup(struct inode *dir, struct 
dentry *dentry,
 
ASSERTCMP(dentry->d_inode, ==, NULL);
 
-   if (dentry->d_name.len > 255) {
+   if (dentry->d_name.len >= AFSNAMEMAX) {
_leave(" = -ENAMETOOLONG");
return ERR_PTR(-ENAMETOOLONG);
}
@@ -736,7 +736,7 @@ static int afs_mkdir(struct inode *dir, struct dentry 
*dentry, int mode)
   dvnode->fid.vid, dvnode->fid.vnode, dentry->d_name.name, mode);
 
ret = -ENAMETOOLONG;
-   if (dentry->d_name.len > 255)
+   if (dentry->d_name.len >= AFSNAMEMAX)
goto error;
 
key = afs_request_key(dvnode->volume->cell);
@@ -801,7 +801,7 @@ static int afs_rmdir(struct inode *dir, struct dentry 
*dentry)
   dvnode->fid.vid, dvnode->fid.vnode, dentry->d_name.name);
 
ret = -ENAMETOOLONG;
-   if (dentry->d_name.len > 255)
+   if (dentry->d_name.len >= AFSNAMEMAX)
goto error;
 
key = afs_request_key(dvnode->volume->cell);
@@ -847,7 +847,7 @@ static int afs_unlink(struct inode *dir, struct dentry 
*dentry)
   dvnode->fid.vid, dvnode->fid.vnode, dentry->d_name.name);
 
ret = -ENAMETOOLONG;
-   if (dentry->d_name.len > 255)
+   if (dentry->d_name.len >= AFSNAMEMAX)
goto error;
 
key = afs_request_key(dvnode->volume->cell);
@@ -921,7 +921,7 @@ static int afs_create(struct inode *dir, struct dentry 
*dentry, int mode,
   dvnode->fid.vid, dvnode->fid.vnode, dentry->d_name.name, mode);
 
ret = -ENAMETOOLONG;
-   if (dentry->d_name.len > 255)
+   if (dentry->d_name.len >= AFSNAMEMAX)

[PATCH 2/2] AFS: Fix a couple of problems with unlinking AFS files

2007-05-10 Thread David Howells
Fix a couple of problems with unlinking AFS files.

 (1) The parent directory wasn't being updated properly between unlink() and
 the following lookup().

 It seems that, for some reason, invalidate_remote_inode() wasn't
 discarding the directory contents correctly, so this patch calls
 invalidate_inode_pages2() instead on non-regular files.

 (2) afs_vnode_deleted_remotely() should handle vnodes that don't have a
 source server recorded without oopsing.

Signed-off-by: David Howells <[EMAIL PROTECTED]>
---

 fs/afs/file.c  |2 +-
 fs/afs/inode.c |   10 +++---
 fs/afs/super.c |3 ++-
 fs/afs/vnode.c |   33 +
 4 files changed, 31 insertions(+), 17 deletions(-)

diff --git a/fs/afs/file.c b/fs/afs/file.c
index 3e25795..9c0e721 100644
--- a/fs/afs/file.c
+++ b/fs/afs/file.c
@@ -236,7 +236,7 @@ static void afs_invalidatepage(struct page *page, unsigned 
long offset)
 {
int ret = 1;
 
-   kenter("{%lu},%lu", page->index, offset);
+   _enter("{%lu},%lu", page->index, offset);
 
BUG_ON(!PageLocked(page));
 
diff --git a/fs/afs/inode.c b/fs/afs/inode.c
index 515a5d1..47f5fed 100644
--- a/fs/afs/inode.c
+++ b/fs/afs/inode.c
@@ -209,11 +209,15 @@ bad_inode:
  */
 void afs_zap_data(struct afs_vnode *vnode)
 {
-   _enter("zap data {%x:%u}", vnode->fid.vid, vnode->fid.vnode);
+   _enter("{%x:%u}", vnode->fid.vid, vnode->fid.vnode);
 
/* nuke all the non-dirty pages that aren't locked, mapped or being
-* written back */
-   invalidate_remote_inode(&vnode->vfs_inode);
+* written back in a regular file and completely discard the pages in a
+* directory or symlink */
+   if (S_ISREG(vnode->vfs_inode.i_mode))
+   invalidate_remote_inode(&vnode->vfs_inode);
+   else
+   invalidate_inode_pages2(vnode->vfs_inode.i_mapping);
 }
 
 /*
diff --git a/fs/afs/super.c b/fs/afs/super.c
index d24be33..422f532 100644
--- a/fs/afs/super.c
+++ b/fs/afs/super.c
@@ -488,6 +488,7 @@ static struct inode *afs_alloc_inode(struct super_block *sb)
vnode->flags= 1 << AFS_VNODE_UNSET;
vnode->cb_promised  = false;
 
+   _leave(" = %p", &vnode->vfs_inode);
return &vnode->vfs_inode;
 }
 
@@ -498,7 +499,7 @@ static void afs_destroy_inode(struct inode *inode)
 {
struct afs_vnode *vnode = AFS_FS_I(inode);
 
-   _enter("{%lu}", inode->i_ino);
+   _enter("%p{%x:%u}", inode, vnode->fid.vid, vnode->fid.vnode);
 
_debug("DESTROY INODE %p", inode);
 
diff --git a/fs/afs/vnode.c b/fs/afs/vnode.c
index ec81466..bea8bd9 100644
--- a/fs/afs/vnode.c
+++ b/fs/afs/vnode.c
@@ -175,24 +175,33 @@ static void afs_vnode_deleted_remotely(struct afs_vnode 
*vnode)
 {
struct afs_server *server;
 
+   _enter("{%p}", vnode->server);
+
set_bit(AFS_VNODE_DELETED, &vnode->flags);
 
server = vnode->server;
-   if (vnode->cb_promised) {
-   spin_lock(&server->cb_lock);
+   if (server) {
if (vnode->cb_promised) {
-   rb_erase(&vnode->cb_promise, &server->cb_promises);
-   vnode->cb_promised = false;
+   spin_lock(&server->cb_lock);
+   if (vnode->cb_promised) {
+   rb_erase(&vnode->cb_promise,
+&server->cb_promises);
+   vnode->cb_promised = false;
+   }
+   spin_unlock(&server->cb_lock);
}
-   spin_unlock(&server->cb_lock);
-   }
 
-   spin_lock(&vnode->server->fs_lock);
-   rb_erase(&vnode->server_rb, &vnode->server->fs_vnodes);
-   spin_unlock(&vnode->server->fs_lock);
+   spin_lock(&server->fs_lock);
+   rb_erase(&vnode->server_rb, &server->fs_vnodes);
+   spin_unlock(&server->fs_lock);
 
-   vnode->server = NULL;
-   afs_put_server(server);
+   vnode->server = NULL;
+   afs_put_server(server);
+   } else {
+   ASSERT(!vnode->cb_promised);
+   }
+
+   _leave("");
 }
 
 /*
@@ -225,7 +234,7 @@ void afs_vnode_finalise_status_update(struct afs_vnode 
*vnode,
  */
 static void afs_vnode_status_update_failed(struct afs_vnode *vnode, int ret)
 {
-   _enter("%p,%d", vnode, ret);
+   _enter("{%x:%u},%d", vnode->fid.vid, vnode->fid.vnode, ret);
 
spin_lock(&vnode->lock);
 

-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 1/2] AFS: Fix interminable loop in afs_write_back_from_locked_page()

2007-05-10 Thread David Howells
Following bug was uncovered by compiling with '-W' flag:

  CC [M]  fs/afs/write.o
fs/afs/write.c: In function ‘afs_write_back_from_locked_page’:
fs/afs/write.c:398: warning: comparison of unsigned expression >= 0 is always 
true

Loop variable 'n' is unsigned, so wraps around happily as far as I can
see. Trival fix attached (compile tested only).

Signed-Off-By: Mika Kukkonen <[EMAIL PROTECTED]>
Signed-off-by: David Howells <[EMAIL PROTECTED]>
---

 fs/afs/write.c |5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/afs/write.c b/fs/afs/write.c
index 67ae4db..28f3751 100644
--- a/fs/afs/write.c
+++ b/fs/afs/write.c
@@ -395,8 +395,9 @@ static int afs_write_back_from_locked_page(struct 
afs_writeback *wb,
if (n == 0)
goto no_more;
if (pages[0]->index != start) {
-   for (n--; n >= 0; n--)
-   put_page(pages[n]);
+   do {
+   put_page(pages[--n]);
+   } while (n > 0);
goto no_more;
}
 

-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/5] fallocate() implementation in i86, x86_64 and powerpc

2007-05-10 Thread Amit K. Arora
On Thu, May 10, 2007 at 10:59:26AM +1000, David Chinner wrote:
> On Wed, May 09, 2007 at 09:31:02PM +0530, Amit K. Arora wrote:
> > I have the updated patches ready which take care of Andrew's comments.
> > Will run some tests and post them soon.
> > 
> > But, before submitting these patches, I think it will be better to finalize
> > on certain things which might be worth some discussion here:
> > 
> > 1) Should the file size change when preallocation is done beyond EOF ?
> >- Andreas and Chris Wedgwood are in favor of not changing the
> >  file size in this case. I also tend to agree with them. Does anyone
> >  has an argument in favor of changing the filesize ?
> >  If not, I will remove the code which changes the filesize, before I
> >  resubmit the concerned ext4 patch.
> 
> I think there needs to be both. If we don't have a mechanism to
> atomically change the file size with the preallocation, then
> applications that use stat() to work out if they need to preallocate
> more space will end up racing.

By "both" above, do you mean we should give user the flexibility if it
wants the filesize changed or not ? It can be done by having *two* modes
for preallocation in the system call - say FA_PREALLOCATE and
FA_ALLOCATE. If we use FA_PREALLOCATE mode, fallocate() will allocate
blocks, but will not change the filesize and [cm]time. If FA_ALLOCATE
mode is used, fallocate() will change the filesize if required (i.e.
when allocation is beyond EOF) and also update [cm]time.
This way, the application can decide what it wants.

This will be helpfull for the partial allocation scenario also. Think of
the case when we do not change the filesize in fallocate() and expect
applications/posix_fallocate() to do ftruncate() after fallocate() for
this. Now if fallocate() results in a partial allocation with -ENOSPC
error returned, applications/posix_fallocate() will not know for what
length ftruncate() has to be called.  :(

Hence it may be a good idea to give user the flexibility if it wants to
atomically change the file size with preallocation or not. But, with
more flexibility there comes inconsistency in behavior, which is worth
considering.

> 
> > 2) For FA_UNALLOCATE mode, should the file system allow unallocation
> >of normal (non-preallocated) blocks (blocks allocated via
> >regular write/truncate operations) also (i.e. work as punch()) ?
> 
> Yes. That is the current XFS implementation for XFS_IOC_UNRESVSP, and
> what i did for FA_UNALLOCATE as well.

Ok. But, some people may not expect/like this. I think, we can keep it
on the backburner for a while, till other issues are sorted out.
 
> >- Though FA_UNALLOCATE mode is yet to be implemented on ext4, still
> >  we need to finalize on the convention here as a general guideline
> >  to all the filesystems that implement fallocate.
> > 
> > 3) If above is true, the file size will need to be changed
> >for "unallocation" when block holding the EOF gets unallocated.
> 
> No - we punch a hole. If you want the filesize to change, then
> you use ftruncate() to remove the blocks at EOF and change the
> file size atomically.

Ok.
> 
> > 4) Should we update mtime & ctime on a successfull allocation/
> >unallocation ?
> >- David Chinner raised this question in following post:
> >  http://lkml.org/lkml/2007/4/29/407
> >  I think it makes sense to update the [mc]time for a successfull
> >  preallocation/unallocation. Does anyone feel otherwise ?
> >  It will be interesting to know how XFS behaves currently. Does XFS
> >  update [mc]time for preallocation ?
> 
> No, XFS does *not* update a/m/ctime on prealloc/punch unless the file size
> changes. If the filesize changes, it behaves exactly the same way that
> ftruncate() behaves.

Having additional mode (of FA_PREALLOCATE) might help here too. Please
see above.

--
Regards,
Amit Arora
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 3/3] AFS: Implement basic file write support

2007-05-10 Thread David Howells
Nick Piggin <[EMAIL PROTECTED]> wrote:

> Why do you call SetPageUptodate when the page is not up to date?
> That leaks uninitialised data, AFAIKS.

It only seems that way.  If afs_prepare_write() is called, but doesn't return
an error, then afs_commit_write() will be called, and it seems that the copy
in of the data will be guaranteed not to fail by the caller.

Furthermore, afs_prepare_page() will have filled in the missing bits.

And whilst all that is going on, the page lock will be help by the caller, so
that no-one else can access the partially complete page.

I suppose I could call SetPageUptodate() in afs_commit_write() instead.

David
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html