[Patch] fs/romfs/inode.c: trivial improvements

2007-07-18 Thread WANG Cong

1. There's no lists in fs/romfs/inode.c, so using list_entry is
a bit confusing. Replace it with container_of.

2. It is unnecessary to cast the return value of kmem_cache_alloc,
since it returns a void* pointer.

This patch is against 2.6.22.1.

Signed-off-by: WANG Cong [EMAIL PROTECTED]

---
 fs/romfs/inode.c |4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux-2.6.22.1/fs/romfs/inode.c
===
--- linux-2.6.22.1.orig/fs/romfs/inode.c2007-07-12 00:47:28.0 
+0800
+++ linux-2.6.22.1/fs/romfs/inode.c 2007-07-18 16:08:56.0 +0800
@@ -92,7 +92,7 @@
 
 static inline struct romfs_inode_info *ROMFS_I(struct inode *inode)
 {
-   return list_entry(inode, struct romfs_inode_info, vfs_inode);
+   return container_of(inode, struct romfs_inode_info, vfs_inode);
 }
 
 static __u32
@@ -555,7 +555,7 @@
 static struct inode *romfs_alloc_inode(struct super_block *sb)
 {
struct romfs_inode_info *ei;
-   ei = (struct romfs_inode_info *)kmem_cache_alloc(romfs_inode_cachep, 
GFP_KERNEL);
+   ei = kmem_cache_alloc(romfs_inode_cachep, GFP_KERNEL);
if (!ei)
return NULL;
return ei-vfs_inode;
-
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] block: cosmetic changes

2007-07-18 Thread Tejun Heo
Cosmetic changes.  This is taken from Jens' zero-length barrier patch.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]
Cc: Jens Axboe [EMAIL PROTECTED]
---
 block/ll_rw_blk.c |5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Index: work/block/ll_rw_blk.c
===
--- work.orig/block/ll_rw_blk.c
+++ work/block/ll_rw_blk.c
@@ -443,7 +443,8 @@ static inline struct request *start_orde
rq_init(q, rq);
if (bio_data_dir(q-orig_bar_rq-bio) == WRITE)
rq-cmd_flags |= REQ_RW;
-   rq-cmd_flags |= q-ordered  QUEUE_ORDERED_FUA ? REQ_FUA : 0;
+   if (q-ordered  QUEUE_ORDERED_FUA)
+   rq-cmd_flags |= REQ_FUA;
rq-elevator_private = NULL;
rq-elevator_private2 = NULL;
init_request_from_bio(rq, q-orig_bar_rq-bio);
@@ -3167,7 +3168,7 @@ end_io:
break;
}
 
-   if (unlikely(bio_sectors(bio)  q-max_hw_sectors)) {
+   if (unlikely(nr_sectors  q-max_hw_sectors)) {
printk(bio too big device %s (%u  %u)\n, 
bdevname(bio-bi_bdev, b),
bio_sectors(bio),
-
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] block: factor out bio_check_eod()

2007-07-18 Thread Tejun Heo
End of device check is done twice in __generic_make_request() and it's
fully inlined each time.  Factor out bio_check_eod().

This is taken from Jens' zero-length barrier patch.

Signed-off-by: Tejun Heo [EMAIL PROTECTED]
Cc: Jens Axboe [EMAIL PROTECTED]
---
 block/ll_rw_blk.c |   63 --
 1 file changed, 33 insertions(+), 30 deletions(-)

Index: work/block/ll_rw_blk.c
===
--- work.orig/block/ll_rw_blk.c
+++ work/block/ll_rw_blk.c
@@ -3094,6 +3094,35 @@ static inline int should_fail_request(st
 
 #endif /* CONFIG_FAIL_MAKE_REQUEST */
 
+/*
+ * Check whether this bio extends beyond the end of the device.
+ */
+static int bio_check_eod(struct bio *bio, unsigned int nr_sectors)
+{
+   sector_t maxsector;
+
+   if (!nr_sectors)
+   return 0;
+
+   /* Test device or partition size, when known. */
+   maxsector = bio-bi_bdev-bd_inode-i_size  9;
+   if (maxsector) {
+   sector_t sector = bio-bi_sector;
+
+   if (maxsector  nr_sectors || maxsector - nr_sectors  sector) {
+   /*
+* This may well happen - the kernel calls bread()
+* without checking the size of the device, e.g., when
+* mounting a device.
+*/
+   handle_bad_sector(bio);
+   return 1;
+   }
+   }
+
+   return 0;
+}
+
 /**
  * generic_make_request: hand a buffer to its device driver for I/O
  * @bio:  The bio describing the location in memory and on the device.
@@ -3121,27 +3150,14 @@ static inline int should_fail_request(st
 static inline void __generic_make_request(struct bio *bio)
 {
request_queue_t *q;
-   sector_t maxsector;
sector_t old_sector;
int ret, nr_sectors = bio_sectors(bio);
dev_t old_dev;
 
might_sleep();
-   /* Test device or partition size, when known. */
-   maxsector = bio-bi_bdev-bd_inode-i_size  9;
-   if (maxsector) {
-   sector_t sector = bio-bi_sector;
 
-   if (maxsector  nr_sectors || maxsector - nr_sectors  sector) {
-   /*
-* This may well happen - the kernel calls bread()
-* without checking the size of the device, e.g., when
-* mounting a device.
-*/
-   handle_bad_sector(bio);
-   goto end_io;
-   }
-   }
+   if (bio_check_eod(bio, nr_sectors))
+   goto end_io;
 
/*
 * Resolve the mapping until finished. (drivers are
@@ -3197,21 +3213,8 @@ end_io:
old_sector = bio-bi_sector;
old_dev = bio-bi_bdev-bd_dev;
 
-   maxsector = bio-bi_bdev-bd_inode-i_size  9;
-   if (maxsector) {
-   sector_t sector = bio-bi_sector;
-
-   if (maxsector  nr_sectors ||
-   maxsector - nr_sectors  sector) {
-   /*
-* This may well happen - partitions are not
-* checked to make sure they are within the size
-* of the whole device.
-*/
-   handle_bad_sector(bio);
-   goto end_io;
-   }
-   }
+   if (bio_check_eod(bio, nr_sectors))
+   goto end_io;
 
ret = q-make_request_fn(q, bio);
} while (ret);
-
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] block: factor out bio_check_eod()

2007-07-18 Thread Jens Axboe
On Wed, Jul 18 2007, Tejun Heo wrote:
 End of device check is done twice in __generic_make_request() and it's
 fully inlined each time.  Factor out bio_check_eod().

Tejun, yeah I should seperate the cleanups and put them in the upstream
branch. Will do so and add your signed-off to both of them.

-- 
Jens Axboe

-
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] block: factor out bio_check_eod()

2007-07-18 Thread Tejun Heo
Jens Axboe wrote:
 On Wed, Jul 18 2007, Tejun Heo wrote:
 End of device check is done twice in __generic_make_request() and it's
 fully inlined each time.  Factor out bio_check_eod().
 
 Tejun, yeah I should seperate the cleanups and put them in the upstream
 branch. Will do so and add your signed-off to both of them.
 

Would they be different from the one I just posted?  No big deal either
way.  I'm just basing the zero-length barrier on top of these patches.
Oh well, the changes are trivial anyway.

-- 
tejun
-
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] block: factor out bio_check_eod()

2007-07-18 Thread Jens Axboe
On Wed, Jul 18 2007, Tejun Heo wrote:
 Jens Axboe wrote:
  On Wed, Jul 18 2007, Tejun Heo wrote:
  End of device check is done twice in __generic_make_request() and it's
  fully inlined each time.  Factor out bio_check_eod().
  
  Tejun, yeah I should seperate the cleanups and put them in the upstream
  branch. Will do so and add your signed-off to both of them.
  
 
 Would they be different from the one I just posted?  No big deal either
 way.  I'm just basing the zero-length barrier on top of these patches.
 Oh well, the changes are trivial anyway.

This one ended up being the same, but in the first one you missed some
of the cleanups. I ended up splitting the patch some more though, see
the series:

http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=barrier

-- 
Jens Axboe

-
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] block: factor out bio_check_eod()

2007-07-18 Thread Tejun Heo
Jens Axboe wrote:
 On Wed, Jul 18 2007, Tejun Heo wrote:
 Jens Axboe wrote:
 On Wed, Jul 18 2007, Tejun Heo wrote:
 End of device check is done twice in __generic_make_request() and it's
 fully inlined each time.  Factor out bio_check_eod().
 Tejun, yeah I should seperate the cleanups and put them in the upstream
 branch. Will do so and add your signed-off to both of them.

 Would they be different from the one I just posted?  No big deal either
 way.  I'm just basing the zero-length barrier on top of these patches.
 Oh well, the changes are trivial anyway.
 
 This one ended up being the same, but in the first one you missed some
 of the cleanups. I ended up splitting the patch some more though, see
 the series:
 
 http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=barrier

Alright, will base on 662d5c5e6afb79d05db5563205b809c0de530286.  Thanks.

-- 
tejun
-
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] block: factor out bio_check_eod()

2007-07-18 Thread Tejun Heo
Jens Axboe wrote:
 On Wed, Jul 18 2007, Tejun Heo wrote:
 Jens Axboe wrote:
 On Wed, Jul 18 2007, Tejun Heo wrote:
 Jens Axboe wrote:
 On Wed, Jul 18 2007, Tejun Heo wrote:
 End of device check is done twice in __generic_make_request() and it's
 fully inlined each time.  Factor out bio_check_eod().
 Tejun, yeah I should seperate the cleanups and put them in the upstream
 branch. Will do so and add your signed-off to both of them.

 Would they be different from the one I just posted?  No big deal either
 way.  I'm just basing the zero-length barrier on top of these patches.
 Oh well, the changes are trivial anyway.
 This one ended up being the same, but in the first one you missed some
 of the cleanups. I ended up splitting the patch some more though, see
 the series:

 http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=barrier
 Alright, will base on 662d5c5e6afb79d05db5563205b809c0de530286.  Thanks.
 
 1781c6a39fb6e31836557618c4505f5f7bc61605, no? Unless you want to rewrite
 it completely :-)

I think I'll start from 662d5c5e and steal most parts from 1781c6a3.  I
like stealing, you know. :-) I think 1781c6a3 also can use splitting -
zero length barrier implementation and issue_flush conversion.

Anyways, how do I pull from git.kernel.dk?
git://git.kernel.dk/linux-2.6-block.git gives me connection reset by server.

Thanks.

-- 
tejun
-
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] block: factor out bio_check_eod()

2007-07-18 Thread Jens Axboe
On Wed, Jul 18 2007, Tejun Heo wrote:
 Jens Axboe wrote:
  On Wed, Jul 18 2007, Tejun Heo wrote:
  Jens Axboe wrote:
  On Wed, Jul 18 2007, Tejun Heo wrote:
  Jens Axboe wrote:
  On Wed, Jul 18 2007, Tejun Heo wrote:
  End of device check is done twice in __generic_make_request() and it's
  fully inlined each time.  Factor out bio_check_eod().
  Tejun, yeah I should seperate the cleanups and put them in the upstream
  branch. Will do so and add your signed-off to both of them.
 
  Would they be different from the one I just posted?  No big deal either
  way.  I'm just basing the zero-length barrier on top of these patches.
  Oh well, the changes are trivial anyway.
  This one ended up being the same, but in the first one you missed some
  of the cleanups. I ended up splitting the patch some more though, see
  the series:
 
  http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=barrier
  Alright, will base on 662d5c5e6afb79d05db5563205b809c0de530286.  Thanks.
  
  1781c6a39fb6e31836557618c4505f5f7bc61605, no? Unless you want to rewrite
  it completely :-)
 
 I think I'll start from 662d5c5e and steal most parts from 1781c6a3.  I
 like stealing, you know. :-) I think 1781c6a3 also can use splitting -
 zero length barrier implementation and issue_flush conversion.

Yes that's true, I could split that in two as well. Will do so!

 Anyways, how do I pull from git.kernel.dk?
 git://git.kernel.dk/linux-2.6-block.git gives me connection reset by server.

git://git.kernel.dk/data/git/linux-2.6-block.git

somewhat annoying, I'll see if I can prefix it with git-daemon in the
future.

-- 
Jens Axboe

-
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] block: factor out bio_check_eod()

2007-07-18 Thread Jens Axboe
On Wed, Jul 18 2007, Jens Axboe wrote:
 On Wed, Jul 18 2007, Tejun Heo wrote:
  Jens Axboe wrote:
   On Wed, Jul 18 2007, Tejun Heo wrote:
   Jens Axboe wrote:
   On Wed, Jul 18 2007, Tejun Heo wrote:
   Jens Axboe wrote:
   On Wed, Jul 18 2007, Tejun Heo wrote:
   End of device check is done twice in __generic_make_request() and 
   it's
   fully inlined each time.  Factor out bio_check_eod().
   Tejun, yeah I should seperate the cleanups and put them in the 
   upstream
   branch. Will do so and add your signed-off to both of them.
  
   Would they be different from the one I just posted?  No big deal either
   way.  I'm just basing the zero-length barrier on top of these patches.
   Oh well, the changes are trivial anyway.
   This one ended up being the same, but in the first one you missed some
   of the cleanups. I ended up splitting the patch some more though, see
   the series:
  
   http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=barrier
   Alright, will base on 662d5c5e6afb79d05db5563205b809c0de530286.  Thanks.
   
   1781c6a39fb6e31836557618c4505f5f7bc61605, no? Unless you want to rewrite
   it completely :-)
  
  I think I'll start from 662d5c5e and steal most parts from 1781c6a3.  I
  like stealing, you know. :-) I think 1781c6a3 also can use splitting -
  zero length barrier implementation and issue_flush conversion.
 
 Yes that's true, I could split that in two as well. Will do so!
 
  Anyways, how do I pull from git.kernel.dk?
  git://git.kernel.dk/linux-2.6-block.git gives me connection reset by server.
 
 git://git.kernel.dk/data/git/linux-2.6-block.git
 
 somewhat annoying, I'll see if I can prefix it with git-daemon in the
 future.

OK, now skip the /data/git/ stuff and just use

git://git.kernel.dk/linux-2.6-block.git

:-)

-- 
Jens Axboe

-
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] block: factor out bio_check_eod()

2007-07-18 Thread Jens Axboe
On Wed, Jul 18 2007, Jens Axboe wrote:
 On Wed, Jul 18 2007, Tejun Heo wrote:
  Jens Axboe wrote:
   On Wed, Jul 18 2007, Tejun Heo wrote:
   Jens Axboe wrote:
   On Wed, Jul 18 2007, Tejun Heo wrote:
   Jens Axboe wrote:
   On Wed, Jul 18 2007, Tejun Heo wrote:
   End of device check is done twice in __generic_make_request() and 
   it's
   fully inlined each time.  Factor out bio_check_eod().
   Tejun, yeah I should seperate the cleanups and put them in the 
   upstream
   branch. Will do so and add your signed-off to both of them.
  
   Would they be different from the one I just posted?  No big deal either
   way.  I'm just basing the zero-length barrier on top of these patches.
   Oh well, the changes are trivial anyway.
   This one ended up being the same, but in the first one you missed some
   of the cleanups. I ended up splitting the patch some more though, see
   the series:
  
   http://git.kernel.dk/?p=linux-2.6-block.git;a=shortlog;h=barrier
   Alright, will base on 662d5c5e6afb79d05db5563205b809c0de530286.  Thanks.
   
   1781c6a39fb6e31836557618c4505f5f7bc61605, no? Unless you want to rewrite
   it completely :-)
  
  I think I'll start from 662d5c5e and steal most parts from 1781c6a3.  I
  like stealing, you know. :-) I think 1781c6a3 also can use splitting -
  zero length barrier implementation and issue_flush conversion.
 
 Yes that's true, I could split that in two as well. Will do so!

Done, result in the same location.

-- 
Jens Axboe

-
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] block: factor out bio_check_eod()

2007-07-18 Thread Tejun Heo
Jens Axboe wrote:
 somewhat annoying, I'll see if I can prefix it with git-daemon in the
 future.
 
 OK, now skip the /data/git/ stuff and just use
 
 git://git.kernel.dk/linux-2.6-block.git

Alright, it works like a charm now.  Thanks.

-- 
tejun
-
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 RFC] extent mapped page cache

2007-07-18 Thread Chris Mason
On Thu, 12 Jul 2007 00:00:28 -0700
Daniel Phillips [EMAIL PROTECTED] wrote:

 On Tuesday 10 July 2007 14:03, Chris Mason wrote:
  This patch aims to demonstrate one way to replace buffer heads with
  a few extent trees...
 
 Hi Chris,
 
 Quite terse commentary on algorithms and data structures, but I
 suppose that is not a problem because Jon has a whole week to reverse
 engineer it for us.
 
 What did you have in mind for subpages?
 

This partially depends on input here.  The goal is to have one
interface that works for subpages, highmem and superpages, and for
the FS maintainers to not care if the mappings come magically from
clameter's work or vmap or whatever.

Given the whole extent based theme, I plan on something like this:

struct extent_ptr {
char *ptr;
some way to indicate size and type of map
struct page pages[];
};

struct extent_ptr *alloc_extent_ptr(struct extent_map_tree *tree,
u64 start, u64 end);
void free_extent_ptr(struct extent_map_tree *tree,
 struct extent_ptr *ptr);

And then some calls along the lines of kmap/kunmap that gives you a
pointer you can use for accessing the ram.  read/write calls would also
be fine by me, but harder to convert filesystems to use.

The struct extent_ptr would increase the ref count on the pages, but
the pages would have no back pointers to it.  All
dirty/locked/writeback state would go in the extent state tree and would
not be stored in the struct extent_ptr.  

The idea is to make a simple mapping entity, and not complicate it
by storing FS specific state in there. It could be variably sized to
hold an array of pages, and allocated via kmap.

-chris
-
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


new ext4 build warnings

2007-07-18 Thread Jeff Garzik

It seems jbd_debug() might need modification:

fs/ext4/inode.c: In function ‘ext4_write_inode’:
fs/ext4/inode.c:2906: warning: comparison is always true due to limited 
range of data type


fs/jbd2/recovery.c: In function ‘jbd2_journal_recover’:
fs/jbd2/recovery.c:254: warning: comparison is always true due to 
limited range of data type
fs/jbd2/recovery.c:257: warning: comparison is always true due to 
limited range of data type


fs/jbd2/recovery.c: In function ‘jbd2_journal_skip_recovery’:
fs/jbd2/recovery.c:301: warning: comparison is always true due to 
limited range of data type


I'm surprised this was not noticed in a test build before pushing upstream.

Jeff


-
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: lease and lock patches

2007-07-18 Thread J. Bruce Fields
Would it still be possible to merge this for 2.6.23?  They've been
through linux-fsdevel, Christoph has taken a pass through them, and I
don't know of any unaddressed problems.

--b.

On Fri, Jul 13, 2007 at 07:34:57PM -0400, J. Bruce Fields wrote:
 Please pull from the 'for-linus' branch at
 
   git://linux-nfs.org/~bfields/linux.git for-linus
 
 for a series of patches which add a setlease() file method.  The
 longer-term goal is to allow cluster and network filesystems to give out
 consistent leases when possible, in particular to allow nfsd to give out
 delegations on cluster filesystems.  For now, though, we're using this
 just to disallow leases selectively on certain filesystems (nfs and gfs2
 for now) where they don't make sense.
 
 Also includes some minor locks.c cleanup.
 
 J. Bruce Fields (9):
   locks: convert an -EINVAL return to a BUG
   locks: clean up lease_alloc()
   locks: share more common lease code
   locks: rename lease functions to reflect locks.c conventions
   locks: provide a file lease method enabling cluster-coherent leases
   locks: export setlease to filesystems
   nfs: disable leases over NFS
   locks: make posix_test_lock() interface more consistent
   locks: fix vfs_test_lock() comment
 
 Marc Eshel (1):
   gfs2: stop giving out non-cluster-coherent leases
 
 david m. richter (1):
   leases: minor break_lease() comment clarification
 
  fs/gfs2/ops_file.c  |   24 +++
  fs/locks.c  |  112 ++
  fs/nfs/file.c   |   16 +++-
  fs/nfsd/nfs4state.c |   10 ++--
  include/linux/fs.h  |4 +-
  5 files changed, 105 insertions(+), 61 deletions(-)
 -
 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
-
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: new ext4 build warnings

2007-07-18 Thread Mingming Cao
On Wed, 2007-07-18 at 17:37 -0400, Jeff Garzik wrote:
 It seems jbd_debug() might need modification:
 
 fs/ext4/inode.c: In function ‘ext4_write_inode’:
 fs/ext4/inode.c:2906: warning: comparison is always true due to limited 
 range of data type
 
 fs/jbd2/recovery.c: In function ‘jbd2_journal_recover’:
 fs/jbd2/recovery.c:254: warning: comparison is always true due to 
 limited range of data type
 fs/jbd2/recovery.c:257: warning: comparison is always true due to 
 limited range of data type
 
 fs/jbd2/recovery.c: In function ‘jbd2_journal_skip_recovery’:
 fs/jbd2/recovery.c:301: warning: comparison is always true due to 
 limited range of data type
 
 I'm surprised this was not noticed in a test build before pushing upstream.
 

Hmm,  I am not sure what happened. I get the compile warning on linus
latest git tree, but could not get the same compile warning on Ted's
ext4 git tree. 

In both build CONFIG_JBD2_DEBUG and DEBUG_FS is enabled.

Mingming


-
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] fix ext4/JBD2 build warnings

2007-07-18 Thread Mingming Cao
On Wed, 2007-07-18 at 17:37 -0400, Jeff Garzik wrote:
 It seems jbd_debug() might need modification:
 
Looking at the current linus-git tree jbd_debug() define in
include/linux/jbd2.h

extern u8 journal_enable_debug;

#define jbd_debug(n, f, a...)   \
do {\
if ((n) = journal_enable_debug) {  \
printk (KERN_DEBUG (%s, %d): %s: ,\
__FILE__, __LINE__, __FUNCTION__);  \
printk (f, ## a);   \
}   \
} while (0)
 fs/ext4/inode.c: In function ‘ext4_write_inode’:
 fs/ext4/inode.c:2906: warning: comparison is always true due to limited 
 range of data type
 
 fs/jbd2/recovery.c: In function ‘jbd2_journal_recover’:
 fs/jbd2/recovery.c:254: warning: comparison is always true due to 
 limited range of data type
 fs/jbd2/recovery.c:257: warning: comparison is always true due to 
 limited range of data type
 
 fs/jbd2/recovery.c: In function ‘jbd2_journal_skip_recovery’:
 fs/jbd2/recovery.c:301: warning: comparison is always true due to 
 limited range of data type
 
Noticed all warnings are occurs when the debug level is 0. Then found
the jbd2: Move jbd2-debug file to debugfs patch
http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=0f49d5d019afa4e94253bfc92f0daca3badb990b

changed the jbd2_journal_enable_debug from int type to u8, makes the
jbd_debug comparision is always true when the debugging level is 0. Thus
the compile warning occurs. 

Thought about changing the jbd2_journal_enable_debug data type back to
int, but can't, because the jbd2-debug is moved to debug fs, where
calling debugfs_create_u8() to create the debugfs entry needs the value
to be u8 type.

Even if we changed the data type back to int, the code is still buggy,
kernel should not print jbd2 debug message if the
jbd2_journal_enable_debug is set to 0. But this is not the case.

The fix is change the level of debugging to 1. The same should fixed in
ext3/JBD, but currently ext3 jbd-debug via /proc fs is broken, so we
probably should fix it all together.


Signed-off-by: Mingming Cao [EMAIL PROTECTED]

---
 fs/ext4/inode.c|2 +-
 fs/jbd2/recovery.c |6 +++---
 2 files changed, 4 insertions(+), 4 deletions(-)

Index: linux-2.6.22/fs/ext4/inode.c
===
--- linux-2.6.22.orig/fs/ext4/inode.c   2007-07-18 19:23:58.0 -0700
+++ linux-2.6.22/fs/ext4/inode.c2007-07-18 19:25:08.0 -0700
@@ -2903,7 +2903,7 @@ int ext4_write_inode(struct inode *inode
return 0;
 
if (ext4_journal_current_handle()) {
-   jbd_debug(0, called recursively, non-PF_MEMALLOC!\n);
+   jbd_debug(1, called recursively, non-PF_MEMALLOC!\n);
dump_stack();
return -EIO;
}
Index: linux-2.6.22/fs/jbd2/recovery.c
===
--- linux-2.6.22.orig/fs/jbd2/recovery.c2007-07-18 19:25:51.0 
-0700
+++ linux-2.6.22/fs/jbd2/recovery.c 2007-07-18 19:26:13.0 -0700
@@ -251,10 +251,10 @@ int jbd2_journal_recover(journal_t *jour
if (!err)
err = do_one_pass(journal, info, PASS_REPLAY);
 
-   jbd_debug(0, JBD: recovery, exit status %d, 
+   jbd_debug(1, JBD: recovery, exit status %d, 
  recovered transactions %u to %u\n,
  err, info.start_transaction, info.end_transaction);
-   jbd_debug(0, JBD: Replayed %d and revoked %d/%d blocks\n,
+   jbd_debug(1, JBD: Replayed %d and revoked %d/%d blocks\n,
  info.nr_replays, info.nr_revoke_hits, info.nr_revokes);
 
/* Restart the log at the next transaction ID, thus invalidating
@@ -298,7 +298,7 @@ int jbd2_journal_skip_recovery(journal_t
 #ifdef CONFIG_JBD2_DEBUG
int dropped = info.end_transaction - 
be32_to_cpu(sb-s_sequence);
 #endif
-   jbd_debug(0,
+   jbd_debug(1,
  JBD: ignoring %d transaction%s from the journal.\n,
  dropped, (dropped == 1) ?  : s);
journal-j_transaction_sequence = ++info.end_transaction;



-
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][TAKE8] manpage for fallocate

2007-07-18 Thread Mark Fasheh
On Sat, Jul 14, 2007 at 12:16:25AM +0530, Amit K. Arora wrote:
 After a successful call, subsequent writes are guaranteed not to fail because
 of lack of disk space.
  
If a write to an unwritten region requires a node split, that could result
in the allocation of new meta data which obviously could fail if the disk is
truly full.

Granted that's unlikely to happen but maybe we should be conservative and
say something like:

After a successful call, subsequent writes are guaranteed to never require
allocation of file data. ?
--Mark

--
Mark Fasheh
Senior Software Developer, Oracle
[EMAIL PROTECTED]
-
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][TAKE8] manpage for fallocate

2007-07-18 Thread David Chinner
On Wed, Jul 18, 2007 at 08:41:55PM -0700, Mark Fasheh wrote:
 On Sat, Jul 14, 2007 at 12:16:25AM +0530, Amit K. Arora wrote:
  After a successful call, subsequent writes are guaranteed not to fail 
  because
  of lack of disk space.
   
 If a write to an unwritten region requires a node split, that could result
 in the allocation of new meta data which obviously could fail if the disk is
 truly full.

% git-log 84e1e99f112dead8f9ba036c02d24a9f5ce7f544 |head -10
commit 84e1e99f112dead8f9ba036c02d24a9f5ce7f544
Author: David Chinner [EMAIL PROTECTED]
Date:   Mon Jun 18 16:50:27 2007 +1000

[XFS] Prevent ENOSPC from aborting transactions that need to succeed

During delayed allocation extent conversion or unwritten extent
conversion, we need to reserve some blocks for transactions reservations.
We need to reserve these blocks in case a btree split occurs and we need
to allocate some blocks.

--

IOWs, XFS didn't provide this guarantee until about a month ago

 Granted that's unlikely to happen but maybe we should be conservative and
 say something like:
 
 After a successful call, subsequent writes are guaranteed to never require
 allocation of file data. ?

Well, the above phrasing is taken directly from the posix_fallocate() man
page, and it is intended that sys_fallocate() is used to implement
posix_fallocate(). In that case, the semantics we have to provide are
writes are guaranteed not to fail due to lack of disk space.

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