linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/4] Add extended attributes to ext2/3
@ 2002-10-15  3:33 tytso
  2002-10-15  4:46 ` Andrew Morton
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: tytso @ 2002-10-15  3:33 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel

Linus,

	Enclosed please find patches to add extended attribute support
to the ext2 and ext3 filesystems.  It is a port of Andreas Gruenbacher's
patches, which have been quite well tested at this point.  Full support
for extended attributes have been in e2fsprogs for quite some time.  In
addition, if CONFIG_EXT[23]_FS_ATTR is not enabled, the code path
changes are quite minimal, and hence this should be a very low-risk
patch.  Please apply.

These patches are a prerequisite for the port of Andreas Gruenbacher's
ACL patches, which will follow shortly.

This first patch creates a generic interface for registering caches with
the VM subsystem so that they can react appropriately to memory
pressure.

						- Ted


# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
#
# cache_def.h               |   15 +++++++++++++++
# include/linux/cache_def.h |   15 +++++++++++++++
# kernel/ksyms.c            |    3 +++
# mm/vmscan.c               |   35 +++++++++++++++++++++++++++++++++++
# 4 files changed, 68 insertions(+)
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/10/14	tytso@snap.thunk.org	1.783
# Port of (updated) 0.8.50 cache-def patch.  (Fixed to take the BKL where necessary)
# 
# --------------------------------------------
#
diff -Nru a/cache_def.h b/cache_def.h
--- /dev/null	Wed Dec 31 16:00:00 1969
+++ b/cache_def.h	Mon Oct 14 23:20:54 2002
@@ -0,0 +1,15 @@
+/*
+ * linux/cache_def.h
+ * Handling of caches defined in drivers, filesystems, ...
+ *
+ * Copyright (C) 2002 by Andreas Gruenbacher, <a.gruenbacher@computer.org>
+ */
+
+struct cache_definition {
+	const char *name;
+	void (*shrink)(int, unsigned int);
+	struct list_head link;
+};
+
+extern void register_cache(struct cache_definition *);
+extern void unregister_cache(struct cache_definition *);
diff -Nru a/include/linux/cache_def.h b/include/linux/cache_def.h
--- /dev/null	Wed Dec 31 16:00:00 1969
+++ b/include/linux/cache_def.h	Mon Oct 14 23:20:54 2002
@@ -0,0 +1,15 @@
+/*
+ * linux/cache_def.h
+ * Handling of caches defined in drivers, filesystems, ...
+ *
+ * Copyright (C) 2002 by Andreas Gruenbacher, <a.gruenbacher@computer.org>
+ */
+
+struct cache_definition {
+	const char *name;
+	void (*shrink)(int, unsigned int);
+	struct list_head link;
+};
+
+extern void register_cache(struct cache_definition *);
+extern void unregister_cache(struct cache_definition *);
diff -Nru a/kernel/ksyms.c b/kernel/ksyms.c
--- a/kernel/ksyms.c	Mon Oct 14 23:20:54 2002
+++ b/kernel/ksyms.c	Mon Oct 14 23:20:54 2002
@@ -31,6 +31,7 @@
 #include <linux/genhd.h>
 #include <linux/blkpg.h>
 #include <linux/swap.h>
+#include <linux/cache_def.h>
 #include <linux/ctype.h>
 #include <linux/file.h>
 #include <linux/iobuf.h>
@@ -103,6 +104,8 @@
 EXPORT_SYMBOL(kmem_cache_alloc);
 EXPORT_SYMBOL(kmem_cache_free);
 EXPORT_SYMBOL(kmem_cache_size);
+EXPORT_SYMBOL(register_cache);
+EXPORT_SYMBOL(unregister_cache);
 EXPORT_SYMBOL(kmalloc);
 EXPORT_SYMBOL(kfree);
 EXPORT_SYMBOL(vfree);
diff -Nru a/mm/vmscan.c b/mm/vmscan.c
--- a/mm/vmscan.c	Mon Oct 14 23:20:54 2002
+++ b/mm/vmscan.c	Mon Oct 14 23:20:54 2002
@@ -15,6 +15,7 @@
 #include <linux/slab.h>
 #include <linux/kernel_stat.h>
 #include <linux/swap.h>
+#include <linux/cache_def.h>
 #include <linux/pagemap.h>
 #include <linux/init.h>
 #include <linux/highmem.h>
@@ -76,6 +77,39 @@
 #define shrink_dqcache_memory(ratio, gfp_mask) do { } while (0)
 #endif
 
+static DECLARE_MUTEX(other_caches_sem);
+static LIST_HEAD(cache_definitions);
+
+void register_cache(struct cache_definition *cache)
+{
+	down(&other_caches_sem);
+	list_add(&cache->link, &cache_definitions);
+	up(&other_caches_sem);
+}
+
+void unregister_cache(struct cache_definition *cache)
+{
+	down(&other_caches_sem);
+	list_del(&cache->link);
+	up(&other_caches_sem);
+}
+
+static void shrink_other_caches(int ratio, int gfp_mask)
+{
+	struct list_head *p;
+
+	down(&other_caches_sem);
+	p = cache_definitions.prev;
+	while (p != &cache_definitions) {
+		struct cache_definition *cache =
+			list_entry(p, struct cache_definition, link);
+
+		cache->shrink(ratio, gfp_mask);
+		p = p->prev;
+	}
+	up(&other_caches_sem);
+}
+
 /* Must be called with page's pte_chain_lock held. */
 static inline int page_mapping_inuse(struct page * page)
 {
@@ -594,6 +628,7 @@
 	shrink_dcache_memory(shrink_ratio, gfp_mask);
 	shrink_icache_memory(shrink_ratio, gfp_mask);
 	shrink_dqcache_memory(shrink_ratio, gfp_mask);
+	shrink_other_caches(shrink_ratio, gfp_mask);
 }
 
 /*

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15  3:33 [PATCH 1/4] Add extended attributes to ext2/3 tytso
@ 2002-10-15  4:46 ` Andrew Morton
  2002-10-15  5:04   ` Theodore Ts'o
  2002-10-15  5:31   ` Andreas Dilger
  2002-10-15 10:26 ` Olaf Dietsche
  2002-10-15 11:58 ` Christoph Hellwig
  2 siblings, 2 replies; 18+ messages in thread
From: Andrew Morton @ 2002-10-15  4:46 UTC (permalink / raw)
  To: tytso; +Cc: torvalds, linux-kernel

tytso@mit.edu wrote:
> 
>...
> This first patch creates a generic interface for registering caches with
> the VM subsystem so that they can react appropriately to memory
> pressure.
> 

Seems our patches passed in the night - Linus already has one of
those APIs.

I've converted xattr to use the set_shrinker/remove_shrinker API.
I'd appreciate it if you could pass an eye over that and give it
a test.  I'll roll it into 2.5.42-mm3 if that makes it easier.

btw, gcc-2.91.66 is saying:

fs/ext2/xattr.c: In function `ext2_xattr_set':
fs/ext2/xattr.c:612: warning: `block' might be used uninitialized in this function

But the code is:

                int block = EXT2_I(inode)->i_file_acl;

which is rather bizarre.  Never seen it do that before.

The patching order becomes:

xattr 2/4
xattr-shrinker
xattr 3/4
xattr 4/4


 fs/mbcache.c |   57 ++++++++++++++++++++++++++++++++-------------------------
 1 files changed, 32 insertions(+), 25 deletions(-)

--- 2.5.42/fs/mbcache.c~xattr-shrinker	Mon Oct 14 21:17:59 2002
+++ 2.5.42-akpm/fs/mbcache.c	Mon Oct 14 21:30:25 2002
@@ -29,9 +29,9 @@
 #include <linux/module.h>
 
 #include <linux/fs.h>
+#include <linux/mm.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
-#include <linux/cache_def.h>
 #include <linux/version.h>
 #include <linux/init.h>
 #include <linux/mbcache.h>
@@ -85,6 +85,7 @@ EXPORT_SYMBOL(mb_cache_entry_find_next);
 static LIST_HEAD(mb_cache_list);
 static LIST_HEAD(mb_cache_lru_list);
 static spinlock_t mb_cache_spinlock = SPIN_LOCK_UNLOCKED;
+static struct shrinker *mb_shrinker;
 
 static inline void
 mb_cache_lock(void)
@@ -112,14 +113,7 @@ mb_cache_indexes(struct mb_cache *cache)
  * What the mbcache registers as to get shrunk dynamically.
  */
 
-static void
-mb_cache_memory_pressure(int priority, unsigned int gfp_mask);
-
-static struct cache_definition mb_cache_definition = {
-	"mb_cache",
-	mb_cache_memory_pressure
-};
-
+static int mb_cache_shrink_fn(int nr_to_scan, unsigned int gfp_mask);
 
 static inline void
 __mb_cache_entry_takeout_lru(struct mb_cache_entry *ce)
@@ -226,16 +220,18 @@ __mb_cache_entry_release_unlock(struct m
 
 
 /*
- * mb_cache_memory_pressure()  memory pressure callback
+ * mb_cache_shrink_fn()  memory pressure callback
  *
  * This function is called by the kernel memory management when memory
  * gets low.
  *
- * @priority: Amount by which to shrink the cache (0 = highes priority)
+ * @nr_to_scan: Number of objects to scan
  * @gfp_mask: (ignored)
+ *
+ * Returns the number of objects which are present in the cache.
  */
-static void
-mb_cache_memory_pressure(int priority, unsigned int gfp_mask)
+static int
+mb_cache_shrink_fn(int nr_to_scan, unsigned int gfp_mask)
 {
 	LIST_HEAD(free_list);
 	struct list_head *l;
@@ -249,11 +245,12 @@ mb_cache_memory_pressure(int priority, u
 			  atomic_read(&cache->c_entry_count));
 		count += atomic_read(&cache->c_entry_count);
 	}
-	mb_debug("trying to free %d of %d entries",
-		  count / (priority ? priority : 1), count);
-	if (priority)
-		count /= priority;
-	while (count && !list_empty(&mb_cache_lru_list)) {
+	mb_debug("trying to free %d entries", nr_to_scan);
+	if (nr_to_scan == 0) {
+		mb_cache_unlock();
+		goto out;
+	}
+	while (nr_to_scan && !list_empty(&mb_cache_lru_list)) {
 		struct mb_cache_entry *ce =
 			list_entry(mb_cache_lru_list.prev,
 				   struct mb_cache_entry, e_lru_list);
@@ -261,7 +258,7 @@ mb_cache_memory_pressure(int priority, u
 		list_add(&ce->e_lru_list, &free_list);
 		if (__mb_cache_entry_is_linked(ce))
 			__mb_cache_entry_unlink(ce);
-		count--;
+		nr_to_scan--;
 	}
 	mb_cache_unlock();
 	l = free_list.prev;
@@ -270,9 +267,11 @@ mb_cache_memory_pressure(int priority, u
 			struct mb_cache_entry, e_lru_list);
 		l = l->prev;
 		__mb_cache_entry_forget(ce);
+		count--;
 	}
-	if (count)
-		mb_debug("%d fewer entries freed", count);
+out:
+	mb_debug("%d remaining entries ", count);
+	return count;
 }
 
 
@@ -342,8 +341,14 @@ mb_cache_create(const char *name, struct
 		goto fail;
 
 	mb_cache_lock();
-	if (list_empty(&mb_cache_list))
-		register_cache(&mb_cache_definition);
+	if (list_empty(&mb_cache_list)) {
+		if (mb_shrinker) {
+			printk(KERN_ERR "%s: already have a shrinker!\n",
+					__FUNCTION__);
+			remove_shrinker(mb_shrinker);
+		}
+		mb_shrinker = set_shrinker(DEFAULT_SEEKS, mb_cache_shrink_fn);
+	}
 	list_add(&cache->c_cache_list, &mb_cache_list);
 	mb_cache_unlock();
 	return cache;
@@ -429,8 +434,10 @@ mb_cache_destroy(struct mb_cache *cache)
 		}
 	}
 	list_del(&cache->c_cache_list);
-	if (list_empty(&mb_cache_list))
-		unregister_cache(&mb_cache_definition);
+	if (list_empty(&mb_cache_list) && mb_shrinker) {
+		remove_shrinker(mb_shrinker);
+		mb_shrinker = 0;
+	}
 	mb_cache_unlock();
 
 	l = free_list.prev;

.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15  4:46 ` Andrew Morton
@ 2002-10-15  5:04   ` Theodore Ts'o
  2002-10-15  5:31   ` Andreas Dilger
  1 sibling, 0 replies; 18+ messages in thread
From: Theodore Ts'o @ 2002-10-15  5:04 UTC (permalink / raw)
  To: Andrew Morton; +Cc: torvalds, linux-kernel

On Mon, Oct 14, 2002 at 09:46:29PM -0700, Andrew Morton wrote:
> tytso@mit.edu wrote:
> > 
> >...
> > This first patch creates a generic interface for registering caches with
> > the VM subsystem so that they can react appropriately to memory
> > pressure.
> > 
> 
> Seems our patches passed in the night - Linus already has one of
> those APIs.
> 
> I've converted xattr to use the set_shrinker/remove_shrinker API.

Great!  I checked bk://linux.bkbits.net/linux-2.5 before I started
assembling this patch set, but I missed the fact that the shrinker
changes had made it in.

> I'd appreciate it if you could pass an eye over that and give it
> a test.

Will do....

							- Ted

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15  4:46 ` Andrew Morton
  2002-10-15  5:04   ` Theodore Ts'o
@ 2002-10-15  5:31   ` Andreas Dilger
  2002-10-15 13:05     ` Theodore Ts'o
  1 sibling, 1 reply; 18+ messages in thread
From: Andreas Dilger @ 2002-10-15  5:31 UTC (permalink / raw)
  To: Andrew Morton; +Cc: tytso, linux-kernel

On Oct 14, 2002  21:46 -0700, Andrew Morton wrote:
> btw, gcc-2.91.66 is saying:
> 
> fs/ext2/xattr.c: In function `ext2_xattr_set':
> fs/ext2/xattr.c:612: warning: `block' might be used uninitialized in this function
> 
> But the code is:
> 
>                 int block = EXT2_I(inode)->i_file_acl;
> 
> which is rather bizarre.  Never seen it do that before.

There's a "goto bad_block" in that function, and it enters _after_
block is set (block is a local variable), so it is impossible for
it to have a valid value.

Cheers, Andreas
--
Andreas Dilger
http://www-mddsp.enel.ucalgary.ca/People/adilger/
http://sourceforge.net/projects/ext2resize/


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15  3:33 [PATCH 1/4] Add extended attributes to ext2/3 tytso
  2002-10-15  4:46 ` Andrew Morton
@ 2002-10-15 10:26 ` Olaf Dietsche
  2002-10-15 11:58 ` Christoph Hellwig
  2 siblings, 0 replies; 18+ messages in thread
From: Olaf Dietsche @ 2002-10-15 10:26 UTC (permalink / raw)
  To: tytso; +Cc: torvalds, linux-kernel

tytso@mit.edu writes:

> 	Enclosed please find patches to add extended attribute support
> to the ext2 and ext3 filesystems.  It is a port of Andreas Gruenbacher's
> patches, which have been quite well tested at this point.  Full support
> for extended attributes have been in e2fsprogs for quite some time.  In
> addition, if CONFIG_EXT[23]_FS_ATTR is not enabled, the code path
> changes are quite minimal, and hence this should be a very low-risk
> patch.  Please apply.

Maybe this is a stupid question. Does this mean, fs capabilities will
come too?

Regards, Olaf.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15  3:33 [PATCH 1/4] Add extended attributes to ext2/3 tytso
  2002-10-15  4:46 ` Andrew Morton
  2002-10-15 10:26 ` Olaf Dietsche
@ 2002-10-15 11:58 ` Christoph Hellwig
  2002-10-15 13:15   ` Theodore Ts'o
  2 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2002-10-15 11:58 UTC (permalink / raw)
  To: tytso; +Cc: torvalds, linux-kernel

On Mon, Oct 14, 2002 at 11:33:04PM -0400, tytso@mit.edu wrote:
> Linus,
> 
> 	Enclosed please find patches to add extended attribute support
> to the ext2 and ext3 filesystems.  It is a port of Andreas Gruenbacher's
> patches, which have been quite well tested at this point.  Full support
> for extended attributes have been in e2fsprogs for quite some time.  In
> addition, if CONFIG_EXT[23]_FS_ATTR is not enabled, the code path
> changes are quite minimal, and hence this should be a very low-risk
> patch.  Please apply.
> 
> These patches are a prerequisite for the port of Andreas Gruenbacher's
> ACL patches, which will follow shortly.
> 
> This first patch creates a generic interface for registering caches with
> the VM subsystem so that they can react appropriately to memory
> pressure.

It doesn't look like you addressed any comments raised, i.e. my
complaints or sct's superblock fields.  I"ll start feeding some updates
to akpm to address a few issues, but I don't really have time to
care of all that.  


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15  5:31   ` Andreas Dilger
@ 2002-10-15 13:05     ` Theodore Ts'o
  0 siblings, 0 replies; 18+ messages in thread
From: Theodore Ts'o @ 2002-10-15 13:05 UTC (permalink / raw)
  To: Andrew Morton, linux-kernel

On Mon, Oct 14, 2002 at 11:31:18PM -0600, Andreas Dilger wrote:
> There's a "goto bad_block" in that function, and it enters _after_
> block is set (block is a local variable), so it is impossible for
> it to have a valid value.

Thanks, I'll fix that the xattr-ext2 and xattr-ext3 patches.

							- Ted

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15 11:58 ` Christoph Hellwig
@ 2002-10-15 13:15   ` Theodore Ts'o
  2002-10-15 15:25     ` Christoph Hellwig
                       ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Theodore Ts'o @ 2002-10-15 13:15 UTC (permalink / raw)
  To: Christoph Hellwig, torvalds, linux-kernel

On Tue, Oct 15, 2002 at 12:58:16PM +0100, Christoph Hellwig wrote:
> It doesn't look like you addressed any comments raised, i.e. my
> complaints or sct's superblock fields.  I"ll start feeding some updates
> to akpm to address a few issues, but I don't really have time to
> care of all that.  

Actually, I did, for those comments that made sense.  The fs/Config.in
logic has been cleaned up, as well as removing excess header files,
stray LINUX_VERSION_CODE #ifdef's that I had missed the first time
around, etc.

fs/mbcache.c is still there because it applies to both ext2 and ext3
filesystems, and so your suggestion of moving it into the ext2 and
ext3 directories would cause code duplication and maintenance
headaches.  It also *can* be used by other filesystems, as it is
written in a generic way.  The fs/Config.in only compiles it in if
necessary (i.e., if ext2/3 extended attribute is enabled) so it won't
cause code bloat for other filesystems if it is not needed.

The superblock fields are more of an issue with the posix acl changes
than for the extended attribute patches.  I had wanted to get the
extended attribute changes in first, since they stand alone, and so I
have fewer patches to juggle.

						- Ted

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15 13:15   ` Theodore Ts'o
@ 2002-10-15 15:25     ` Christoph Hellwig
  2002-10-15 15:31     ` Christoph Hellwig
                       ` (2 subsequent siblings)
  3 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2002-10-15 15:25 UTC (permalink / raw)
  To: Theodore Ts'o, Christoph Hellwig, torvalds, linux-kernel

On Tue, Oct 15, 2002 at 09:15:07AM -0400, Theodore Ts'o wrote:
> Actually, I did, for those comments that made sense.  The fs/Config.in
> logic has been cleaned up, as well as removing excess header files,
> stray LINUX_VERSION_CODE #ifdef's that I had missed the first time
> around, etc.
> 
> fs/mbcache.c is still there because it applies to both ext2 and ext3
> filesystems, and so your suggestion of moving it into the ext2 and
> ext3 directories would cause code duplication and maintenance
> headaches.  It also *can* be used by other filesystems, as it is
> written in a generic way.  The fs/Config.in only compiles it in if
> necessary (i.e., if ext2/3 extended attribute is enabled) so it won't
> cause code bloat for other filesystems if it is not needed.
> 
> The superblock fields are more of an issue with the posix acl changes
> than for the extended attribute patches.  I had wanted to get the
> extended attribute changes in first, since they stand alone, and so I
> have fewer patches to juggle.

Patch 1: Config.in (all against 2.5.42-mm3):

The xattr options must be dep_mbool to actually work, ext2 xattrs
should depends on ext2, not ext3


--- linux-2.5.42-mm3-plain/fs/Config.in	Tue Oct 15 17:05:08 2002
+++ linux-2.5.42-mm3/fs/Config.in	Tue Oct 15 16:03:45 2002
@@ -27,7 +27,7 @@
 dep_tristate 'BFS file system support (EXPERIMENTAL)' CONFIG_BFS_FS $CONFIG_EXPERIMENTAL
 
 tristate 'Ext3 journalling file system support' CONFIG_EXT3_FS
-dep_bool '  Ext3 extended attributes' CONFIG_EXT3_FS_XATTR $CONFIG_EXT3_FS
+dep_mbool '  Ext3 extended attributes' CONFIG_EXT3_FS_XATTR $CONFIG_EXT3_FS
 # CONFIG_JBD could be its own option (even modular), but until there are
 # other users than ext3, we will simply make it be the same as CONFIG_EXT3_FS
 # dep_tristate '  Journal Block Device support (JBD for ext3)' CONFIG_JBD $CONFIG_EXT3_FS
@@ -97,7 +97,7 @@
 tristate 'ROM file system support' CONFIG_ROMFS_FS
 
 tristate 'Second extended fs support' CONFIG_EXT2_FS
-dep_bool '  Ext2 extended attributes' CONFIG_EXT2_FS_XATTR $CONFIG_EXT3_FS
+dep_mbool '  Ext2 extended attributes' CONFIG_EXT2_FS_XATTR $CONFIG_EXT2_FS
 
 tristate 'System V/Xenix/V7/Coherent file system support' CONFIG_SYSV_FS
 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15 13:15   ` Theodore Ts'o
  2002-10-15 15:25     ` Christoph Hellwig
@ 2002-10-15 15:31     ` Christoph Hellwig
  2002-10-15 16:04       ` Andreas Dilger
  2002-10-15 16:41       ` Theodore Ts'o
  2002-10-15 15:33     ` Christoph Hellwig
  2002-10-15 15:34     ` Christoph Hellwig
  3 siblings, 2 replies; 18+ messages in thread
From: Christoph Hellwig @ 2002-10-15 15:31 UTC (permalink / raw)
  To: Theodore Ts'o, Christoph Hellwig, torvalds, linux-kernel

On Tue, Oct 15, 2002 at 09:15:07AM -0400, Theodore Ts'o wrote:
> Actually, I did, for those comments that made sense.  The fs/Config.in
> logic has been cleaned up, as well as removing excess header files,
> stray LINUX_VERSION_CODE #ifdef's that I had missed the first time
> around, etc.
> 
> fs/mbcache.c is still there because it applies to both ext2 and ext3
> filesystems, and so your suggestion of moving it into the ext2 and
> ext3 directories would cause code duplication and maintenance
> headaches.  It also *can* be used by other filesystems, as it is
> written in a generic way.  The fs/Config.in only compiles it in if
> necessary (i.e., if ext2/3 extended attribute is enabled) so it won't
> cause code bloat for other filesystems if it is not needed.
> 
> The superblock fields are more of an issue with the posix acl changes
> than for the extended attribute patches.  I had wanted to get the
> extended attribute changes in first, since they stand alone, and so I
> have fewer patches to juggle.

Patch 2: mbcache (all against 2.5.42-mm3):

o remove LINUX_VERSION_CODE mess
o cleanup mb_cache_lock/mb_cache_unlock mess
o use list_move
o use list_del_init/list_empty instead of messing with .prev directly
o remove useless MOD_INC_USE_COUNT/MOD_DEC_USE_COUNT


--- linux-2.5.42-mm3-plain/fs/mbcache.c	Tue Oct 15 17:05:09 2002
+++ linux-2.5.42-mm3/fs/mbcache.c	Tue Oct 15 17:26:53 2002
@@ -27,12 +27,10 @@
 
 #include <linux/kernel.h>
 #include <linux/module.h>
-
 #include <linux/fs.h>
 #include <linux/mm.h>
 #include <linux/slab.h>
 #include <linux/sched.h>
-#include <linux/version.h>
 #include <linux/init.h>
 #include <linux/mbcache.h>
 
@@ -56,9 +54,7 @@
 		
 MODULE_AUTHOR("Andreas Gruenbacher <a.gruenbacher@computer.org>");
 MODULE_DESCRIPTION("Meta block cache (for extended attributes)");
-#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,4,0)
 MODULE_LICENSE("GPL");
-#endif
 
 EXPORT_SYMBOL(mb_cache_create);
 EXPORT_SYMBOL(mb_cache_shrink);
@@ -87,18 +83,6 @@
 static spinlock_t mb_cache_spinlock = SPIN_LOCK_UNLOCKED;
 static struct shrinker *mb_shrinker;
 
-static inline void
-mb_cache_lock(void)
-{
-	spin_lock(&mb_cache_spinlock);
-}
-
-static inline void
-mb_cache_unlock(void)
-{
-	spin_unlock(&mb_cache_spinlock);
-}
-
 static inline int
 mb_cache_indexes(struct mb_cache *cache)
 {
@@ -118,10 +102,8 @@
 static inline void
 __mb_cache_entry_takeout_lru(struct mb_cache_entry *ce)
 {
-	if (ce->e_lru_list.prev) {
-		list_del(&ce->e_lru_list);
-		ce->e_lru_list.prev = NULL;
-	}
+	if (!list_empty(&ce->e_lru_list))
+		list_del_init(&ce->e_lru_list);
 }
 
 
@@ -135,7 +117,7 @@
 static inline int
 __mb_cache_entry_in_lru(struct mb_cache_entry *ce)
 {
-	return (ce->e_lru_list.prev != NULL);
+	return (!list_empty(&ce->e_lru_list));
 }
 
 
@@ -167,9 +149,8 @@
 {
 	int n;
 
-	list_del(&ce->e_block_list);
-	ce->e_block_list.prev = NULL;
-	for (n=0; n<mb_cache_indexes(ce->e_cache); n++)
+	list_del_init(&ce->e_block_list);
+	for (n = 0; n < mb_cache_indexes(ce->e_cache); n++)
 		list_del(&ce->e_indexes[n].o_list);
 }
 
@@ -177,7 +158,7 @@
 static inline int
 __mb_cache_entry_is_linked(struct mb_cache_entry *ce)
 {
-	return (ce->e_block_list.prev != NULL);
+	return (!list_empty(&ce->e_block_list));
 }
 
 
@@ -207,15 +188,15 @@
 __mb_cache_entry_release_unlock(struct mb_cache_entry *ce)
 {
 	if (atomic_dec_and_test(&ce->e_used)) {
-		if (__mb_cache_entry_is_linked(ce))
-			__mb_cache_entry_into_lru(ce);
-		else {
-			mb_cache_unlock();
-			__mb_cache_entry_forget(ce);
-			return;
-		}
+		if (!__mb_cache_entry_is_linked(ce))
+			goto forget;
 	}
-	mb_cache_unlock();
+
+	spin_unlock(&mb_cache_spinlock);
+	return;
+ forget:
+	spin_unlock(&mb_cache_spinlock);
+	__mb_cache_entry_forget(ce);
 }
 
 
@@ -237,7 +218,7 @@
 	struct list_head *l;
 	int count = 0;
 
-	mb_cache_lock();
+	spin_lock(&mb_cache_spinlock);
 	list_for_each_prev(l, &mb_cache_list) {
 		struct mb_cache *cache =
 			list_entry(l, struct mb_cache, c_cache_list);
@@ -247,20 +228,20 @@
 	}
 	mb_debug("trying to free %d entries", nr_to_scan);
 	if (nr_to_scan == 0) {
-		mb_cache_unlock();
+		spin_unlock(&mb_cache_spinlock);
 		goto out;
 	}
 	while (nr_to_scan && !list_empty(&mb_cache_lru_list)) {
 		struct mb_cache_entry *ce =
 			list_entry(mb_cache_lru_list.prev,
 				   struct mb_cache_entry, e_lru_list);
-		list_del(&ce->e_lru_list);
-		list_add(&ce->e_lru_list, &free_list);
+		list_move(&ce->e_lru_list, &free_list);
 		if (__mb_cache_entry_is_linked(ce))
 			__mb_cache_entry_unlink(ce);
 		nr_to_scan--;
 	}
-	mb_cache_unlock();
+	spin_unlock(&mb_cache_spinlock);
+
 	l = free_list.prev;
 	while (l != &free_list) {
 		struct mb_cache_entry *ce = list_entry(l,
@@ -303,7 +284,6 @@
 	   indexes_count * sizeof(struct mb_cache_entry_index))
 		return NULL;
 
-	MOD_INC_USE_COUNT;
 	cache = kmalloc(sizeof(struct mb_cache) +
 	                indexes_count * sizeof(struct list_head), GFP_KERNEL);
 	if (!cache)
@@ -340,7 +320,7 @@
 	if (!cache->c_entry_cache)
 		goto fail;
 
-	mb_cache_lock();
+	spin_lock(&mb_cache_spinlock);
 	if (list_empty(&mb_cache_list)) {
 		if (mb_shrinker) {
 			printk(KERN_ERR "%s: already have a shrinker!\n",
@@ -350,7 +330,7 @@
 		mb_shrinker = set_shrinker(DEFAULT_SEEKS, mb_cache_shrink_fn);
 	}
 	list_add(&cache->c_cache_list, &mb_cache_list);
-	mb_cache_unlock();
+	spin_unlock(&mb_cache_spinlock);
 	return cache;
 
 fail:
@@ -361,7 +341,6 @@
 			kfree(cache->c_block_hash);
 		kfree(cache);
 	}
-	MOD_DEC_USE_COUNT;
 	return NULL;
 }
 
@@ -382,20 +361,19 @@
 	LIST_HEAD(free_list);
 	struct list_head *l;
 
-	mb_cache_lock();
+	spin_lock(&mb_cache_spinlock);
 	l = mb_cache_lru_list.prev;
 	while (l != &mb_cache_lru_list) {
 		struct mb_cache_entry *ce =
 			list_entry(l, struct mb_cache_entry, e_lru_list);
 		l = l->prev;
 		if (ce->e_dev == dev) {
-			list_del(&ce->e_lru_list);
-			list_add(&ce->e_lru_list, &free_list);
+			list_move(&ce->e_lru_list, &free_list);
 			if (__mb_cache_entry_is_linked(ce))
 				__mb_cache_entry_unlink(ce);
 		}
 	}
-	mb_cache_unlock();
+	spin_unlock(&mb_cache_spinlock);
 	l = free_list.prev;
 	while (l != &free_list) {
 		struct mb_cache_entry *ce =
@@ -420,15 +398,14 @@
 	struct list_head *l;
 	int n;
 
-	mb_cache_lock();
+	spin_lock(&mb_cache_spinlock);
 	l = mb_cache_lru_list.prev;
 	while (l != &mb_cache_lru_list) {
 		struct mb_cache_entry *ce =
 			list_entry(l, struct mb_cache_entry, e_lru_list);
 		l = l->prev;
 		if (ce->e_cache == cache) {
-			list_del(&ce->e_lru_list);
-			list_add(&ce->e_lru_list, &free_list);
+			list_move(&ce->e_lru_list, &free_list);
 			if (__mb_cache_entry_is_linked(ce))
 				__mb_cache_entry_unlink(ce);
 		}
@@ -438,7 +415,7 @@
 		remove_shrinker(mb_shrinker);
 		mb_shrinker = 0;
 	}
-	mb_cache_unlock();
+	spin_unlock(&mb_cache_spinlock);
 
 	l = free_list.prev;
 	while (l != &free_list) {
@@ -454,19 +431,12 @@
 			  atomic_read(&cache->c_entry_count));
 	}
 
-#if (LINUX_VERSION_CODE < KERNEL_VERSION(2,3,0))
-	/* We don't have kmem_cache_destroy() in 2.2.x */
-	kmem_cache_shrink(cache->c_entry_cache);
-#else
 	kmem_cache_destroy(cache->c_entry_cache);
-#endif
 	for (n=0; n < mb_cache_indexes(cache); n++)
 		kfree(cache->c_indexes_hash[n]);
 	kfree(cache->c_block_hash);
 
 	kfree(cache);
-
-	MOD_DEC_USE_COUNT;
 }
 
 
@@ -486,8 +456,8 @@
 	atomic_inc(&cache->c_entry_count);
 	ce = kmem_cache_alloc(cache->c_entry_cache, GFP_KERNEL);
 	if (ce) {
-		ce->e_lru_list.prev = NULL;
-		ce->e_block_list.prev = NULL;
+		INIT_LIST_HEAD(&ce->e_lru_list);
+		INIT_LIST_HEAD(&ce->e_block_list);
 		ce->e_cache = cache;
 		atomic_set(&ce->e_used, 1);
 	}
@@ -519,7 +489,7 @@
 	struct list_head *l;
 	int error = -EBUSY, n;
 
-	mb_cache_lock();
+	spin_lock(&mb_cache_spinlock);
 	list_for_each_prev(l, &cache->c_block_hash[bucket]) {
 		struct mb_cache_entry *ce =
 			list_entry(l, struct mb_cache_entry, e_block_list);
@@ -533,7 +503,7 @@
 		ce->e_indexes[n].o_key = keys[n];
 	__mb_cache_entry_link(ce);
 out:
-	mb_cache_unlock();
+	spin_unlock(&mb_cache_spinlock);
 	return error;
 }
 
@@ -548,7 +518,7 @@
 void
 mb_cache_entry_release(struct mb_cache_entry *ce)
 {
-	mb_cache_lock();
+	spin_lock(&mb_cache_spinlock);
 	__mb_cache_entry_release_unlock(ce);
 }
 
@@ -563,11 +533,11 @@
 void
 mb_cache_entry_takeout(struct mb_cache_entry *ce)
 {
-	mb_cache_lock();
+	spin_lock(&mb_cache_spinlock);
 	mb_assert(!__mb_cache_entry_in_lru(ce));
 	if (__mb_cache_entry_is_linked(ce))
 		__mb_cache_entry_unlink(ce);
-	mb_cache_unlock();
+	spin_unlock(&mb_cache_spinlock);
 }
 
 
@@ -580,7 +550,7 @@
 void
 mb_cache_entry_free(struct mb_cache_entry *ce)
 {
-	mb_cache_lock();
+	spin_lock(&mb_cache_spinlock);
 	mb_assert(!__mb_cache_entry_in_lru(ce));
 	if (__mb_cache_entry_is_linked(ce))
 		__mb_cache_entry_unlink(ce);
@@ -616,7 +586,7 @@
 	struct list_head *l;
 	struct mb_cache_entry *ce;
 
-	mb_cache_lock();
+	spin_lock(&mb_cache_spinlock);
 	list_for_each(l, &cache->c_block_hash[bucket]) {
 		ce = list_entry(l, struct mb_cache_entry, e_block_list);
 		if (ce->e_dev == dev && ce->e_block == block) {
@@ -627,7 +597,7 @@
 	ce = NULL;
 
 cleanup:
-	mb_cache_unlock();
+	spin_unlock(&mb_cache_spinlock);
 	return ce;
 }
 
@@ -674,11 +644,12 @@
 	struct mb_cache_entry *ce;
 
 	mb_assert(index < mb_cache_indexes(cache));
-	mb_cache_lock();
+
+	spin_lock(&mb_cache_spinlock);
 	l = cache->c_indexes_hash[index][bucket].next;
 	ce = __mb_cache_entry_find(l, &cache->c_indexes_hash[index][bucket],
 	                           index, dev, key);
-	mb_cache_unlock();
+	spin_unlock(&mb_cache_spinlock);
 	return ce;
 }
 
@@ -712,7 +683,8 @@
 	struct mb_cache_entry *ce;
 
 	mb_assert(index < mb_cache_indexes(cache));
-	mb_cache_lock();
+
+	spin_lock(&mb_cache_spinlock);
 	l = prev->e_indexes[index].o_list.next;
 	ce = __mb_cache_entry_find(l, &cache->c_indexes_hash[index][bucket],
 	                           index, dev, key);

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15 13:15   ` Theodore Ts'o
  2002-10-15 15:25     ` Christoph Hellwig
  2002-10-15 15:31     ` Christoph Hellwig
@ 2002-10-15 15:33     ` Christoph Hellwig
  2002-10-15 15:34     ` Christoph Hellwig
  3 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2002-10-15 15:33 UTC (permalink / raw)
  To: Theodore Ts'o, Christoph Hellwig, torvalds, linux-kernel

On Tue, Oct 15, 2002 at 09:15:07AM -0400, Theodore Ts'o wrote:
> Actually, I did, for those comments that made sense.  The fs/Config.in
> logic has been cleaned up, as well as removing excess header files,
> stray LINUX_VERSION_CODE #ifdef's that I had missed the first time
> around, etc.
> 
> fs/mbcache.c is still there because it applies to both ext2 and ext3
> filesystems, and so your suggestion of moving it into the ext2 and
> ext3 directories would cause code duplication and maintenance
> headaches.  It also *can* be used by other filesystems, as it is
> written in a generic way.  The fs/Config.in only compiles it in if
> necessary (i.e., if ext2/3 extended attribute is enabled) so it won't
> cause code bloat for other filesystems if it is not needed.
> 
> The superblock fields are more of an issue with the posix acl changes
> than for the extended attribute patches.  I had wanted to get the
> extended attribute changes in first, since they stand alone, and so I
> have fewer patches to juggle.

Patch 3: first round of ext2 cleanups (all against 2.5.42-mm3):

o move include/linux/ext2_xattr.h to fs/ext2/xattr.h
o include xattr.h in the files that actually need it
o remove old compat code
o remove dead quota code
o remove ext2_xattr_lock/ext2_xattr_unlock wrappers (only two callers..)

More cleanups will follow.


--- linux-2.5.42-mm3-plain/fs/ext2/ext2.h	Tue Oct 15 17:05:08 2002
+++ linux-2.5.42-mm3/fs/ext2/ext2.h	Tue Oct 15 13:46:09 2002
@@ -1,6 +1,5 @@
 #include <linux/fs.h>
 #include <linux/ext2_fs.h>
-#include <linux/ext2_xattr.h>
 
 /*
  * second extended file system inode data in memory
--- linux-2.5.42-mm3-plain/fs/ext2/file.c	Tue Oct 15 17:05:08 2002
+++ linux-2.5.42-mm3/fs/ext2/file.c	Tue Oct 15 13:20:55 2002
@@ -18,8 +18,9 @@
  * 	(jj@sunsite.ms.mff.cuni.cz)
  */
 
-#include "ext2.h"
 #include <linux/time.h>
+#include "ext2.h"
+#include "xattr.h"
 
 /*
  * Called when an inode is released. Note that this is different
--- linux-2.5.42-mm3-plain/fs/ext2/ialloc.c	Tue Oct 15 17:05:08 2002
+++ linux-2.5.42-mm3/fs/ext2/ialloc.c	Tue Oct 15 13:22:21 2002
@@ -14,6 +14,7 @@
 
 #include <linux/config.h>
 #include "ext2.h"
+#include "xattr.h"
 #include <linux/quotaops.h>
 #include <linux/sched.h>
 #include <linux/backing-dev.h>
--- linux-2.5.42-mm3-plain/fs/ext2/namei.c	Tue Oct 15 17:05:08 2002
+++ linux-2.5.42-mm3/fs/ext2/namei.c	Tue Oct 15 13:22:55 2002
@@ -29,8 +29,10 @@
  *        David S. Miller (davem@caip.rutgers.edu), 1995
  */
 
-#include "ext2.h"
 #include <linux/pagemap.h>
+#include "ext2.h"
+#include "xattr.h"
+
 
 /*
  * Couple of helper functions - make the code slightly cleaner.
--- linux-2.5.42-mm3-plain/fs/ext2/super.c	Tue Oct 15 17:05:08 2002
+++ linux-2.5.42-mm3/fs/ext2/super.c	Tue Oct 15 13:23:20 2002
@@ -27,6 +27,7 @@
 #include <linux/buffer_head.h>
 #include <linux/smp_lock.h>
 #include <asm/uaccess.h>
+#include "xattr.h"
 
 
 static void ext2_sync_super(struct super_block *sb,
--- linux-2.5.42-mm3-plain/fs/ext2/symlink.c	Tue Oct 15 17:05:08 2002
+++ linux-2.5.42-mm3/fs/ext2/symlink.c	Tue Oct 15 13:49:15 2002
@@ -18,6 +18,7 @@
  */
 
 #include "ext2.h"
+#include "xattr.h"
 
 static int ext2_readlink(struct dentry *dentry, char *buffer, int buflen)
 {
--- linux-2.5.42-mm3-plain/fs/ext2/xattr.c	Tue Oct 15 17:05:08 2002
+++ linux-2.5.42-mm3/fs/ext2/xattr.c	Tue Oct 15 16:11:43 2002
@@ -49,20 +49,17 @@
  * For writing we also grab the ext2_xattr_sem semaphore. This ensures that
  * only a single process is modifying an extended attribute block, even
  * if the block is shared among inodes.
- *
- * Note for porting to 2.5
- * -----------------------
- * The BKL will no longer be held in the xattr inode operations.
  */
 
-#include "ext2.h"
 #include <linux/buffer_head.h>
 #include <linux/module.h>
+#include <linux/init.h>
 #include <linux/slab.h>
 #include <linux/mbcache.h>
 #include <linux/quotaops.h>
 #include <asm/semaphore.h>
-#include <linux/compatmac.h>
+#include "ext2.h"
+#include "xattr.h"
 
 /* These symbols may be needed by a module. */
 EXPORT_SYMBOL(ext2_xattr_register);
@@ -71,10 +68,6 @@
 EXPORT_SYMBOL(ext2_xattr_list);
 EXPORT_SYMBOL(ext2_xattr_set);
 
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,4,0)
-# define mark_buffer_dirty(bh) mark_buffer_dirty(bh, 1)
-#endif
-
 #define HDR(bh) ((struct ext2_xattr_header *)((bh)->b_data))
 #define ENTRY(ptr) ((struct ext2_xattr_entry *)(ptr))
 #define FIRST_ENTRY(bh) ENTRY(HDR(bh)+1)
@@ -116,92 +109,9 @@
  * filesystem may still contain shared blocks, so we always take
  * the lock.
  */
-
-DECLARE_MUTEX(ext2_xattr_sem);
-
-static inline void
-ext2_xattr_lock(void)
-{
-	down(&ext2_xattr_sem);
-}
-
-static inline void
-ext2_xattr_unlock(void)
-{
-	up(&ext2_xattr_sem);
-}
-
-static inline int
-ext2_xattr_new_block(struct inode *inode, int * errp, int force)
-{
-	struct super_block *sb = inode->i_sb;
-	int goal = le32_to_cpu(EXT2_SB(sb)->s_es->s_first_data_block) +
-		EXT2_I(inode)->i_block_group * EXT2_BLOCKS_PER_GROUP(sb);
-
-	/* How can we enforce the allocation? */
-	int block = ext2_new_block(inode, goal, 0, 0, errp);
-#ifdef OLD_QUOTAS
-	if (!*errp)
-		inode->i_blocks += inode->i_sb->s_blocksize >> 9;
-#endif
-	return block;
-}
-
-static inline int
-ext2_xattr_quota_alloc(struct inode *inode, int force)
-{
-	/* How can we enforce the allocation? */
-#ifdef OLD_QUOTAS
-	int error = DQUOT_ALLOC_BLOCK(inode->i_sb, inode, 1);
-	if (!error)
-		inode->i_blocks += inode->i_sb->s_blocksize >> 9;
-#else
-	int error = DQUOT_ALLOC_BLOCK(inode, 1);
-#endif
-	return error;
-}
-
-#ifdef OLD_QUOTAS
-
-static inline void
-ext2_xattr_quota_free(struct inode *inode)
-{
-	DQUOT_FREE_BLOCK(inode->i_sb, inode, 1);
-	inode->i_blocks -= inode->i_sb->s_blocksize >> 9;
-}
-
-static inline void
-ext2_xattr_free_block(struct inode * inode, unsigned long block)
-{
-	ext2_free_blocks(inode, block, 1);
-	inode->i_blocks -= inode->i_sb->s_blocksize >> 9;
-}
-
-#else
-# define ext2_xattr_quota_free(inode) \
-	DQUOT_FREE_BLOCK(inode, 1)
-# define ext2_xattr_free_block(inode, block) \
-	ext2_free_blocks(inode, block, 1)
-#endif
-
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,4,18)
-
-static inline struct buffer_head *
-sb_bread(struct super_block *sb, int block)
-{
-	return bread(sb->s_dev, block, sb->s_blocksize);
-}
-
-static inline struct buffer_head *
-sb_getblk(struct super_block *sb, int block)
-{
-	return getblk(sb->s_dev, block, sb->s_blocksize);
-}
-
-#endif
-
-struct ext2_xattr_handler *ext2_xattr_handlers[EXT2_XATTR_INDEX_MAX];
-rwlock_t ext2_handler_lock = RW_LOCK_UNLOCKED;
+static DECLARE_MUTEX(ext2_xattr_sem);
+static struct ext2_xattr_handler *ext2_xattr_handlers[EXT2_XATTR_INDEX_MAX];
+static rwlock_t ext2_handler_lock = RW_LOCK_UNLOCKED;
 
 int
 ext2_xattr_register(int name_index, struct ext2_xattr_handler *handler)
@@ -545,9 +455,6 @@
 		return;
 
 	lock_super(sb);
-#if LINUX_VERSION_CODE < KERNEL_VERSION(2,4,0)
-	EXT2_SB(sb)->s_feature_compat |= EXT2_FEATURE_COMPAT_EXT_ATTR;
-#endif
 	EXT2_SB(sb)->s_es->s_feature_compat |=
 		cpu_to_le32(EXT2_FEATURE_COMPAT_EXT_ATTR);
 	sb->s_dirt = 1;
@@ -605,8 +512,8 @@
 	name_len = strlen(name);
 	if (name_len > 255 || value_len > sb->s_blocksize)
 		return -ERANGE;
-	ext2_xattr_lock();
 
+	down(&ext2_xattr_sem);
 	if (EXT2_I(inode)->i_file_acl) {
 		/* The inode already has an extended attribute block. */
 		int block = EXT2_I(inode)->i_file_acl;
@@ -802,7 +709,7 @@
 	brelse(bh);
 	if (!(bh && header == HDR(bh)))
 		kfree(header);
-	ext2_xattr_unlock();
+	up(&ext2_xattr_sem);
 
 	return error;
 }
@@ -830,7 +737,8 @@
 				new_bh->b_blocknr);
 			
 			error = -EDQUOT;
-			if (ext2_xattr_quota_alloc(inode, 1))
+			/* How can we enforce the allocation? */
+			if (DQUOT_ALLOC_BLOCK(inode, 1))
 				goto cleanup;
 			
 			HDR(new_bh)->h_refcount = cpu_to_le32(
@@ -843,15 +751,18 @@
 			ext2_xattr_cache_insert(new_bh);
 		} else {
 			/* We need to allocate a new block */
-			int force = EXT2_I(inode)->i_file_acl != 0;
-			int block = ext2_xattr_new_block(inode, &error, force);
+			int goal = le32_to_cpu(EXT2_SB(sb)->s_es->s_first_data_block) +
+					EXT2_I(inode)->i_block_group *
+					EXT2_BLOCKS_PER_GROUP(sb);
+			/* How can we enforce the allocation? */
+			int block = ext2_new_block(inode, goal, 0, 0, &error);
 			if (error)
 				goto cleanup;
 			ea_idebug(inode, "creating block %d", block);
 
 			new_bh = sb_getblk(sb, block);
 			if (!new_bh) {
-				ext2_xattr_free_block(inode, block);
+				ext2_free_blocks(inode, block, 1);
 				error = -EIO;
 				goto cleanup;
 			}
@@ -894,7 +805,7 @@
 		if (refcount == 1) {
 			/* Free the old block. */
 			ea_bdebug(old_bh, "freeing");
-			ext2_xattr_free_block(inode, old_bh->b_blocknr);
+			ext2_free_blocks(inode, old_bh->b_blocknr, 1);
 			/* We let our caller release old_bh, so we
 			 * need to duplicate the buffer before. */
 			get_bh(old_bh);
@@ -903,7 +814,7 @@
 			/* Decrement the refcount only. */
 			refcount--;
 			HDR(old_bh)->h_refcount = cpu_to_le32(refcount);
-			ext2_xattr_quota_free(inode);
+			DQUOT_FREE_BLOCK(inode, 1);
 			mark_buffer_dirty(old_bh);
 			ea_bdebug(old_bh, "refcount now=%d", refcount);
 		}
@@ -930,8 +841,8 @@
 
 	if (!block)
 		return;
-	ext2_xattr_lock();
 
+	down(&ext2_xattr_sem);
 	bh = sb_bread(inode->i_sb, block);
 	if (!bh) {
 		ext2_error(inode->i_sb, "ext2_xattr_delete_inode",
@@ -948,7 +859,7 @@
 	ea_bdebug(bh, "refcount now=%d", le32_to_cpu(HDR(bh)->h_refcount) - 1);
 	if (HDR(bh)->h_refcount == cpu_to_le32(1)) {
 		ext2_xattr_cache_remove(bh);
-		ext2_xattr_free_block(inode, block);
+		ext2_free_blocks(inode, block, 1);
 		bforget(bh);
 		bh = NULL;
 	} else {
@@ -959,13 +870,13 @@
 			ll_rw_block(WRITE, 1, &bh);
 			wait_on_buffer(bh);
 		}
-		ext2_xattr_quota_free(inode);
+		DQUOT_FREE_BLOCK(inode, 1);
 	}
 	EXT2_I(inode)->i_file_acl = 0;
 
 cleanup:
 	brelse(bh);
-	ext2_xattr_unlock();
+	up(&ext2_xattr_sem);
 }
 
 /*
--- linux-2.5.42-mm3-plain/fs/ext2/xattr.h	Thu Jan  1 01:00:00 1970
+++ linux-2.5.42-mm3/fs/ext2/xattr.h	Tue Oct 15 13:45:05 2002
@@ -0,0 +1,134 @@
+/*
+  File: fs/ext2/xattr.h
+
+  On-disk format of extended attributes for the ext2 filesystem.
+
+  (C) 2001 Andreas Gruenbacher, <a.gruenbacher@computer.org>
+*/
+
+#include <linux/config.h>
+#include <linux/xattr.h>
+
+/* Magic value in attribute blocks */
+#define EXT2_XATTR_MAGIC		0xEA020000
+
+/* Maximum number of references to one attribute block */
+#define EXT2_XATTR_REFCOUNT_MAX		1024
+
+/* Name indexes */
+#define EXT2_XATTR_INDEX_MAX			10
+#define EXT2_XATTR_INDEX_USER			1
+#define EXT2_XATTR_INDEX_POSIX_ACL_ACCESS	2
+#define EXT2_XATTR_INDEX_POSIX_ACL_DEFAULT	3
+
+struct ext2_xattr_header {
+	__u32	h_magic;	/* magic number for identification */
+	__u32	h_refcount;	/* reference count */
+	__u32	h_blocks;	/* number of disk blocks used */
+	__u32	h_hash;		/* hash value of all attributes */
+	__u32	h_reserved[4];	/* zero right now */
+};
+
+struct ext2_xattr_entry {
+	__u8	e_name_len;	/* length of name */
+	__u8	e_name_index;	/* attribute name index */
+	__u16	e_value_offs;	/* offset in disk block of value */
+	__u32	e_value_block;	/* disk block attribute is stored on (n/i) */
+	__u32	e_value_size;	/* size of attribute value */
+	__u32	e_hash;		/* hash value of name and value */
+	char	e_name[0];	/* attribute name */
+};
+
+#define EXT2_XATTR_PAD_BITS		2
+#define EXT2_XATTR_PAD		(1<<EXT2_XATTR_PAD_BITS)
+#define EXT2_XATTR_ROUND		(EXT2_XATTR_PAD-1)
+#define EXT2_XATTR_LEN(name_len) \
+	(((name_len) + EXT2_XATTR_ROUND + \
+	sizeof(struct ext2_xattr_entry)) & ~EXT2_XATTR_ROUND)
+#define EXT2_XATTR_NEXT(entry) \
+	( (struct ext2_xattr_entry *)( \
+	  (char *)(entry) + EXT2_XATTR_LEN((entry)->e_name_len)) )
+#define EXT2_XATTR_SIZE(size) \
+	(((size) + EXT2_XATTR_ROUND) & ~EXT2_XATTR_ROUND)
+
+# ifdef CONFIG_EXT2_FS_XATTR
+
+struct ext2_xattr_handler {
+	char *prefix;
+	size_t (*list)(char *list, struct inode *inode, const char *name,
+		       int name_len);
+	int (*get)(struct inode *inode, const char *name, void *buffer,
+		   size_t size);
+	int (*set)(struct inode *inode, const char *name, const void *buffer,
+		   size_t size, int flags);
+};
+
+extern int ext2_xattr_register(int, struct ext2_xattr_handler *);
+extern void ext2_xattr_unregister(int, struct ext2_xattr_handler *);
+
+extern int ext2_setxattr(struct dentry *, const char *, void *, size_t, int);
+extern ssize_t ext2_getxattr(struct dentry *, const char *, void *, size_t);
+extern ssize_t ext2_listxattr(struct dentry *, char *, size_t);
+extern int ext2_removexattr(struct dentry *, const char *);
+
+extern int ext2_xattr_get(struct inode *, int, const char *, void *, size_t);
+extern int ext2_xattr_list(struct inode *, char *, size_t);
+extern int ext2_xattr_set(struct inode *, int, const char *, const void *, size_t, int);
+
+extern void ext2_xattr_delete_inode(struct inode *);
+extern void ext2_xattr_put_super(struct super_block *);
+
+extern int init_ext2_xattr(void);
+extern void exit_ext2_xattr(void);
+
+# else  /* CONFIG_EXT2_FS_XATTR */
+#  define ext2_setxattr		NULL
+#  define ext2_getxattr		NULL
+#  define ext2_listxattr	NULL
+#  define ext2_removexattr	NULL
+
+static inline int
+ext2_xattr_get(struct inode *inode, int name_index,
+	       const char *name, void *buffer, size_t size)
+{
+	return -ENOTSUPP;
+}
+
+static inline int
+ext2_xattr_list(struct inode *inode, char *buffer, size_t size)
+{
+	return -ENOTSUPP;
+}
+
+static inline int
+ext2_xattr_set(struct inode *inode, int name_index, const char *name,
+	       const void *value, size_t size, int flags)
+{
+	return -ENOTSUPP;
+}
+
+static inline void
+ext2_xattr_delete_inode(struct inode *inode)
+{
+}
+
+static inline void
+ext2_xattr_put_super(struct super_block *sb)
+{
+}
+
+static inline int
+init_ext2_xattr(void)
+{
+	return 0;
+}
+
+static inline void
+exit_ext2_xattr(void)
+{
+}
+
+# endif  /* CONFIG_EXT2_FS_XATTR */
+
+extern struct ext2_xattr_handler ext2_xattr_user_handler;
+
--- linux-2.5.42-mm3-plain/fs/ext2/xattr_user.c	Tue Oct 15 17:05:08 2002
+++ linux-2.5.42-mm3/fs/ext2/xattr_user.c	Tue Oct 15 16:10:37 2002
@@ -5,9 +5,11 @@
  * Copyright (C) 2001 by Andreas Gruenbacher, <a.gruenbacher@computer.org>
  */
 
-#include "ext2.h"
+#include <linux/init.h>
 #include <linux/module.h>
 #include <linux/string.h>
+#include "ext2.h"
+#include "xattr.h"
 
 #ifdef CONFIG_EXT2_FS_POSIX_ACL
 # include <linux/ext2_acl.h>
--- linux-2.5.42-mm3-plain/include/linux/ext2_xattr.h	Tue Oct 15 17:05:12 2002
+++ linux-2.5.42-mm3/include/linux/ext2_xattr.h	Thu Jan  1 01:00:00 1970
@@ -1,135 +0,0 @@
-/*
-  File: linux/ext2_xattr.h
-
-  On-disk format of extended attributes for the ext2 filesystem.
-
-  (C) 2001 Andreas Gruenbacher, <a.gruenbacher@computer.org>
-*/
-
-#include <linux/config.h>
-#include <linux/init.h>
-#include <linux/xattr.h>
-
-/* Magic value in attribute blocks */
-#define EXT2_XATTR_MAGIC		0xEA020000
-
-/* Maximum number of references to one attribute block */
-#define EXT2_XATTR_REFCOUNT_MAX		1024
-
-/* Name indexes */
-#define EXT2_XATTR_INDEX_MAX			10
-#define EXT2_XATTR_INDEX_USER			1
-#define EXT2_XATTR_INDEX_POSIX_ACL_ACCESS	2
-#define EXT2_XATTR_INDEX_POSIX_ACL_DEFAULT	3
-
-struct ext2_xattr_header {
-	__u32	h_magic;	/* magic number for identification */
-	__u32	h_refcount;	/* reference count */
-	__u32	h_blocks;	/* number of disk blocks used */
-	__u32	h_hash;		/* hash value of all attributes */
-	__u32	h_reserved[4];	/* zero right now */
-};
-
-struct ext2_xattr_entry {
-	__u8	e_name_len;	/* length of name */
-	__u8	e_name_index;	/* attribute name index */
-	__u16	e_value_offs;	/* offset in disk block of value */
-	__u32	e_value_block;	/* disk block attribute is stored on (n/i) */
-	__u32	e_value_size;	/* size of attribute value */
-	__u32	e_hash;		/* hash value of name and value */
-	char	e_name[0];	/* attribute name */
-};
-
-#define EXT2_XATTR_PAD_BITS		2
-#define EXT2_XATTR_PAD		(1<<EXT2_XATTR_PAD_BITS)
-#define EXT2_XATTR_ROUND		(EXT2_XATTR_PAD-1)
-#define EXT2_XATTR_LEN(name_len) \
-	(((name_len) + EXT2_XATTR_ROUND + \
-	sizeof(struct ext2_xattr_entry)) & ~EXT2_XATTR_ROUND)
-#define EXT2_XATTR_NEXT(entry) \
-	( (struct ext2_xattr_entry *)( \
-	  (char *)(entry) + EXT2_XATTR_LEN((entry)->e_name_len)) )
-#define EXT2_XATTR_SIZE(size) \
-	(((size) + EXT2_XATTR_ROUND) & ~EXT2_XATTR_ROUND)
-
-# ifdef CONFIG_EXT2_FS_XATTR
-
-struct ext2_xattr_handler {
-	char *prefix;
-	size_t (*list)(char *list, struct inode *inode, const char *name,
-		       int name_len);
-	int (*get)(struct inode *inode, const char *name, void *buffer,
-		   size_t size);
-	int (*set)(struct inode *inode, const char *name, const void *buffer,
-		   size_t size, int flags);
-};
-
-extern int ext2_xattr_register(int, struct ext2_xattr_handler *);
-extern void ext2_xattr_unregister(int, struct ext2_xattr_handler *);
-
-extern int ext2_setxattr(struct dentry *, const char *, void *, size_t, int);
-extern ssize_t ext2_getxattr(struct dentry *, const char *, void *, size_t);
-extern ssize_t ext2_listxattr(struct dentry *, char *, size_t);
-extern int ext2_removexattr(struct dentry *, const char *);
-
-extern int ext2_xattr_get(struct inode *, int, const char *, void *, size_t);
-extern int ext2_xattr_list(struct inode *, char *, size_t);
-extern int ext2_xattr_set(struct inode *, int, const char *, const void *, size_t, int);
-
-extern void ext2_xattr_delete_inode(struct inode *);
-extern void ext2_xattr_put_super(struct super_block *);
-
-extern int init_ext2_xattr(void) __init;
-extern void exit_ext2_xattr(void);
-
-# else  /* CONFIG_EXT2_FS_XATTR */
-#  define ext2_setxattr		NULL
-#  define ext2_getxattr		NULL
-#  define ext2_listxattr	NULL
-#  define ext2_removexattr	NULL
-
-static inline int
-ext2_xattr_get(struct inode *inode, int name_index,
-	       const char *name, void *buffer, size_t size)
-{
-	return -ENOTSUP;
-}
-
-static inline int
-ext2_xattr_list(struct inode *inode, char *buffer, size_t size)
-{
-	return -ENOTSUP;
-}
-
-static inline int
-ext2_xattr_set(struct inode *inode, int name_index, const char *name,
-	       const void *value, size_t size, int flags)
-{
-	return -ENOTSUP;
-}
-
-static inline void
-ext2_xattr_delete_inode(struct inode *inode)
-{
-}
-
-static inline void
-ext2_xattr_put_super(struct super_block *sb)
-{
-}
-
-static inline int
-init_ext2_xattr(void)
-{
-	return 0;
-}
-
-static inline void
-exit_ext2_xattr(void)
-{
-}
-
-# endif  /* CONFIG_EXT2_FS_XATTR */
-
-extern struct ext2_xattr_handler ext2_xattr_user_handler;
-

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15 13:15   ` Theodore Ts'o
                       ` (2 preceding siblings ...)
  2002-10-15 15:33     ` Christoph Hellwig
@ 2002-10-15 15:34     ` Christoph Hellwig
  3 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2002-10-15 15:34 UTC (permalink / raw)
  To: Theodore Ts'o, Christoph Hellwig, torvalds, linux-kernel

On Tue, Oct 15, 2002 at 09:15:07AM -0400, Theodore Ts'o wrote:
> Actually, I did, for those comments that made sense.  The fs/Config.in
> logic has been cleaned up, as well as removing excess header files,
> stray LINUX_VERSION_CODE #ifdef's that I had missed the first time
> around, etc.
> 
> fs/mbcache.c is still there because it applies to both ext2 and ext3
> filesystems, and so your suggestion of moving it into the ext2 and
> ext3 directories would cause code duplication and maintenance
> headaches.  It also *can* be used by other filesystems, as it is
> written in a generic way.  The fs/Config.in only compiles it in if
> necessary (i.e., if ext2/3 extended attribute is enabled) so it won't
> cause code bloat for other filesystems if it is not needed.
> 
> The superblock fields are more of an issue with the posix acl changes
> than for the extended attribute patches.  I had wanted to get the
> extended attribute changes in first, since they stand alone, and so I
> have fewer patches to juggle.

Patch 4: first round of ext3 cleanups (all against 2.5.42-mm3):

o same for ext3


--- linux-2.5.42-mm3-plain/fs/ext3/file.c	Tue Oct 15 17:05:08 2002
+++ linux-2.5.42-mm3/fs/ext3/file.c	Tue Oct 15 13:49:48 2002
@@ -22,9 +22,8 @@
 #include <linux/fs.h>
 #include <linux/jbd.h>
 #include <linux/ext3_fs.h>
-#include <linux/ext3_xattr.h>
 #include <linux/ext3_jbd.h>
-#include <linux/smp_lock.h>
+#include "xattr.h"
 
 /*
  * Called when an inode is released. Note that this is different
--- linux-2.5.42-mm3-plain/fs/ext3/ialloc.c	Tue Oct 15 17:05:08 2002
+++ linux-2.5.42-mm3/fs/ext3/ialloc.c	Tue Oct 15 13:22:07 2002
@@ -17,7 +17,6 @@
 #include <linux/jbd.h>
 #include <linux/ext3_fs.h>
 #include <linux/ext3_jbd.h>
-#include <linux/ext3_xattr.h>
 #include <linux/stat.h>
 #include <linux/string.h>
 #include <linux/quotaops.h>
@@ -25,6 +24,8 @@
 
 #include <asm/bitops.h>
 #include <asm/byteorder.h>
+
+#include "xattr.h"
 
 /*
  * ialloc.c contains the inodes allocation and deallocation routines
--- linux-2.5.42-mm3-plain/fs/ext3/namei.c	Tue Oct 15 17:05:08 2002
+++ linux-2.5.42-mm3/fs/ext3/namei.c	Tue Oct 15 13:22:36 2002
@@ -30,13 +30,13 @@
 #include <linux/time.h>
 #include <linux/ext3_fs.h>
 #include <linux/ext3_jbd.h>
-#include <linux/ext3_xattr.h>
 #include <linux/fcntl.h>
 #include <linux/stat.h>
 #include <linux/string.h>
 #include <linux/quotaops.h>
 #include <linux/buffer_head.h>
 #include <linux/smp_lock.h>
+#include "xattr.h"
 
 
 /*
--- linux-2.5.42-mm3-plain/fs/ext3/super.c	Tue Oct 15 17:05:09 2002
+++ linux-2.5.42-mm3/fs/ext3/super.c	Tue Oct 15 13:23:10 2002
@@ -24,13 +24,13 @@
 #include <linux/jbd.h>
 #include <linux/ext3_fs.h>
 #include <linux/ext3_jbd.h>
-#include <linux/ext3_xattr.h>
 #include <linux/slab.h>
 #include <linux/init.h>
 #include <linux/blkdev.h>
 #include <linux/smp_lock.h>
 #include <linux/buffer_head.h>
 #include <asm/uaccess.h>
+#include "xattr.h"
 
 #ifdef CONFIG_JBD_DEBUG
 static int ext3_ro_after; /* Make fs read-only after this many jiffies */
--- linux-2.5.42-mm3-plain/fs/ext3/symlink.c	Tue Oct 15 17:05:09 2002
+++ linux-2.5.42-mm3/fs/ext3/symlink.c	Tue Oct 15 13:50:16 2002
@@ -20,7 +20,7 @@
 #include <linux/fs.h>
 #include <linux/jbd.h>
 #include <linux/ext3_fs.h>
-#include <linux/ext3_xattr.h>
+#include "xattr.h"
 
 static int ext3_readlink(struct dentry *dentry, char *buffer, int buflen)
 {
--- linux-2.5.42-mm3-plain/fs/ext3/xattr.c	Tue Oct 15 17:05:09 2002
+++ linux-2.5.42-mm3/fs/ext3/xattr.c	Tue Oct 15 16:13:25 2002
@@ -52,15 +52,15 @@
  * if the block is shared among inodes.
  */
 
+#include <linux/init.h>
 #include <linux/fs.h>
 #include <linux/slab.h>
 #include <linux/ext3_jbd.h>
 #include <linux/ext3_fs.h>
-#include <linux/ext3_xattr.h>
 #include <linux/mbcache.h>
 #include <linux/quotaops.h>
 #include <asm/semaphore.h>
-#include <linux/compatmac.h>
+#include "xattr.h"
 
 #define EXT3_EA_USER "user."
 
@@ -105,76 +105,7 @@
  * filesystem may still contain shared blocks, so we always take
  * the lock.
  */
-
-DECLARE_MUTEX(ext3_xattr_sem);
-
-static inline void
-ext3_xattr_lock(void)
-{
-	down(&ext3_xattr_sem);
-}
-
-static inline void
-ext3_xattr_unlock(void)
-{
-	up(&ext3_xattr_sem);
-}
-
-static inline int
-ext3_xattr_new_block(handle_t *handle, struct inode *inode,
-		     int * errp, int force)
-{
-	struct super_block *sb = inode->i_sb;
-	int goal = le32_to_cpu(EXT3_SB(sb)->s_es->s_first_data_block) +
-		EXT3_I(inode)->i_block_group * EXT3_BLOCKS_PER_GROUP(sb);
-
-	/* How can we enforce the allocation? */
-	int block = ext3_new_block(handle, inode, goal, 0, 0, errp);
-#ifdef OLD_QUOTAS
-	if (!*errp)
-		inode->i_blocks += inode->i_sb->s_blocksize >> 9;
-#endif
-	return block;
-}
-
-static inline int
-ext3_xattr_quota_alloc(struct inode *inode, int force)
-{
-	/* How can we enforce the allocation? */
-#ifdef OLD_QUOTAS
-	int error = DQUOT_ALLOC_BLOCK(inode->i_sb, inode, 1);
-	if (!error)
-		inode->i_blocks += inode->i_sb->s_blocksize >> 9;
-#else
-	int error = DQUOT_ALLOC_BLOCK(inode, 1);
-#endif
-	return error;
-}
-
-#ifdef OLD_QUOTAS
-
-static inline void
-ext3_xattr_quota_free(struct inode *inode)
-{
-	DQUOT_FREE_BLOCK(inode->i_sb, inode, 1);
-	inode->i_blocks -= inode->i_sb->s_blocksize >> 9;
-}
-
-static inline void
-ext3_xattr_free_block(handle_t *handle, struct inode * inode,
-		      unsigned long block)
-{
-	ext3_free_blocks(handle, inode, block, 1);
-	inode->i_blocks -= inode->i_sb->s_blocksize >> 9;
-}
-
-#else
-# define ext3_xattr_quota_free(inode) \
-	DQUOT_FREE_BLOCK(inode, 1)
-# define ext3_xattr_free_block(handle, inode, block) \
-	ext3_free_blocks(handle, inode, block, 1)
-#endif
-
+static DECLARE_MUTEX(ext3_xattr_sem);
 struct ext3_xattr_handler *ext3_xattr_handlers[EXT3_XATTR_INDEX_MAX];
 rwlock_t ext3_handler_lock = RW_LOCK_UNLOCKED;
 
@@ -575,8 +506,8 @@
 	name_len = strlen(name);
 	if (name_len > 255 || value_len > sb->s_blocksize)
 		return -ERANGE;
-	ext3_xattr_lock();
 
+	down(&ext3_xattr_sem);
 	if (EXT3_I(inode)->i_file_acl) {
 		/* The inode already has an extended attribute block. */
 		int block = EXT3_I(inode)->i_file_acl;
@@ -775,7 +706,7 @@
 	brelse(bh);
 	if (!(bh && header == HDR(bh)))
 		kfree(header);
-	ext3_xattr_unlock();
+	up(&ext3_xattr_sem);
 
 	return error;
 }
@@ -803,7 +734,8 @@
 				new_bh->b_blocknr);
 			
 			error = -EDQUOT;
-			if (ext3_xattr_quota_alloc(inode, 1))
+			/* How can we enforce the allocation? */
+			if (DQUOT_ALLOC_BLOCK(inode, 1))
 				goto cleanup;
 			
 			error = ext3_journal_get_write_access(handle, new_bh);
@@ -819,16 +751,18 @@
 			ext3_xattr_cache_insert(new_bh);
 		} else {
 			/* We need to allocate a new block */
-			int force = EXT3_I(inode)->i_file_acl != 0;
-			int block = ext3_xattr_new_block(handle, inode,
-							 &error, force);
+			int goal = le32_to_cpu(EXT3_SB(sb)->s_es->s_first_data_block) +
+					EXT3_I(inode)->i_block_group * EXT3_BLOCKS_PER_GROUP(sb);
+			/* How can we enforce the allocation? */
+			int block = ext3_new_block(handle, inode, goal, 0, 0, &error);
 			if (error)
 				goto cleanup;
 			ea_idebug(inode, "creating block %d", block);
 
 			new_bh = sb_getblk(sb, block);
 			if (!new_bh) {
-getblk_failed:			ext3_xattr_free_block(handle, inode, block);
+getblk_failed:
+				ext3_free_blocks(handle, inode, block, 1);
 				error = -EIO;
 				goto cleanup;
 			}
@@ -871,7 +805,7 @@
 		if (refcount == 1) {
 			/* Free the old block. */
 			ea_bdebug(old_bh, "freeing");
-			ext3_xattr_free_block(handle, inode, old_bh->b_blocknr);
+			ext3_free_blocks(handle, inode, old_bh->b_blocknr, 1);
 
 			/* ext3_forget() calls bforget() for us, but we
 			   let our caller release old_bh, so we need to
@@ -882,7 +816,7 @@
 			/* Decrement the refcount only. */
 			refcount--;
 			HDR(old_bh)->h_refcount = cpu_to_le32(refcount);
-			ext3_xattr_quota_free(inode);
+			DQUOT_FREE_BLOCK(inode, 1);
 			ext3_journal_dirty_metadata(handle, old_bh);
 			ea_bdebug(old_bh, "refcount now=%d", refcount);
 		}
@@ -909,8 +843,8 @@
 
 	if (!block)
 		return;
-	ext3_xattr_lock();
 
+	down(&ext3_xattr_sem);
 	bh = sb_bread(inode->i_sb, block);
 	if (!bh) {
 		ext3_error(inode->i_sb, "ext3_xattr_delete_inode",
@@ -928,7 +862,7 @@
 	ea_bdebug(bh, "refcount now=%d", le32_to_cpu(HDR(bh)->h_refcount) - 1);
 	if (HDR(bh)->h_refcount == cpu_to_le32(1)) {
 		ext3_xattr_cache_remove(bh);
-		ext3_xattr_free_block(handle, inode, block);
+		ext3_free_blocks(handle, inode, block, 1);
 		ext3_forget(handle, 1, inode, bh, block);
 		bh = NULL;
 	} else {
@@ -937,13 +871,13 @@
 		ext3_journal_dirty_metadata(handle, bh);
 		if (IS_SYNC(inode))
 			handle->h_sync = 1;
-		ext3_xattr_quota_free(inode);
+		DQUOT_FREE_BLOCK(inode, 1);
 	}
 	EXT3_I(inode)->i_file_acl = 0;
 
 cleanup:
 	brelse(bh);
-	ext3_xattr_unlock();
+	up(&ext3_xattr_sem);
 }
 
 /*
--- linux-2.5.42-mm3-plain/fs/ext3/xattr.h	Thu Jan  1 01:00:00 1970
+++ linux-2.5.42-mm3/fs/ext3/xattr.h	Tue Oct 15 13:45:27 2002
@@ -0,0 +1,136 @@
+/*
+  File: fs/ext3/xattr.h
+
+  On-disk format of extended attributes for the ext3 filesystem.
+
+  (C) 2001 Andreas Gruenbacher, <a.gruenbacher@computer.org>
+*/
+
+#include <linux/config.h>
+#include <linux/xattr.h>
+
+/* Magic value in attribute blocks */
+#define EXT3_XATTR_MAGIC		0xEA020000
+
+/* Maximum number of references to one attribute block */
+#define EXT3_XATTR_REFCOUNT_MAX		1024
+
+/* Name indexes */
+#define EXT3_XATTR_INDEX_MAX			10
+#define EXT3_XATTR_INDEX_USER			1
+#define EXT3_XATTR_INDEX_POSIX_ACL_ACCESS	2
+#define EXT3_XATTR_INDEX_POSIX_ACL_DEFAULT	3
+
+struct ext3_xattr_header {
+	__u32	h_magic;	/* magic number for identification */
+	__u32	h_refcount;	/* reference count */
+	__u32	h_blocks;	/* number of disk blocks used */
+	__u32	h_hash;		/* hash value of all attributes */
+	__u32	h_reserved[4];	/* zero right now */
+};
+
+struct ext3_xattr_entry {
+	__u8	e_name_len;	/* length of name */
+	__u8	e_name_index;	/* attribute name index */
+	__u16	e_value_offs;	/* offset in disk block of value */
+	__u32	e_value_block;	/* disk block attribute is stored on (n/i) */
+	__u32	e_value_size;	/* size of attribute value */
+	__u32	e_hash;		/* hash value of name and value */
+	char	e_name[0];	/* attribute name */
+};
+
+#define EXT3_XATTR_PAD_BITS		2
+#define EXT3_XATTR_PAD		(1<<EXT3_XATTR_PAD_BITS)
+#define EXT3_XATTR_ROUND		(EXT3_XATTR_PAD-1)
+#define EXT3_XATTR_LEN(name_len) \
+	(((name_len) + EXT3_XATTR_ROUND + \
+	sizeof(struct ext3_xattr_entry)) & ~EXT3_XATTR_ROUND)
+#define EXT3_XATTR_NEXT(entry) \
+	( (struct ext3_xattr_entry *)( \
+	  (char *)(entry) + EXT3_XATTR_LEN((entry)->e_name_len)) )
+#define EXT3_XATTR_SIZE(size) \
+	(((size) + EXT3_XATTR_ROUND) & ~EXT3_XATTR_ROUND)
+
+# ifdef CONFIG_EXT3_FS_XATTR
+
+struct ext3_xattr_handler {
+	char *prefix;
+	size_t (*list)(char *list, struct inode *inode, const char *name,
+		       int name_len);
+	int (*get)(struct inode *inode, const char *name, void *buffer,
+		   size_t size);
+	int (*set)(struct inode *inode, const char *name, const void *buffer,
+		   size_t size, int flags);
+};
+
+extern int ext3_xattr_register(int, struct ext3_xattr_handler *);
+extern void ext3_xattr_unregister(int, struct ext3_xattr_handler *);
+
+extern int ext3_setxattr(struct dentry *, const char *, void *, size_t, int);
+extern ssize_t ext3_getxattr(struct dentry *, const char *, void *, size_t);
+extern ssize_t ext3_listxattr(struct dentry *, char *, size_t);
+extern int ext3_removexattr(struct dentry *, const char *);
+
+extern int ext3_xattr_get(struct inode *, int, const char *, void *, size_t);
+extern int ext3_xattr_list(struct inode *, char *, size_t);
+extern int ext3_xattr_set(handle_t *handle, struct inode *, int, const char *, const void *, size_t, int);
+
+extern void ext3_xattr_delete_inode(handle_t *, struct inode *);
+extern void ext3_xattr_put_super(struct super_block *);
+
+extern int init_ext3_xattr(void);
+extern void exit_ext3_xattr(void);
+
+# else  /* CONFIG_EXT3_FS_XATTR */
+#  define ext3_setxattr		NULL
+#  define ext3_getxattr		NULL
+#  define ext3_listxattr	NULL
+#  define ext3_removexattr	NULL
+
+static inline int
+ext3_xattr_get(struct inode *inode, int name_index, const char *name,
+	       void *buffer, size_t size, int flags)
+{
+	return -ENOTSUPP;
+}
+
+static inline int
+ext3_xattr_list(struct inode *inode, void *buffer, size_t size, int flags)
+{
+	return -ENOTSUPP;
+}
+
+static inline int
+ext3_xattr_set(handle_t *handle, struct inode *inode, int name_index,
+	       const char *name, const void *value, size_t size, int flags)
+{
+	return -ENOTSUPP;
+}
+
+static inline void
+ext3_xattr_delete_inode(handle_t *handle, struct inode *inode)
+{
+}
+
+static inline void
+ext3_xattr_put_super(struct super_block *sb)
+{
+}
+
+static inline int
+init_ext3_xattr(void)
+{
+	return 0;
+}
+
+static inline void
+exit_ext3_xattr(void)
+{
+}
+
+# endif  /* CONFIG_EXT3_FS_XATTR */
+
+extern struct ext3_xattr_handler ext3_xattr_user_handler;
+
+
+
--- linux-2.5.42-mm3-plain/fs/ext3/xattr_user.c	Tue Oct 15 17:05:09 2002
+++ linux-2.5.42-mm3/fs/ext3/xattr_user.c	Tue Oct 15 16:13:54 2002
@@ -11,7 +11,7 @@
 #include <linux/smp_lock.h>
 #include <linux/ext3_jbd.h>
 #include <linux/ext3_fs.h>
-#include <linux/ext3_xattr.h>
+#include "xattr.h"
 
 #ifdef CONFIG_EXT3_FS_POSIX_ACL
 # include <linux/ext3_acl.h>
--- linux-2.5.42-mm3-plain/include/linux/ext3_xattr.h	Tue Oct 15 17:05:12 2002
+++ linux-2.5.42-mm3/include/linux/ext3_xattr.h	Thu Jan  1 01:00:00 1970
@@ -1,137 +0,0 @@
-/*
-  File: linux/ext3_xattr.h
-
-  On-disk format of extended attributes for the ext3 filesystem.
-
-  (C) 2001 Andreas Gruenbacher, <a.gruenbacher@computer.org>
-*/
-
-#include <linux/config.h>
-#include <linux/init.h>
-#include <linux/xattr.h>
-
-/* Magic value in attribute blocks */
-#define EXT3_XATTR_MAGIC		0xEA020000
-
-/* Maximum number of references to one attribute block */
-#define EXT3_XATTR_REFCOUNT_MAX		1024
-
-/* Name indexes */
-#define EXT3_XATTR_INDEX_MAX			10
-#define EXT3_XATTR_INDEX_USER			1
-#define EXT3_XATTR_INDEX_POSIX_ACL_ACCESS	2
-#define EXT3_XATTR_INDEX_POSIX_ACL_DEFAULT	3
-
-struct ext3_xattr_header {
-	__u32	h_magic;	/* magic number for identification */
-	__u32	h_refcount;	/* reference count */
-	__u32	h_blocks;	/* number of disk blocks used */
-	__u32	h_hash;		/* hash value of all attributes */
-	__u32	h_reserved[4];	/* zero right now */
-};
-
-struct ext3_xattr_entry {
-	__u8	e_name_len;	/* length of name */
-	__u8	e_name_index;	/* attribute name index */
-	__u16	e_value_offs;	/* offset in disk block of value */
-	__u32	e_value_block;	/* disk block attribute is stored on (n/i) */
-	__u32	e_value_size;	/* size of attribute value */
-	__u32	e_hash;		/* hash value of name and value */
-	char	e_name[0];	/* attribute name */
-};
-
-#define EXT3_XATTR_PAD_BITS		2
-#define EXT3_XATTR_PAD		(1<<EXT3_XATTR_PAD_BITS)
-#define EXT3_XATTR_ROUND		(EXT3_XATTR_PAD-1)
-#define EXT3_XATTR_LEN(name_len) \
-	(((name_len) + EXT3_XATTR_ROUND + \
-	sizeof(struct ext3_xattr_entry)) & ~EXT3_XATTR_ROUND)
-#define EXT3_XATTR_NEXT(entry) \
-	( (struct ext3_xattr_entry *)( \
-	  (char *)(entry) + EXT3_XATTR_LEN((entry)->e_name_len)) )
-#define EXT3_XATTR_SIZE(size) \
-	(((size) + EXT3_XATTR_ROUND) & ~EXT3_XATTR_ROUND)
-
-# ifdef CONFIG_EXT3_FS_XATTR
-
-struct ext3_xattr_handler {
-	char *prefix;
-	size_t (*list)(char *list, struct inode *inode, const char *name,
-		       int name_len);
-	int (*get)(struct inode *inode, const char *name, void *buffer,
-		   size_t size);
-	int (*set)(struct inode *inode, const char *name, const void *buffer,
-		   size_t size, int flags);
-};
-
-extern int ext3_xattr_register(int, struct ext3_xattr_handler *);
-extern void ext3_xattr_unregister(int, struct ext3_xattr_handler *);
-
-extern int ext3_setxattr(struct dentry *, const char *, void *, size_t, int);
-extern ssize_t ext3_getxattr(struct dentry *, const char *, void *, size_t);
-extern ssize_t ext3_listxattr(struct dentry *, char *, size_t);
-extern int ext3_removexattr(struct dentry *, const char *);
-
-extern int ext3_xattr_get(struct inode *, int, const char *, void *, size_t);
-extern int ext3_xattr_list(struct inode *, char *, size_t);
-extern int ext3_xattr_set(handle_t *handle, struct inode *, int, const char *, const void *, size_t, int);
-
-extern void ext3_xattr_delete_inode(handle_t *, struct inode *);
-extern void ext3_xattr_put_super(struct super_block *);
-
-extern int init_ext3_xattr(void) __init;
-extern void exit_ext3_xattr(void);
-
-# else  /* CONFIG_EXT3_FS_XATTR */
-#  define ext3_setxattr		NULL
-#  define ext3_getxattr		NULL
-#  define ext3_listxattr	NULL
-#  define ext3_removexattr	NULL
-
-static inline int
-ext3_xattr_get(struct inode *inode, int name_index, const char *name,
-	       void *buffer, size_t size, int flags)
-{
-	return -ENOTSUP;
-}
-
-static inline int
-ext3_xattr_list(struct inode *inode, void *buffer, size_t size, int flags)
-{
-	return -ENOTSUP;
-}
-
-static inline int
-ext3_xattr_set(handle_t *handle, struct inode *inode, int name_index,
-	       const char *name, const void *value, size_t size, int flags)
-{
-	return -ENOTSUP;
-}
-
-static inline void
-ext3_xattr_delete_inode(handle_t *handle, struct inode *inode)
-{
-}
-
-static inline void
-ext3_xattr_put_super(struct super_block *sb)
-{
-}
-
-static inline int
-init_ext3_xattr(void)
-{
-	return 0;
-}
-
-static inline void
-exit_ext3_xattr(void)
-{
-}
-
-# endif  /* CONFIG_EXT3_FS_XATTR */
-
-extern struct ext3_xattr_handler ext3_xattr_user_handler;
-
-
-

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15 15:31     ` Christoph Hellwig
@ 2002-10-15 16:04       ` Andreas Dilger
  2002-10-15 16:16         ` Christoph Hellwig
  2002-10-15 16:41       ` Theodore Ts'o
  1 sibling, 1 reply; 18+ messages in thread
From: Andreas Dilger @ 2002-10-15 16:04 UTC (permalink / raw)
  To: Christoph Hellwig, Theodore Ts'o, linux-kernel

On Oct 15, 2002  16:31 +0100, Christoph Hellwig wrote:
> Patch 2: mbcache (all against 2.5.42-mm3):
> 
> --- linux-2.5.42-mm3-plain/fs/mbcache.c	Tue Oct 15 17:05:09 2002
> +++ linux-2.5.42-mm3/fs/mbcache.c	Tue Oct 15 17:26:53 2002
> @@ -247,20 +228,20 @@
>  	}
>  	while (nr_to_scan && !list_empty(&mb_cache_lru_list)) {
>  		struct mb_cache_entry *ce =
>  			list_entry(mb_cache_lru_list.prev,
>  				   struct mb_cache_entry, e_lru_list);
> @@ -382,20 +361,19 @@
>  	LIST_HEAD(free_list);
>  	struct list_head *l;
>  
> -	mb_cache_lock();
> +	spin_lock(&mb_cache_spinlock);
>  	l = mb_cache_lru_list.prev;
>  	while (l != &mb_cache_lru_list) {
>  		struct mb_cache_entry *ce =
>  			list_entry(l, struct mb_cache_entry, e_lru_list);
> @@ -420,15 +398,14 @@
>  	struct list_head *l;
>  	int n;
>  
> -	mb_cache_lock();
> +	spin_lock(&mb_cache_spinlock);
>  	l = mb_cache_lru_list.prev;
>  	while (l != &mb_cache_lru_list) {
>  		struct mb_cache_entry *ce =
>  			list_entry(l, struct mb_cache_entry, e_lru_list);

Couldn't these all be "list_for_each{_safe}"?

Cheers, Andreas
--
Andreas Dilger
http://www-mddsp.enel.ucalgary.ca/People/adilger/
http://sourceforge.net/projects/ext2resize/


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15 16:04       ` Andreas Dilger
@ 2002-10-15 16:16         ` Christoph Hellwig
  2002-10-15 16:21           ` Andreas Dilger
  0 siblings, 1 reply; 18+ messages in thread
From: Christoph Hellwig @ 2002-10-15 16:16 UTC (permalink / raw)
  To: Theodore Ts'o, linux-kernel

On Tue, Oct 15, 2002 at 10:04:17AM -0600, Andreas Dilger wrote:
> > -	mb_cache_lock();
> > +	spin_lock(&mb_cache_spinlock);
> >  	l = mb_cache_lru_list.prev;
> >  	while (l != &mb_cache_lru_list) {
> >  		struct mb_cache_entry *ce =
> >  			list_entry(l, struct mb_cache_entry, e_lru_list);
> 
> Couldn't these all be "list_for_each{_safe}"?

They'd have to be list_for_each_safe_prev, which is not currently
in list.h.  The EVMS folks have a patch to add it, though..


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15 16:16         ` Christoph Hellwig
@ 2002-10-15 16:21           ` Andreas Dilger
  2002-10-15 16:43             ` Andreas Gruenbacher
  0 siblings, 1 reply; 18+ messages in thread
From: Andreas Dilger @ 2002-10-15 16:21 UTC (permalink / raw)
  To: Christoph Hellwig, Theodore Ts'o, linux-kernel

On Oct 15, 2002  17:16 +0100, Christoph Hellwig wrote:
> On Tue, Oct 15, 2002 at 10:04:17AM -0600, Andreas Dilger wrote:
> > > -	mb_cache_lock();
> > > +	spin_lock(&mb_cache_spinlock);
> > >  	l = mb_cache_lru_list.prev;
> > >  	while (l != &mb_cache_lru_list) {
> > >  		struct mb_cache_entry *ce =
> > >  			list_entry(l, struct mb_cache_entry, e_lru_list);
> > 
> > Couldn't these all be "list_for_each{_safe}"?
> 
> They'd have to be list_for_each_safe_prev, which is not currently
> in list.h.  The EVMS folks have a patch to add it, though..

Is there a reason why the code can't just add items into the list in
the reverse order (i.e. list_add_tail()) and then walk in the normal
direction via list_for_each_safe()?

Cheers, Andreas
--
Andreas Dilger
http://www-mddsp.enel.ucalgary.ca/People/adilger/
http://sourceforge.net/projects/ext2resize/


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15 15:31     ` Christoph Hellwig
  2002-10-15 16:04       ` Andreas Dilger
@ 2002-10-15 16:41       ` Theodore Ts'o
  2002-10-15 16:59         ` Christoph Hellwig
  1 sibling, 1 reply; 18+ messages in thread
From: Theodore Ts'o @ 2002-10-15 16:41 UTC (permalink / raw)
  To: Christoph Hellwig, torvalds, linux-kernel

On Tue, Oct 15, 2002 at 04:31:21PM +0100, Christoph Hellwig wrote:
> 
> Patch 2: mbcache (all against 2.5.42-mm3):
> 
> o remove LINUX_VERSION_CODE mess

Note that this was fixed in the version I sent out; I don't think
Andrew grabbed my latest patches.  Anyway, I'll merge in the various
suggested changes, and send Andrew an updated patch.

						- Ted

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15 16:21           ` Andreas Dilger
@ 2002-10-15 16:43             ` Andreas Gruenbacher
  0 siblings, 0 replies; 18+ messages in thread
From: Andreas Gruenbacher @ 2002-10-15 16:43 UTC (permalink / raw)
  To: Andreas Dilger; +Cc: Christoph Hellwig, Theodore Ts'o, linux-kernel

On Tuesday 15 October 2002 18:21, Andreas Dilger wrote:
> Is there a reason why the code can't just add items into the list in
> the reverse order (i.e. list_add_tail()) and then walk in the normal
> direction via list_for_each_safe()?

No it could well be reversed.

--Andreas.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH 1/4] Add extended attributes to ext2/3
  2002-10-15 16:41       ` Theodore Ts'o
@ 2002-10-15 16:59         ` Christoph Hellwig
  0 siblings, 0 replies; 18+ messages in thread
From: Christoph Hellwig @ 2002-10-15 16:59 UTC (permalink / raw)
  To: Theodore Ts'o, Christoph Hellwig, torvalds, linux-kernel

On Tue, Oct 15, 2002 at 12:41:16PM -0400, Theodore Ts'o wrote:
> On Tue, Oct 15, 2002 at 04:31:21PM +0100, Christoph Hellwig wrote:
> > 
> > Patch 2: mbcache (all against 2.5.42-mm3):
> > 
> > o remove LINUX_VERSION_CODE mess
> 
> Note that this was fixed in the version I sent out; I don't think
> Andrew grabbed my latest patches.  Anyway, I'll merge in the various
> suggested changes, and send Andrew an updated patch.

Well, it'ß still in the patch series I that started this thread.
I haven't seen newer patches yet.


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2002-10-15 16:53 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2002-10-15  3:33 [PATCH 1/4] Add extended attributes to ext2/3 tytso
2002-10-15  4:46 ` Andrew Morton
2002-10-15  5:04   ` Theodore Ts'o
2002-10-15  5:31   ` Andreas Dilger
2002-10-15 13:05     ` Theodore Ts'o
2002-10-15 10:26 ` Olaf Dietsche
2002-10-15 11:58 ` Christoph Hellwig
2002-10-15 13:15   ` Theodore Ts'o
2002-10-15 15:25     ` Christoph Hellwig
2002-10-15 15:31     ` Christoph Hellwig
2002-10-15 16:04       ` Andreas Dilger
2002-10-15 16:16         ` Christoph Hellwig
2002-10-15 16:21           ` Andreas Dilger
2002-10-15 16:43             ` Andreas Gruenbacher
2002-10-15 16:41       ` Theodore Ts'o
2002-10-15 16:59         ` Christoph Hellwig
2002-10-15 15:33     ` Christoph Hellwig
2002-10-15 15:34     ` 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).