[Ocfs2-devel] [PATCH 3/3] ocfs2: modify reservation code to support discontigous localalloc

2012-05-07 Thread Srinivas Eeda
Currently reservation code assumes a bitmap given to it is all one contigous
chunk. This patch enhances it to handle a discontigous chunks. It adds new
fields m_bitmap_ext_cnt and m_bitmap_ext_arr. m_bitmap_ext_arr tracks the sizes
of each contigous free bits and m_bitmap_ext_cnt trackes number of
m_bitmap_ext_arr.

Signed-off-by: Srinivas Eeda srinivas.e...@oracle.com
---
 fs/ocfs2/reservations.c |   41 ++---
 fs/ocfs2/reservations.h |7 ++-
 2 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/fs/ocfs2/reservations.c b/fs/ocfs2/reservations.c
index 41ffd36..fea93d7 100644
--- a/fs/ocfs2/reservations.c
+++ b/fs/ocfs2/reservations.c
@@ -291,7 +291,15 @@ static void ocfs2_resmap_clear_all_resv(struct 
ocfs2_reservation_map *resmap)
}
 }
 
-void ocfs2_resmap_restart(struct ocfs2_reservation_map *resmap,
+void ocfs2_resmap_set_ext(struct ocfs2_reservation_map *resmap, int arr, u32 
sz)
+{
+   if (ocfs2_resmap_disabled(resmap))
+   return;
+
+   resmap-m_bitmap_ext_arr[arr] = sz;
+}
+
+void ocfs2_resmap_restart(struct ocfs2_reservation_map *resmap, u32 ext_cnt,
  unsigned int clen, char *disk_bitmap)
 {
if (ocfs2_resmap_disabled(resmap))
@@ -300,9 +308,21 @@ void ocfs2_resmap_restart(struct ocfs2_reservation_map 
*resmap,
spin_lock(resv_lock);
 
ocfs2_resmap_clear_all_resv(resmap);
+
+   /* free existing extent array */
+   if (resmap-m_bitmap_ext_arr)
+   kfree(resmap-m_bitmap_ext_arr);
+
resmap-m_bitmap_len = clen;
resmap-m_disk_bitmap = disk_bitmap;
 
+   resmap-m_bitmap_ext_cnt = ext_cnt;
+   resmap-m_bitmap_ext_arr = kmalloc((sizeof(u32) * ext_cnt), GFP_NOFS);
+   if (!resmap-m_bitmap_ext_arr) {
+   mlog_errno(-ENOMEM);
+   resmap-m_osb-osb_resv_level = 0;
+   }
+
spin_unlock(resv_lock);
 }
 
@@ -419,20 +439,26 @@ static int ocfs2_resmap_find_free_bits(struct 
ocfs2_reservation_map *resmap,
   unsigned int *rlen)
 {
void *bitmap = resmap-m_disk_bitmap;
-   unsigned int best_start, best_len = 0;
+   unsigned int best_start, len, ext, best_len = 0;
int offset, start, found;
 
trace_ocfs2_resmap_find_free_bits_begin(search_start, search_len,
wanted, resmap-m_bitmap_len);
 
-   found = best_start = best_len = 0;
-
+   found = best_start = best_len = ext = 0;
start = search_start;
+   len = resmap-m_bitmap_ext_arr[ext++];
while ((offset = ocfs2_find_next_zero_bit(bitmap, resmap-m_bitmap_len,
-start)) != -1) {
+ start)) != -1) {
/* Search reached end of the region */
if (offset = (search_start + search_len))
-   break;
+   goto out;
+
+   if (offset = len) {
+   len += resmap-m_bitmap_ext_arr[ext];
+   found = 1;
+   start = offset + 1;
+   }
 
if (offset == start) {
/* we found a zero */
@@ -450,9 +476,10 @@ static int ocfs2_resmap_find_free_bits(struct 
ocfs2_reservation_map *resmap,
}
 
if (found = wanted)
-   break;
+   goto out;
}
 
+out:
if (best_len == 0)
return 0;
 
diff --git a/fs/ocfs2/reservations.h b/fs/ocfs2/reservations.h
index 42c2b80..bb5e94f 100644
--- a/fs/ocfs2/reservations.h
+++ b/fs/ocfs2/reservations.h
@@ -56,6 +56,8 @@ struct ocfs2_reservation_map {
u32 m_bitmap_len;   /* Number of valid
 * bits available */
 
+   u32 m_bitmap_ext_cnt;
+   u32 *m_bitmap_ext_arr;
struct list_headm_lru;  /* LRU of reservations
 * structures. */
 
@@ -94,6 +96,9 @@ void ocfs2_resv_discard(struct ocfs2_reservation_map *resmap,
 int ocfs2_resmap_init(struct ocfs2_super *osb,
  struct ocfs2_reservation_map *resmap);
 
+void ocfs2_resmap_set_ext(struct ocfs2_reservation_map *resmap, int arr,
+ u32 sz);
+
 /**
  * ocfs2_resmap_restart() - restart a reservation bitmap
  * @resmap: reservations bitmap
@@ -107,7 +112,7 @@ int ocfs2_resmap_init(struct ocfs2_super *osb,
  * reservations. A future version will recalculate existing
  * reservations based on the new bitmap.
  */
-void ocfs2_resmap_restart(struct ocfs2_reservation_map *resmap,
+void ocfs2_resmap_restart(struct ocfs2_reservation_map *resmap, u32 ext_cnt,
  unsigned int clen, char *disk_bitmap);
 
 /**
-- 
1.5.4.3



[Ocfs2-devel] [PATCH 1/3] ocfs2: new structure to implment discontiguous local alloc bitmap

2012-05-07 Thread Srinivas Eeda
Current local alloc handles single contiguous free chunk of clusters. This
patch enhances local alloc to handle discontigous free chunks. It adds a new
ocfs2_local_alloc_rec structure which tracks single contiguous free chunk. An
array of these sit in the bitmap itself and track discontiguous chunks. In
best case there is only one record and increases as the filesystem gets
fragmented. Number of records at a time are limited depending on the size
of the bitmap and the max limit is defined by OCFS2_MAX_LOCAL_ALLOC_RECS.

Signed-off-by: Srinivas Eeda srinivas.e...@oracle.com
---
 fs/ocfs2/localalloc.c |   10 ++
 fs/ocfs2/ocfs2.h  |8 
 fs/ocfs2/ocfs2_fs.h   |   48 ++--
 3 files changed, 60 insertions(+), 6 deletions(-)

diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
index 210c352..4190e53 100644
--- a/fs/ocfs2/localalloc.c
+++ b/fs/ocfs2/localalloc.c
@@ -48,6 +48,16 @@
 
 #define OCFS2_LOCAL_ALLOC(dinode)  (((dinode)-id2.i_lab))
 
+#define OCFS2_LOCAL_ALLOC_REC_SZ(la)   (le16_to_cpu(la-la_rec_count) *\
+sizeof(struct ocfs2_local_alloc_rec))
+#define OCFS2_LOCAL_ALLOC_BITMAP(la)((char *)((la-la_recs)) +\
+OCFS2_LOCAL_ALLOC_REC_SZ(la))
+#define OCFS2_LOCAL_ALLOC_BITS_PER_REC (sizeof(struct ocfs2_local_alloc_rec)*8)
+
+/* Maximum number of local alloc records */
+#define OCFS2_MAX_LOCAL_ALLOC_REC_LIMIT128
+
+
 static u32 ocfs2_local_alloc_count_bits(struct ocfs2_dinode *alloc);
 
 static int ocfs2_local_alloc_find_clear_bits(struct ocfs2_super *osb,
diff --git a/fs/ocfs2/ocfs2.h b/fs/ocfs2/ocfs2.h
index d355e6e..d4c36d2 100644
--- a/fs/ocfs2/ocfs2.h
+++ b/fs/ocfs2/ocfs2.h
@@ -367,6 +367,7 @@ struct ocfs2_super
 * by osb_lock */
 
struct buffer_head *local_alloc_bh;
+   struct inode   *local_alloc_inode;
 
u64 la_last_gd;
 
@@ -522,6 +523,13 @@ static inline int ocfs2_supports_discontig_bg(struct 
ocfs2_super *osb)
return 0;
 }
 
+static inline int ocfs2_supports_discontig_la(struct ocfs2_super *osb)
+{
+   if (osb-s_feature_incompat  OCFS2_FEATURE_INCOMPAT_DISCONTIG_LA)
+   return 1;
+   return 0;
+}
+
 static inline unsigned int ocfs2_link_max(struct ocfs2_super *osb)
 {
if (ocfs2_supports_indexed_dirs(osb))
diff --git a/fs/ocfs2/ocfs2_fs.h b/fs/ocfs2/ocfs2_fs.h
index 938387a..6a0fe02 100644
--- a/fs/ocfs2/ocfs2_fs.h
+++ b/fs/ocfs2/ocfs2_fs.h
@@ -102,7 +102,8 @@
 | OCFS2_FEATURE_INCOMPAT_INDEXED_DIRS \
 | OCFS2_FEATURE_INCOMPAT_REFCOUNT_TREE 
\
 | OCFS2_FEATURE_INCOMPAT_DISCONTIG_BG  
\
-| OCFS2_FEATURE_INCOMPAT_CLUSTERINFO)
+| OCFS2_FEATURE_INCOMPAT_CLUSTERINFO \
+| OCFS2_FEATURE_INCOMPAT_DISCONTIG_LA)
 #define OCFS2_FEATURE_RO_COMPAT_SUPP   (OCFS2_FEATURE_RO_COMPAT_UNWRITTEN \
 | OCFS2_FEATURE_RO_COMPAT_USRQUOTA \
 | OCFS2_FEATURE_RO_COMPAT_GRPQUOTA)
@@ -177,6 +178,9 @@
  */
 #define OCFS2_FEATURE_INCOMPAT_CLUSTERINFO 0x4000
 
+/* Discontiguous local alloc */
+#define OCFS2_FEATURE_INCOMPAT_DISCONTIG_LA0x8000
+
 /*
  * backup superblock flag is used to indicate that this volume
  * has backup superblocks.
@@ -664,14 +668,19 @@ struct ocfs2_super_block {
  * Local allocation bitmap for OCFS2 slots
  * Note that it exists inside an ocfs2_dinode, so all offsets are
  * relative to the start of ocfs2_dinode.id2.
+ * Each ocfs2_local_alloc_rec tracks one contigous chunk of clusters.
  */
+struct ocfs2_local_alloc_rec {
+   __le32 la_start;/* 1st cluster in this extent */
+   __le32 la_clusters; /* Number of contiguous clusters */
+};
+
 struct ocfs2_local_alloc
 {
 /*00*/ __le32 la_bm_off;   /* Starting bit offset in main bitmap */
__le16 la_size; /* Size of included bitmap, in bytes */
-   __le16 la_reserved1;
-   __le64 la_reserved2;
-/*10*/ __u8   la_bitmap[0];
+   __le16 la_rec_count;/* Number of discontiguous records */
+   struct ocfs2_local_alloc_rec la_recs[0]; /* Localalloc records */
 };
 
 /*
@@ -1380,11 +1389,24 @@ static inline u16 ocfs2_local_alloc_size(struct 
super_block *sb)
u16 size;
 
size = sb-s_blocksize -
-   offsetof(struct ocfs2_dinode, id2.i_lab.la_bitmap);
+   offsetof(struct ocfs2_dinode, id2.i_lab.la_recs);
+   size -= sizeof(struct ocfs2_local_alloc_rec);
 
return size;
 }
 
+/* effectively this is also the bitmap size */
+static inline u32 ocfs2_local_alloc_cluster_count(struct ocfs2_local_alloc *la)
+{
+   u32 i, clusters;
+
+   clusters = 0;
+   for (i 

[Ocfs2-devel] ocfs2 discontiguous localalloc patches

2012-05-07 Thread Srinivas Eeda
Hi all,

can you please review following 3 patches that implement discontiguous
localalloc bitmap support for ocfs2 file system. This feature helps
applications that significantly fragment the filesystem.

These fixes needs changes to ocfs2 tools as well. I am sending those patches
for review separately.

A write up on this feature is available at
http://oss.oracle.com/osswiki/OCFS2/DesignDocs/DiscontiguousLocalAlloc.html

Thanks,
--Srini


___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel


[Ocfs2-devel] [PATCH 2/3] ocfs2: implement discontiguous localalloc bitmap

2012-05-07 Thread Srinivas Eeda
This patch adds supporting functions and modifies localalloc code to implement
discontiguous localalloc bitmap.

Signed-off-by: Srinivas Eeda srinivas.e...@oracle.com
---
 fs/ocfs2/localalloc.c |  523 -
 1 files changed, 342 insertions(+), 181 deletions(-)

diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
index 4190e53..f63381e 100644
--- a/fs/ocfs2/localalloc.c
+++ b/fs/ocfs2/localalloc.c
@@ -48,6 +48,9 @@
 
 #define OCFS2_LOCAL_ALLOC(dinode)  (((dinode)-id2.i_lab))
 
+/* defines minimum contiguous required */
+#define OCFS2_LOCAL_ALLOC_MIN_BITS 2
+
 #define OCFS2_LOCAL_ALLOC_REC_SZ(la)   (le16_to_cpu(la-la_rec_count) *\
 sizeof(struct ocfs2_local_alloc_rec))
 #define OCFS2_LOCAL_ALLOC_BITMAP(la)((char *)((la-la_recs)) +\
@@ -58,7 +61,8 @@
 #define OCFS2_MAX_LOCAL_ALLOC_REC_LIMIT128
 
 
-static u32 ocfs2_local_alloc_count_bits(struct ocfs2_dinode *alloc);
+static u32 ocfs2_local_alloc_count_bits(struct ocfs2_super *osb,
+   struct ocfs2_dinode *alloc);
 
 static int ocfs2_local_alloc_find_clear_bits(struct ocfs2_super *osb,
 struct ocfs2_dinode *alloc,
@@ -82,8 +86,7 @@ static int ocfs2_local_alloc_new_window(struct ocfs2_super 
*osb,
handle_t *handle,
struct ocfs2_alloc_context *ac);
 
-static int ocfs2_local_alloc_slide_window(struct ocfs2_super *osb,
- struct inode *local_alloc_inode);
+static int ocfs2_local_alloc_slide_window(struct ocfs2_super *osb);
 
 /*
  * ocfs2_la_default_mb() - determine a default size, in megabytes of
@@ -202,6 +205,74 @@ unsigned int ocfs2_la_default_mb(struct ocfs2_super *osb)
return la_mb;
 }
 
+static u32 ocfs2_local_bitmap_to_cluster(struct ocfs2_local_alloc *la, u32 bit)
+{
+   u32 start, prev, offset;
+   int rec;
+
+   rec = start = prev = 0;
+   for (rec = 0; rec  le16_to_cpu(la-la_rec_count); rec++) {
+   prev = start;
+   start += le32_to_cpu(la-la_recs[rec].la_clusters);
+   if (bit  start)
+   break;
+   }
+   offset = le32_to_cpu(la-la_recs[rec].la_start) + (bit - prev);
+
+   return offset;
+}
+
+/*
+ * This function is called before allocating a new chunk for the localalloc
+ * bitmap to make sure there is enough space in the bitmap for the new record
+ */
+static u32 ocfs2_local_alloc_adjust_bits_wanted(struct ocfs2_local_alloc *la,
+   struct ocfs2_alloc_context *ac)
+{
+   u32 required, available, cluster_cnt;
+
+   if (ac-ac_bits_given == ac-ac_bits_wanted)
+   return 0;
+
+   /* total bits available in bitmap */
+   available   = le16_to_cpu(la-la_size)  3;
+   cluster_cnt = ocfs2_local_alloc_cluster_count(la);
+
+   /*
+* Wanted shouldn't be greater than bitmap size and given should be
+* equal to cluster count
+*/
+   BUG_ON(ac-ac_bits_given  ac-ac_bits_wanted);
+   BUG_ON(ac-ac_bits_wanted  available);
+   BUG_ON(ac-ac_bits_given != cluster_cnt);
+
+   /* reduce bits taken by each record structure */
+   available -= (le16_to_cpu(la-la_rec_count) *
+ OCFS2_LOCAL_ALLOC_BITS_PER_REC);
+
+   /* reduce space reserved for bitmap for already allocated clusters */
+   available -= cluster_cnt;
+
+   /* if available bits are not enough to fit a new record return 0 */
+   if (available  (OCFS2_LOCAL_ALLOC_BITS_PER_REC + 1))
+   return 0;
+
+   /* Adjust space that will be consumed by new record structure */
+   available -= OCFS2_LOCAL_ALLOC_BITS_PER_REC;
+
+   required = ac-ac_bits_wanted - ac-ac_bits_given;
+
+   /*
+* we can't allocate clusters more than the bits available. Adjust
+* bits wanted
+*/
+   if (required  available) {
+   ac-ac_bits_wanted = ac-ac_bits_given + available;
+   return available;
+   } else
+   return required;
+}
+
 void ocfs2_la_set_sizes(struct ocfs2_super *osb, int requested_mb)
 {
struct super_block *sb = osb-sb;
@@ -239,12 +310,14 @@ void ocfs2_local_alloc_seen_free_bits(struct ocfs2_super 
*osb,
  unsigned int num_clusters)
 {
spin_lock(osb-osb_lock);
-   if (osb-local_alloc_state == OCFS2_LA_DISABLED ||
-   osb-local_alloc_state == OCFS2_LA_THROTTLED)
-   if (num_clusters = osb-local_alloc_default_bits) {
-   cancel_delayed_work(osb-la_enable_wq);
+   if (osb-local_alloc_state == OCFS2_LA_DISABLED) {
+   cancel_delayed_work(osb-la_enable_wq);
+   if (num_clusters = osb-local_alloc_bits)
+   osb-local_alloc_state = 

Re: [Ocfs2-devel] ocfs2 discontiguous localalloc patches

2012-05-07 Thread Joel Becker
On Mon, May 07, 2012 at 04:21:27PM -0700, Srinivas Eeda wrote:
 can you please review following 3 patches that implement discontiguous
 localalloc bitmap support for ocfs2 file system. This feature helps
 applications that significantly fragment the filesystem.

Hi Srini.  Have you some performance numbers backing this?  That
is, I believe that the described filesystem turned off local alloc.  Do
you have proof that these patches, turning it back on, improved the
customer's performance?

Joel

-- 

But all my words come back to me
 In shades of mediocrity.
 Like emptiness in harmony
 I need someone to comfort me.

http://www.jlbec.org/
jl...@evilplan.org

___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 1/3] ocfs2: new structure to implment discontiguous local alloc bitmap

2012-05-07 Thread Joel Becker
On Mon, May 07, 2012 at 04:21:28PM -0700, Srinivas Eeda wrote:
 Current local alloc handles single contiguous free chunk of clusters. This
 patch enhances local alloc to handle discontigous free chunks. It adds a new
 ocfs2_local_alloc_rec structure which tracks single contiguous free chunk. An
 array of these sit in the bitmap itself and track discontiguous chunks. In
 best case there is only one record and increases as the filesystem gets
 fragmented. Number of records at a time are limited depending on the size
 of the bitmap and the max limit is defined by OCFS2_MAX_LOCAL_ALLOC_RECS.
 
 Signed-off-by: Srinivas Eeda srinivas.e...@oracle.com
 ---
  fs/ocfs2/localalloc.c |   10 ++
  fs/ocfs2/ocfs2.h  |8 
  fs/ocfs2/ocfs2_fs.h   |   48 ++--
  3 files changed, 60 insertions(+), 6 deletions(-)
 
 diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
 index 210c352..4190e53 100644
 --- a/fs/ocfs2/localalloc.c
 +++ b/fs/ocfs2/localalloc.c
 @@ -48,6 +48,16 @@
  
  #define OCFS2_LOCAL_ALLOC(dinode)(((dinode)-id2.i_lab))
  
 +#define OCFS2_LOCAL_ALLOC_REC_SZ(la) (le16_to_cpu(la-la_rec_count) *\
 +  sizeof(struct ocfs2_local_alloc_rec))
 +#define OCFS2_LOCAL_ALLOC_BITMAP(la)((char *)((la-la_recs)) +\
 +  OCFS2_LOCAL_ALLOC_REC_SZ(la))
 +#define OCFS2_LOCAL_ALLOC_BITS_PER_REC (sizeof(struct 
 ocfs2_local_alloc_rec)*8)
 +
 +/* Maximum number of local alloc records */
 +#define OCFS2_MAX_LOCAL_ALLOC_REC_LIMIT  128
 +
 +
  static u32 ocfs2_local_alloc_count_bits(struct ocfs2_dinode *alloc);
  
  static int ocfs2_local_alloc_find_clear_bits(struct ocfs2_super *osb,
 diff --git a/fs/ocfs2/ocfs2.h b/fs/ocfs2/ocfs2.h
 index d355e6e..d4c36d2 100644
 --- a/fs/ocfs2/ocfs2.h
 +++ b/fs/ocfs2/ocfs2.h
 @@ -367,6 +367,7 @@ struct ocfs2_super
* by osb_lock */
  
   struct buffer_head *local_alloc_bh;
 + struct inode   *local_alloc_inode;
  
   u64 la_last_gd;
  
 @@ -522,6 +523,13 @@ static inline int ocfs2_supports_discontig_bg(struct 
 ocfs2_super *osb)
   return 0;
  }
  
 +static inline int ocfs2_supports_discontig_la(struct ocfs2_super *osb)
 +{
 + if (osb-s_feature_incompat  OCFS2_FEATURE_INCOMPAT_DISCONTIG_LA)
 + return 1;
 + return 0;
 +}
 +
  static inline unsigned int ocfs2_link_max(struct ocfs2_super *osb)
  {
   if (ocfs2_supports_indexed_dirs(osb))
 diff --git a/fs/ocfs2/ocfs2_fs.h b/fs/ocfs2/ocfs2_fs.h
 index 938387a..6a0fe02 100644
 --- a/fs/ocfs2/ocfs2_fs.h
 +++ b/fs/ocfs2/ocfs2_fs.h
 @@ -102,7 +102,8 @@
| OCFS2_FEATURE_INCOMPAT_INDEXED_DIRS \
| OCFS2_FEATURE_INCOMPAT_REFCOUNT_TREE 
 \
| OCFS2_FEATURE_INCOMPAT_DISCONTIG_BG  
 \
 -  | OCFS2_FEATURE_INCOMPAT_CLUSTERINFO)
 +  | OCFS2_FEATURE_INCOMPAT_CLUSTERINFO \
 +  | OCFS2_FEATURE_INCOMPAT_DISCONTIG_LA)
  #define OCFS2_FEATURE_RO_COMPAT_SUPP (OCFS2_FEATURE_RO_COMPAT_UNWRITTEN \
| OCFS2_FEATURE_RO_COMPAT_USRQUOTA \
| OCFS2_FEATURE_RO_COMPAT_GRPQUOTA)
 @@ -177,6 +178,9 @@
   */
  #define OCFS2_FEATURE_INCOMPAT_CLUSTERINFO   0x4000
  
 +/* Discontiguous local alloc */
 +#define OCFS2_FEATURE_INCOMPAT_DISCONTIG_LA  0x8000

I really wish this could be an RO_COMPAT flag, but I think that
recovery on RO mounts will break with this.  Mark, please confirm, but I
think it has to be INCOMPAT.

 @@ -664,14 +668,19 @@ struct ocfs2_super_block {
   * Local allocation bitmap for OCFS2 slots
   * Note that it exists inside an ocfs2_dinode, so all offsets are
   * relative to the start of ocfs2_dinode.id2.
 + * Each ocfs2_local_alloc_rec tracks one contigous chunk of clusters.
   */
 +struct ocfs2_local_alloc_rec {
 + __le32 la_start;/* 1st cluster in this extent */
 + __le32 la_clusters; /* Number of contiguous clusters */
 +};
 +
  struct ocfs2_local_alloc
  {
  /*00*/   __le32 la_bm_off;   /* Starting bit offset in main bitmap */
   __le16 la_size; /* Size of included bitmap, in bytes */
 - __le16 la_reserved1;
 - __le64 la_reserved2;
 -/*10*/   __u8   la_bitmap[0];
 + __le16 la_rec_count;/* Number of discontiguous records */
 + struct ocfs2_local_alloc_rec la_recs[0]; /* Localalloc records */
  };

You can't delete la_bitmap.  Any filesystem without DISCONTIG_LA
will be expecting the inline bitmap to start there.

 @@ -1380,11 +1389,24 @@ static inline u16 ocfs2_local_alloc_size(struct 
 super_block *sb)
   u16 size;
  
   size = sb-s_blocksize -
 - offsetof(struct ocfs2_dinode, id2.i_lab.la_bitmap);
 + 

Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: implement discontiguous localalloc bitmap

2012-05-07 Thread Joel Becker
On Mon, May 07, 2012 at 04:21:29PM -0700, Srinivas Eeda wrote:
 This patch adds supporting functions and modifies localalloc code to implement
 discontiguous localalloc bitmap.
 
 Signed-off-by: Srinivas Eeda srinivas.e...@oracle.com
 ---
  fs/ocfs2/localalloc.c |  523 
 -
  1 files changed, 342 insertions(+), 181 deletions(-)
 
 diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
 index 4190e53..f63381e 100644
 --- a/fs/ocfs2/localalloc.c
 +++ b/fs/ocfs2/localalloc.c
 @@ -48,6 +48,9 @@
  
  #define OCFS2_LOCAL_ALLOC(dinode)(((dinode)-id2.i_lab))
  
 +/* defines minimum contiguous required */
 +#define OCFS2_LOCAL_ALLOC_MIN_BITS   2
 +
  #define OCFS2_LOCAL_ALLOC_REC_SZ(la) (le16_to_cpu(la-la_rec_count) *\
sizeof(struct ocfs2_local_alloc_rec))
  #define OCFS2_LOCAL_ALLOC_BITMAP(la)((char *)((la-la_recs)) +\
 @@ -58,7 +61,8 @@
  #define OCFS2_MAX_LOCAL_ALLOC_REC_LIMIT  128
  
  
 -static u32 ocfs2_local_alloc_count_bits(struct ocfs2_dinode *alloc);
 +static u32 ocfs2_local_alloc_count_bits(struct ocfs2_super *osb,
 + struct ocfs2_dinode *alloc);
  
  static int ocfs2_local_alloc_find_clear_bits(struct ocfs2_super *osb,
struct ocfs2_dinode *alloc,
 @@ -82,8 +86,7 @@ static int ocfs2_local_alloc_new_window(struct ocfs2_super 
 *osb,
   handle_t *handle,
   struct ocfs2_alloc_context *ac);
  
 -static int ocfs2_local_alloc_slide_window(struct ocfs2_super *osb,
 -   struct inode *local_alloc_inode);
 +static int ocfs2_local_alloc_slide_window(struct ocfs2_super *osb);

I noted that you moved local_alloc_inode into ocfs2_super in the
previous patch.  Lifting that into the super should be one distinct
patch.  It should add the field to ocfs2_super and change the function
signatures at the same time.  Munging it with other patches confuses the
issue.
 
 @@ -202,6 +205,74 @@ unsigned int ocfs2_la_default_mb(struct ocfs2_super *osb)
   return la_mb;
  }
  
 +static u32 ocfs2_local_bitmap_to_cluster(struct ocfs2_local_alloc *la, u32 
 bit)
 +{
 + u32 start, prev, offset;
 + int rec;
 +
 + rec = start = prev = 0;
 + for (rec = 0; rec  le16_to_cpu(la-la_rec_count); rec++) {
 + prev = start;
 + start += le32_to_cpu(la-la_recs[rec].la_clusters);
 + if (bit  start)
 + break;
 + }
 + offset = le32_to_cpu(la-la_recs[rec].la_start) + (bit - prev);
 +
 + return offset;
 +}

This can't work for non-DISCONTIG_LA filesystems.  I looked, and
you call this regardless of the feature bits.  Old filesystems will
crash, because they have bitmap bits instead of la_rec_count.  This is
why I said you couldn't remove la_bitmap.

 +/*
 + * This function is called before allocating a new chunk for the localalloc
 + * bitmap to make sure there is enough space in the bitmap for the new record
 + */
 +static u32 ocfs2_local_alloc_adjust_bits_wanted(struct ocfs2_local_alloc *la,
 + struct ocfs2_alloc_context *ac)
 +{
 + u32 required, available, cluster_cnt;
 +
 + if (ac-ac_bits_given == ac-ac_bits_wanted)
 + return 0;
 +
 + /* total bits available in bitmap */
 + available   = le16_to_cpu(la-la_size)  3;
 + cluster_cnt = ocfs2_local_alloc_cluster_count(la);
 +
 + /*
 +  * Wanted shouldn't be greater than bitmap size and given should be
 +  * equal to cluster count
 +  */
 + BUG_ON(ac-ac_bits_given  ac-ac_bits_wanted);
 + BUG_ON(ac-ac_bits_wanted  available);
 + BUG_ON(ac-ac_bits_given != cluster_cnt);
 +
 + /* reduce bits taken by each record structure */
 + available -= (le16_to_cpu(la-la_rec_count) *
 +   OCFS2_LOCAL_ALLOC_BITS_PER_REC);

Again, no check for DISCONTIG_LA.  I'm going to stop mentioning
this.  Just assume that every place you want to touch la_rec_count, you
need to make sure you have a DISCONTIG_LA filesystem.

 @@ -348,21 +421,21 @@ int ocfs2_load_local_alloc(struct ocfs2_super *osb)
   }
  
   /* do a little verification. */
 - num_used = ocfs2_local_alloc_count_bits(alloc);
 + num_used = ocfs2_local_alloc_count_bits(osb, alloc);
  
   /* hopefully the local alloc has always been recovered before
* we load it. */
   if (num_used
   || alloc-id1.bitmap1.i_used
   || alloc-id1.bitmap1.i_total
 - || la-la_bm_off)
 + || la-la_rec_count)

I lied.  You can't trust la_rec_count for non-DISCONTIG_LA
filesystems, so you can't have a naked check here.  Conversely,
la_bm_off is the valid check for those filesystems.  You need to
alternate based on the feature.

 @@ -690,8 +739,7 @@ int ocfs2_reserve_local_alloc_bits(struct ocfs2_super 
 

Re: [Ocfs2-devel] [PATCH 1/3] ocfs2: new structure to implment discontiguous local alloc bitmap

2012-05-07 Thread Joel Becker
On Mon, May 07, 2012 at 04:21:28PM -0700, Srinivas Eeda wrote:
 Current local alloc handles single contiguous free chunk of clusters. This
 patch enhances local alloc to handle discontigous free chunks. It adds a new
 ocfs2_local_alloc_rec structure which tracks single contiguous free chunk. An
 array of these sit in the bitmap itself and track discontiguous chunks. In
 best case there is only one record and increases as the filesystem gets
 fragmented. Number of records at a time are limited depending on the size
 of the bitmap and the max limit is defined by OCFS2_MAX_LOCAL_ALLOC_RECS.
 
 Signed-off-by: Srinivas Eeda srinivas.e...@oracle.com
 ---
  fs/ocfs2/localalloc.c |   10 ++
  fs/ocfs2/ocfs2.h  |8 
  fs/ocfs2/ocfs2_fs.h   |   48 ++--
  3 files changed, 60 insertions(+), 6 deletions(-)
 
 diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c
 index 210c352..4190e53 100644
 --- a/fs/ocfs2/localalloc.c
 +++ b/fs/ocfs2/localalloc.c
 @@ -48,6 +48,16 @@
  
  #define OCFS2_LOCAL_ALLOC(dinode)(((dinode)-id2.i_lab))
  
 +#define OCFS2_LOCAL_ALLOC_REC_SZ(la) (le16_to_cpu(la-la_rec_count) *\
 +  sizeof(struct ocfs2_local_alloc_rec))
 +#define OCFS2_LOCAL_ALLOC_BITMAP(la)((char *)((la-la_recs)) +\
 +  OCFS2_LOCAL_ALLOC_REC_SZ(la))

Another point.  Not only does this macro not handle
!DISCONTIG_LA filesystems (as described in my other email about this
patch), it should be a static inline function.  See eg: INODE_CACHE() in
fs/ocfs2/inode.h

Joel

-- 

Life's Little Instruction Book #456

Send your loved one flowers.  Think of a reason later.

http://www.jlbec.org/
jl...@evilplan.org

___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 3/3] ocfs2: modify reservation code to support discontigous localalloc

2012-05-07 Thread Joel Becker
On Mon, May 07, 2012 at 04:21:30PM -0700, Srinivas Eeda wrote:
 Currently reservation code assumes a bitmap given to it is all one contigous
 chunk. This patch enhances it to handle a discontigous chunks. It adds new
 fields m_bitmap_ext_cnt and m_bitmap_ext_arr. m_bitmap_ext_arr tracks the 
 sizes
 of each contigous free bits and m_bitmap_ext_cnt trackes number of
 m_bitmap_ext_arr.
 
 Signed-off-by: Srinivas Eeda srinivas.e...@oracle.com

Hi Srini,
A patch like this should come before the feature patch.  Once
this code can treat the old single-range bitmap as a one-element
multiple-range bitmap, you can add the multiple-range change easily.

 +void ocfs2_resmap_set_ext(struct ocfs2_reservation_map *resmap, int arr, u32 
 sz)
 +{
 + if (ocfs2_resmap_disabled(resmap))
 + return;
 +
 + resmap-m_bitmap_ext_arr[arr] = sz;
 +}

I don't see this function called anywhere.  And please don't use
needless abbreviations.  If you want to say ocfs2_resmap_set_extent(),
write it out.  I don't quite get the arguments, and since it isn't
called, I can't figure out how they are used.

Joel

-- 

To announce that there must be no criticism of them president, or
 that we are to stand by the president, right or wrong, is not only
 unpatriotic and servile, but is morally treasonable to the American
 public.
- Theodore Roosevelt

http://www.jlbec.org/
jl...@evilplan.org

___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] ocfs2 discontiguous localalloc patches

2012-05-07 Thread Srinivas Eeda
Joel Becker wrote:
 On Mon, May 07, 2012 at 04:21:27PM -0700, Srinivas Eeda wrote:
   
 can you please review following 3 patches that implement discontiguous
 localalloc bitmap support for ocfs2 file system. This feature helps
 applications that significantly fragment the filesystem.
 

   Hi Srini.  Have you some performance numbers backing this?  That
 is, I believe that the described filesystem turned off local alloc.  Do
 you have proof that these patches, turning it back on, improved the
 customer's performance?

 Joel
   
Hi Joel,

thanks a lot for the quick reply.

I have some stat_sysdir.sh snapshots at 
http://oss.oracle.com/~seeda/diag/stat_sysdir/ collected from a system. 
It has 4 snapshots collected when the file system usage is at 8%, 19%, 
21% and 52%.

In file stat_sysdir_52_percent_usage_slow_del.out, for the filesystem 
that has UUID: 3A6F54DF288C4AF2ABD1E00FC49BE7ED you could see that 
local_alloc: bitmap total is 38 and is 0(disabled) for 
local_alloc:0001, and local_alloc:0002. for the filesystem that has uuid 
AC444DB162AE427C899BA89E076DD479, all localalloc appears to be disabled. 
Sorry I didn't collect /sys/kernel/debug/fs/uuid/fs_state. But, given 
the file system state, even if localalloc is not disabled localalloc 
need to be refilled every 40 clusters.

Thanks,
--Srini








___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel


Re: [Ocfs2-devel] [PATCH 2/3] ocfs2: implement discontiguous localalloc bitmap

2012-05-07 Thread Srinivas Eeda
Joel Becker wrote:
 On Mon, May 07, 2012 at 04:21:29PM -0700, Srinivas Eeda wrote:
   

   OH MY DOG NO.  NEVER EVER DO THIS.  You cannot update an old
 filesystem on the fly!  What about other nodes that are running older
 versions of the software?  They will crash or corrupt data!  The entire
 point of feature bits is to make sure all nodes are speaking the same
 code.

 NAK NAK NAK

   This explains why you trusted la_rec_count earlier.  But that is
 broken.  When your patches are done, the code should use la_bm_off and
 la_bitmap when !DISCONTIG_LA and then use la_rec_count, etc when
 DISCONTIG_LA.  The only way to transition between them is a tunefs.ocfs2
 operation that walks the filesystem, flushes the bitmap, and then
 sets/clears la_rec_count appropriately depending on the direction..
   
Please please don't hate me :( ... the changes takes care of old formats 
as well ...  I used the reserved space in the structure so that the code 
changes will be minimal and still compatible with old file system 
formats. I agree that we need to have some reserved space still 
available. So as discussed I'll redo the changes accordingly. Please 
ignore all the patches.

Thanks,
--Srini

___
Ocfs2-devel mailing list
Ocfs2-devel@oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel