gmane.comp.file-systems.ocfs2.devel

Re: [PATCH] Fix waiting status race condition in dlmrecovery

srinivas eeda — 2012-05-25T22:17:57

comments inline

On 5/24/2012 10:53 PM, xiaowei.hu< at >oracle.com wrote:
fix seems to address the issue, but can you please add a function 
dlm_is_node_in_livemap similar to dlm_is_node_dead so that it' improves 
readability. You can then add the following to check if the node is 
still alive
+        if (!dlm_is_node_in_livemap(dlm, ndata->node_num))
+            ndate->state = DLM_RECO_NODE_DATA_DEAD;
+        else
+            all_nodes_done = 0;

[PATCH] Fix waiting status race condition in dlmrecovery

2012-05-25T05:53:22

From: "Xiaowei.Hu" <xiaowei.hu< at >oracle.com>

when the master requested locks ,but one/some of the live nodes died,
after it received the request msg and before send out the locks packages,
the recovery will fall into endless loop,waiting for the status changed to finalize

NodeA                                     NodeB
selected as recovery master
dlm_remaster_locks
  -> dlm_requeset_all_locks
  this send request locks msg to B
                                          received the msg from A,
                                          queue worker dlm_request_all_locks_worker
                                          return 0
go on set state to requested
wait for the state become done
                                          NodeB lost connection due to network
                                          before the worker begin, or it die.
NodeA still waiting for the
change of reco state.
It won't end if it not get data done msg
And at this time nodeB do not realize this (or it just died),
it won't send the ms

Re: [PATCH 2/3] ocfs2: implement discontiguouslocalalloc bitmap

Srinivas Eeda — 2012-05-08T02:10:32

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

Re: ocfs2 discontiguous localalloc patches

Srinivas Eeda — 2012-05-08T01:26:58

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:0000 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

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

Joel Becker — 2012-05-08T00:34:31

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.


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

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

Joel Becker — 2012-05-08T00:28:23

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

Re: [PATCH 2/3] ocfs2: implement discontiguouslocalalloc bitmap

Joel Becker — 2012-05-08T00:22:58

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.
 

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.


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.


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.


This is what I mean about osb->local_alloc_inode.  There should
be a

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

Joel Becker — 2012-05-08T00:05:33

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.


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


You can't do this without checking for DISCONTIG_LA.  Again,
filesystems without DISCONTIG_LA will be starting at la_bitmap.

Joel

Re: ocfs2 discontiguous localalloc patches

Joel Becker — 2012-05-08T00:01:01

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

ocfs2 discontiguous localalloc patches

Srinivas Eeda — 2012-05-07T23:21:27

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

[PATCH 2/3] ocfs2: implement discontiguous localallocbitmap

Srinivas Eeda — 2012-05-07T23:21:29

This patch adds supporting functions and modifies localalloc code to implement
discontiguous localalloc bitmap.

Signed-off-by: Srinivas Eeda <srinivas.eeda< at >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
< at >< at > -48,6 +48,9 < at >< at >
 
 #define OCFS2_LOCAL_ALLOC(dinode)(&((dinode)->id2.i_lab))
 
+/* defines minimum contiguous required */
+#define OCFS2_LOCAL_ALLOC_MIN_BITS2
+
 #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)) +\
< at >< at > -58,7 +61,8 < at >< at >
 #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 oc

[PATCH 1/3] ocfs2: new structure to implmentdiscontiguous local alloc bitmap

Srinivas Eeda — 2012-05-07T23:21:28

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.eeda< at >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
< at >< at > -48,6 +48,16 < at >< at >
 
 #define OCFS2_LOCAL_ALLOC(dinode)(&((dinode)->id2.i_lab))

[PATCH 3/3] ocfs2: modify reservation code to supportdiscontigous localalloc

Srinivas Eeda — 2012-05-07T23:21:30

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.eeda< at >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
< at >< at > -291,7 +291,15 < at >< at > 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

Ocfs2 + Quota!

Marek Królikowski — 2012-04-17T21:13:44

Hello Guys
This is awsome you want guys do more stable OCFS2 but mayby someone repair 
problem with ocfs2 + quota deadlock?
Because is amazing unstable i report bug 4 month ago on ocfs2-users/ 
ocfs2-devel mailing list and Oracle Bugzilla and everyone confirm problem 
but noone repair.. :(
http://oss.oracle.com/bugzilla/show_bug.cgi?id=1339
and:
http://oss.oracle.com/pipermail/ocfs2-users/2012-January/005637.html


Thanks.

Re: Ocfs2/move_extents: move/defrag extents within acertain range.

Dan Carpenter — 2012-02-15T20:40:56

Gar...  I somehow missed that this warning was really old.  Sorry
for that.  I will fix my scripts to avoid this.

regards,
dan carpenter

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel< at >oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel

Re: [patch] ocfs2: cleanup error handling ino2hb_alloc_hb_set()

Dan Carpenter — 2012-02-13T20:29:24

It's a static checker thing.  It's either an unneeded NULL check or
a check on the wrong variable depending on how you look at it.
Recently, I've been sending a lot of patches to remove unneeded NULL
checks for static checkers.  I try to fix the mistakes that are
harmless so that the real bugs aren't drowned out in noise.

regards,
dan carpenter

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel< at >oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel

Re: Ocfs2/move_extents: move/defrag extents within acertain range.

Dan Carpenter — 2012-02-15T20:31:51

Hello Tristan Ye,

This is a semi-automatic email about new static checker warnings.

The patch 53069d4e7695: "Ocfs2/move_extents: move/defrag extents 
within a certain range." from May 25, 2011, leads to the following 
Smatch complaint:

fs/ocfs2/move_extents.c:980 ocfs2_move_extents()
 warn: variable dereferenced before check 'inode' (see line 978)

fs/ocfs2/move_extents.c
   977struct buffer_head *di_bh = NULL;
   978struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
                                                   ^^^^^^^^^^^
Dereference.

   979
   980if (!inode)
                    ^^^^^^
Check.

   981return -ENOENT;
   982

regards,
dan carpenter

[patch] ocfs2: cleanup error handling ino2hb_alloc_hb_set()

Dan Carpenter — 2012-02-13T13:50:47

If "ret" is NULL, then "hs" is also NULL, so there is no need to free
it.  config_group_init_type_name() can't fail if the name ("heartbeat"
in this case) is less than CONFIGFS_ITEM_NAME_LEN (20) characters long
so we can just remove this error handling code.

Signed-off-by: Dan Carpenter <dan.carpenter< at >oracle.com>

diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c
index f3f2d95..0bca51b 100644
--- a/fs/ocfs2/cluster/heartbeat.c
+++ b/fs/ocfs2/cluster/heartbeat.c
< at >< at > -2306,20 +2306,14 < at >< at > static struct config_item_type o2hb_heartbeat_group_type = {
 struct config_group *o2hb_alloc_hb_set(void)
 {
 struct o2hb_heartbeat_group *hs = NULL;
-struct config_group *ret = NULL;
 
 hs = kzalloc(sizeof(struct o2hb_heartbeat_group), GFP_KERNEL);
 if (hs == NULL)
-goto out;
+return NULL;
 
 config_group_init_type_name(&hs->hs_group, "heartbeat",
     &o2hb_heartbeat_group_type);
-
-ret = &hs->hs_group;
-out:
-if (ret == NULL)
-kfree(hs);
-return ret;
+return &hs->hs_group;
 }
 
 v

Re: [patch] ocfs2: cleanup error handling ino2hb_alloc_hb_set()

Dan Carpenter — 2012-02-13T20:08:35

The error handling isn't correct because checking "&hs->hs_group"
for NULL doesn't work.  "hs" and "&hs->hs_group" are the same
address and we checked "hs" already.  If we wanted to check for
allocation errors, then we would need to change the check to:

        if (!hs->hs_group.cg_item.ci_name)
kfree(hs);

But that's not how the function is supposed to be used.  The example
code in Documentation/filesystems/configfs/configfs_example_explicit.c
doesn't have error handling.

regards,
dan carpenter

_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel< at >oss.oracle.com
http://oss.oracle.com/mailman/listinfo/ocfs2-devel

[PATCH 09/27] fs: Push mnt_want_write() outside ofi_mutex

Jan Kara — 2012-04-16T16:13:47

Currently, mnt_want_write() is sometimes called with i_mutex held and sometimes
without it. This isn't really a problem because mnt_want_write() is a
non-blocking operation (essentially has a trylock semantics) but when the
function starts to handle also frozen filesystems, it will get a full lock
semantics and thus proper lock ordering has to be established. So move
all mnt_want_write() calls outside of i_mutex.

One non-trivial case needing conversion is kern_path_create() /
user_path_create() which didn't include mnt_want_write() but now needs to
because it acquires i_mutex.  Because there are virtual file systems which
don't bother with freeze / remount-ro protection we actually provide both
versions of the function - one which calls mnt_want_write() and one which does
not.

CC: ocfs2-devel< at >oss.oracle.com
CC: Mark Fasheh <mfasheh< at >suse.com>
CC: Joel Becker <jlbec< at >evilplan.org>
CC: "David S. Miller" <davem< at >davemloft.net>
BugLink: https://bugs.launchpad.net/bugs/897421
Tested-by: Kamal Mostafa <kamal< at >canoni

[PATCH 19/27] ocfs2: Convert to new freezing mechanism

Jan Kara — 2012-04-16T16:13:57

Protect ocfs2_page_mkwrite() and ocfs2_file_aio_write() using the new
freeze protection. We also protect several ioctl entry points which
were missing the protection.

CC: Mark Fasheh <mfasheh< at >suse.com>
CC: Joel Becker <jlbec< at >evilplan.org>
CC: ocfs2-devel< at >oss.oracle.com
Signed-off-by: Jan Kara <jack< at >suse.cz>
---
 fs/ocfs2/file.c  |   11 +++++++++--
 fs/ocfs2/ioctl.c |   14 ++++++++++++--
 fs/ocfs2/mmap.c  |    2 ++
 3 files changed, 23 insertions(+), 4 deletions(-)

diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index 061591a..9b1e3d4 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
< at >< at > -1971,6 +1971,7 < at >< at > int ocfs2_change_file_space(struct file *file, unsigned int cmd,
 {
 struct inode *inode = file->f_path.dentry->d_inode;
 struct ocfs2_super *osb = OCFS2_SB(inode->i_sb);
+int ret;
 
 if ((cmd == OCFS2_IOC_RESVSP || cmd == OCFS2_IOC_RESVSP64) &&
     !ocfs2_writes_unwritten_extents(osb))
< at >< at > -1985,7 +1986,12 < at >< at > int ocfs2_change_file_space(struct file *file, unsigned int cmd,
 if (!(file->f_mode & FMOD