ocfs2: remove OCFS2_IOCB_SEM lock type in direct io
diff mbox

Message ID 55556992.6040002@huawei.com
State New
Headers show

Commit Message

WeiWei Wang May 15, 2015, 3:35 a.m. UTC
Hi all,

In ocfs2 direct read/write, OCFS2_IOCB_SEM lock type is used to protect
inode->i_alloc_sem rw semaphore lock in the earlier kernel version.
However, in the latest kernel, inode->i_alloc_sem rw semaphore lock
is not used at all, so OCFS2_IOCB_SEM lock type needs to be removed.

Any comments are appreciated, thanks!
					-- Weiwei Wang


Signed-off-by: Weiwei Wang <wangww631@huawei.com>
---
 fs/ocfs2/aops.c |  3 ---
 fs/ocfs2/aops.h |  7 -------
 fs/ocfs2/file.c | 31 ++++---------------------------
 3 files changed, 4 insertions(+), 37 deletions(-)

Comments

Shichangkuo May 16, 2015, 1:20 p.m. UTC | #1
Hi all,
    In my 10 nodes cluster, I find some strange logs in kernel.log when operating some days. logs as follws:

May 14 14:01:33 cvk68 kernel: [272684.537063] (umount,52420,10):dlm_prepare_lvb_for_migration:1216 ERROR: Mismatched lvb in lock cookie=8:982, name=M00000000000000007e012000000000, node=8
May 14 14:01:33 cvk68 kernel: [272684.537066] lockres: M00000000000000007e012000000000, owner=6, state=32
May 14 14:01:33 cvk68 kernel: [272684.537067]   last used: 0, refcnt: 11, on purge list: no
May 14 14:01:33 cvk68 kernel: [272684.537068]   on dirty list: no, on reco list: no, migrating pending: no
May 14 14:01:33 cvk68 kernel: [272684.537069]   inflight locks: 0, asts reserved: 0
May 14 14:01:33 cvk68 kernel: [272684.537069]   refmap nodes: [ 1 2 3 4 5 7 8 9 10 ], inflight=0
May 14 14:01:33 cvk68 kernel: [272684.537073]   granted queue:
May 14 14:01:33 cvk68 kernel: [272684.537074]     type=3, conv=-1, node=2, cookie=2:949, ref=2, ast=(empty=y,pend=n), bast=(empty=y,pend=n), pending=(conv=n,lock=n,cancel=n,unlock=n)
May 14 14:01:33 cvk68 kernel: [272684.537075] 050000000000005900000000000000001554ffffffc162ffffffec2a26ffffffd41554ffffffc162ffffffeaffffffbbfffffff0ffffffd41554ffffffc162ffffffeaffffffbbfffffff0ffffffd40000000005ffffffa00000ffffff81ffffff800001000000006d16ffffffc96400000000
May 14 14:01:33 cvk68 kernel: [272684.537092]     type=3, conv=-1, node=5, cookie=5:1103, ref=2, ast=(empty=y,pend=n), bast=(empty=y,pend=n), pending=(conv=n,lock=n,cancel=n,unlock=n)
May 14 14:01:33 cvk68 kernel: [272684.537094]     type=3, conv=-1, node=4, cookie=4:1117, ref=2, ast=(empty=y,pend=n), bast=(empty=y,pend=n), pending=(conv=n,lock=n,cancel=n,unlock=n)
May 14 14:01:33 cvk68 kernel: [272684.537095]     type=3, conv=-1, node=1, cookie=1:987, ref=2, ast=(empty=y,pend=n), bast=(empty=y,pend=n), pending=(conv=n,lock=n,cancel=n,unlock=n)
May 14 14:01:33 cvk68 kernel: [272684.537096]     type=3, conv=-1, node=7, cookie=7:973, ref=2, ast=(empty=y,pend=n), bast=(empty=y,pend=n), pending=(conv=n,lock=n,cancel=n,unlock=n)
May 14 14:01:33 cvk68 kernel: [272684.537097] 050000000000005900000000000000001554ffffffc162ffffffec2a26ffffffd41554ffffffc162ffffffeaffffffbbfffffff0ffffffd41554ffffffc162ffffffeaffffffbbfffffff0ffffffd40000000005ffffffa00000ffffff81ffffff800001000000006d16ffffffc96400000000
May 14 14:01:33 cvk68 kernel: [272684.537114]     type=3, conv=-1, node=8, cookie=8:982, ref=2, ast=(empty=y,pend=n), bast=(empty=y,pend=n), pending=(conv=n,lock=n,cancel=n,unlock=n)
May 14 14:01:33 cvk68 kernel: [272684.537116]     type=3, conv=-1, node=10, cookie=10:632, ref=2, ast=(empty=y,pend=n), bast=(empty=y,pend=n), pending=(conv=n,lock=n,cancel=n,unlock=n)
May 14 14:01:33 cvk68 kernel: [272684.537117] 050000000000005900000000000000001554ffffffc162ffffffec2a26ffffffd41554ffffffc162ffffffeaffffffbbfffffff0ffffffd41554ffffffc162ffffffeaffffffbbfffffff0ffffffd40000000005ffffffa00000ffffff81ffffff800001000000006d16ffffffc96400000000
May 14 14:01:33 cvk68 kernel: [272684.537135]     type=3, conv=-1, node=3, cookie=3:988, ref=2, ast=(empty=y,pend=n), bast=(empty=y,pend=n), pending=(conv=n,lock=n,cancel=n,unlock=n)
May 14 14:01:33 cvk68 kernel: [272684.537136]     type=3, conv=-1, node=9, cookie=9:399, ref=2, ast=(empty=y,pend=n), bast=(empty=y,pend=n), pending=(conv=n,lock=n,cancel=n,unlock=n)
May 14 14:01:33 cvk68 kernel: [272684.537137] 050000000000005900000000000000001554ffffffc162ffffffec2a26ffffffd41554ffffffc162ffffffeaffffffbbfffffff0ffffffd41554ffffffc162ffffffeaffffffbbfffffff0ffffffd40000000005ffffffa00000ffffff81ffffff800001000000006d16ffffffc96400000000
May 14 14:01:33 cvk68 kernel: [272684.537154]   converting queue:
May 14 14:01:33 cvk68 kernel: [272684.537154]   blocked queue:

This seems nodes [ 2 7 9 10 ] have lvb value, but nodes [ 1 3 4 5 8 ] don't have.
So, my question is:
1, What is the function of LVB ?
2, In which case this issue occurs?
3, If the value of LVB is meaningless, could we do not compare them between different nodes?

Thanks!

-------------------------------------------------------------------------------------------------------------------------------------
????????????????????????????????????????
????????????????????????????????????????
????????????????????????????????????????
???
This e-mail and its attachments contain confidential information from H3C, which is
intended only for the person or entity whose address is listed above. Any use of the
information contained herein in any way (including, but not limited to, total or partial
disclosure, reproduction, or dissemination) by persons other than the intended
recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender
by phone or email immediately and delete it!
Junxiao Bi May 20, 2015, 4:04 a.m. UTC | #2
On 05/15/2015 11:35 AM, WeiWei Wang wrote:
> Hi all,
> 
> In ocfs2 direct read/write, OCFS2_IOCB_SEM lock type is used to protect
> inode->i_alloc_sem rw semaphore lock in the earlier kernel version.
> However, in the latest kernel, inode->i_alloc_sem rw semaphore lock
> is not used at all, so OCFS2_IOCB_SEM lock type needs to be removed.
> 
> Any comments are appreciated, thanks!
> 					-- Weiwei Wang
> 
> 
> Signed-off-by: Weiwei Wang <wangww631@huawei.com>
Looks fine.

Reviewed-by: Junxiao Bi <junxiao.bi@oracle.com>

Thanks,
Junxiao.
> ---
>  fs/ocfs2/aops.c |  3 ---
>  fs/ocfs2/aops.h |  7 -------
>  fs/ocfs2/file.c | 31 ++++---------------------------
>  3 files changed, 4 insertions(+), 37 deletions(-)
> 
> diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
> index f906a25..d3eccc0 100644
> --- a/fs/ocfs2/aops.c
> +++ b/fs/ocfs2/aops.c
> @@ -619,9 +619,6 @@ static void ocfs2_dio_end_io(struct kiocb *iocb,
>  	/* this io's submitter should not have unlocked this before we could */
>  	BUG_ON(!ocfs2_iocb_is_rw_locked(iocb));
> 
> -	if (ocfs2_iocb_is_sem_locked(iocb))
> -		ocfs2_iocb_clear_sem_locked(iocb);
> -
>  	if (ocfs2_iocb_is_unaligned_aio(iocb)) {
>  		ocfs2_iocb_clear_unaligned_aio(iocb);
> 
> diff --git a/fs/ocfs2/aops.h b/fs/ocfs2/aops.h
> index dd59599..24e496d 100644
> --- a/fs/ocfs2/aops.h
> +++ b/fs/ocfs2/aops.h
> @@ -79,7 +79,6 @@ static inline void ocfs2_iocb_set_rw_locked(struct kiocb *iocb, int level)
>  enum ocfs2_iocb_lock_bits {
>  	OCFS2_IOCB_RW_LOCK = 0,
>  	OCFS2_IOCB_RW_LOCK_LEVEL,
> -	OCFS2_IOCB_SEM,
>  	OCFS2_IOCB_UNALIGNED_IO,
>  	OCFS2_IOCB_NUM_LOCKS
>  };
> @@ -88,12 +87,6 @@ enum ocfs2_iocb_lock_bits {
>  	clear_bit(OCFS2_IOCB_RW_LOCK, (unsigned long *)&iocb->private)
>  #define ocfs2_iocb_rw_locked_level(iocb) \
>  	test_bit(OCFS2_IOCB_RW_LOCK_LEVEL, (unsigned long *)&iocb->private)
> -#define ocfs2_iocb_set_sem_locked(iocb) \
> -	set_bit(OCFS2_IOCB_SEM, (unsigned long *)&iocb->private)
> -#define ocfs2_iocb_clear_sem_locked(iocb) \
> -	clear_bit(OCFS2_IOCB_SEM, (unsigned long *)&iocb->private)
> -#define ocfs2_iocb_is_sem_locked(iocb) \
> -	test_bit(OCFS2_IOCB_SEM, (unsigned long *)&iocb->private)
> 
>  #define ocfs2_iocb_set_unaligned_aio(iocb) \
>  	set_bit(OCFS2_IOCB_UNALIGNED_IO, (unsigned long *)&iocb->private)
> diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
> index d8b670c..fbfadb2 100644
> --- a/fs/ocfs2/file.c
> +++ b/fs/ocfs2/file.c
> @@ -2250,7 +2250,7 @@ out:
>  static ssize_t ocfs2_file_write_iter(struct kiocb *iocb,
>  				    struct iov_iter *from)
>  {
> -	int direct_io, appending, rw_level, have_alloc_sem  = 0;
> +	int direct_io, appending, rw_level;
>  	int can_do_direct, has_refcount = 0;
>  	ssize_t written = 0;
>  	ssize_t ret;
> @@ -2279,16 +2279,7 @@ static ssize_t ocfs2_file_write_iter(struct kiocb *iocb,
> 
>  	mutex_lock(&inode->i_mutex);
> 
> -	ocfs2_iocb_clear_sem_locked(iocb);
> -
>  relock:
> -	/* to match setattr's i_mutex -> rw_lock ordering */
> -	if (direct_io) {
> -		have_alloc_sem = 1;
> -		/* communicate with ocfs2_dio_end_io */
> -		ocfs2_iocb_set_sem_locked(iocb);
> -	}
> -
>  	/*
>  	 * Concurrent O_DIRECT writes are allowed with
>  	 * mount_option "coherency=buffered".
> @@ -2298,7 +2289,7 @@ relock:
>  	ret = ocfs2_rw_lock(inode, rw_level);
>  	if (ret < 0) {
>  		mlog_errno(ret);
> -		goto out_sems;
> +		goto out_mutex;
>  	}
> 
>  	/*
> @@ -2347,7 +2338,6 @@ relock:
>  	if (direct_io && !can_do_direct) {
>  		ocfs2_rw_unlock(inode, rw_level);
> 
> -		have_alloc_sem = 0;
>  		rw_level = -1;
> 
>  		direct_io = 0;
> @@ -2416,7 +2406,6 @@ no_sync:
>  	 */
>  	if ((ret == -EIOCBQUEUED) || (!ocfs2_iocb_is_rw_locked(iocb))) {
>  		rw_level = -1;
> -		have_alloc_sem = 0;
>  		unaligned_dio = 0;
>  	}
> 
> @@ -2429,10 +2418,7 @@ out:
>  	if (rw_level != -1)
>  		ocfs2_rw_unlock(inode, rw_level);
> 
> -out_sems:
> -	if (have_alloc_sem)
> -		ocfs2_iocb_clear_sem_locked(iocb);
> -
> +out_mutex:
>  	mutex_unlock(&inode->i_mutex);
> 
>  	if (written)
> @@ -2473,7 +2459,7 @@ bail:
>  static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
>  				   struct iov_iter *to)
>  {
> -	int ret = 0, rw_level = -1, have_alloc_sem = 0, lock_level = 0;
> +	int ret = 0, rw_level = -1, lock_level = 0;
>  	struct file *filp = iocb->ki_filp;
>  	struct inode *inode = file_inode(filp);
> 
> @@ -2490,16 +2476,11 @@ static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
>  		goto bail;
>  	}
> 
> -	ocfs2_iocb_clear_sem_locked(iocb);
> -
>  	/*
>  	 * buffered reads protect themselves in ->readpage().  O_DIRECT reads
>  	 * need locks to protect pending reads from racing with truncate.
>  	 */
>  	if (iocb->ki_flags & IOCB_DIRECT) {
> -		have_alloc_sem = 1;
> -		ocfs2_iocb_set_sem_locked(iocb);
> -
>  		ret = ocfs2_rw_lock(inode, 0);
>  		if (ret < 0) {
>  			mlog_errno(ret);
> @@ -2535,13 +2516,9 @@ static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
>  	/* see ocfs2_file_write_iter */
>  	if (ret == -EIOCBQUEUED || !ocfs2_iocb_is_rw_locked(iocb)) {
>  		rw_level = -1;
> -		have_alloc_sem = 0;
>  	}
> 
>  bail:
> -	if (have_alloc_sem)
> -		ocfs2_iocb_clear_sem_locked(iocb);
> -
>  	if (rw_level != -1)
>  		ocfs2_rw_unlock(inode, rw_level);
>

Patch
diff mbox

diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c
index f906a25..d3eccc0 100644
--- a/fs/ocfs2/aops.c
+++ b/fs/ocfs2/aops.c
@@ -619,9 +619,6 @@  static void ocfs2_dio_end_io(struct kiocb *iocb,
 	/* this io's submitter should not have unlocked this before we could */
 	BUG_ON(!ocfs2_iocb_is_rw_locked(iocb));

-	if (ocfs2_iocb_is_sem_locked(iocb))
-		ocfs2_iocb_clear_sem_locked(iocb);
-
 	if (ocfs2_iocb_is_unaligned_aio(iocb)) {
 		ocfs2_iocb_clear_unaligned_aio(iocb);

diff --git a/fs/ocfs2/aops.h b/fs/ocfs2/aops.h
index dd59599..24e496d 100644
--- a/fs/ocfs2/aops.h
+++ b/fs/ocfs2/aops.h
@@ -79,7 +79,6 @@  static inline void ocfs2_iocb_set_rw_locked(struct kiocb *iocb, int level)
 enum ocfs2_iocb_lock_bits {
 	OCFS2_IOCB_RW_LOCK = 0,
 	OCFS2_IOCB_RW_LOCK_LEVEL,
-	OCFS2_IOCB_SEM,
 	OCFS2_IOCB_UNALIGNED_IO,
 	OCFS2_IOCB_NUM_LOCKS
 };
@@ -88,12 +87,6 @@  enum ocfs2_iocb_lock_bits {
 	clear_bit(OCFS2_IOCB_RW_LOCK, (unsigned long *)&iocb->private)
 #define ocfs2_iocb_rw_locked_level(iocb) \
 	test_bit(OCFS2_IOCB_RW_LOCK_LEVEL, (unsigned long *)&iocb->private)
-#define ocfs2_iocb_set_sem_locked(iocb) \
-	set_bit(OCFS2_IOCB_SEM, (unsigned long *)&iocb->private)
-#define ocfs2_iocb_clear_sem_locked(iocb) \
-	clear_bit(OCFS2_IOCB_SEM, (unsigned long *)&iocb->private)
-#define ocfs2_iocb_is_sem_locked(iocb) \
-	test_bit(OCFS2_IOCB_SEM, (unsigned long *)&iocb->private)

 #define ocfs2_iocb_set_unaligned_aio(iocb) \
 	set_bit(OCFS2_IOCB_UNALIGNED_IO, (unsigned long *)&iocb->private)
diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c
index d8b670c..fbfadb2 100644
--- a/fs/ocfs2/file.c
+++ b/fs/ocfs2/file.c
@@ -2250,7 +2250,7 @@  out:
 static ssize_t ocfs2_file_write_iter(struct kiocb *iocb,
 				    struct iov_iter *from)
 {
-	int direct_io, appending, rw_level, have_alloc_sem  = 0;
+	int direct_io, appending, rw_level;
 	int can_do_direct, has_refcount = 0;
 	ssize_t written = 0;
 	ssize_t ret;
@@ -2279,16 +2279,7 @@  static ssize_t ocfs2_file_write_iter(struct kiocb *iocb,

 	mutex_lock(&inode->i_mutex);

-	ocfs2_iocb_clear_sem_locked(iocb);
-
 relock:
-	/* to match setattr's i_mutex -> rw_lock ordering */
-	if (direct_io) {
-		have_alloc_sem = 1;
-		/* communicate with ocfs2_dio_end_io */
-		ocfs2_iocb_set_sem_locked(iocb);
-	}
-
 	/*
 	 * Concurrent O_DIRECT writes are allowed with
 	 * mount_option "coherency=buffered".
@@ -2298,7 +2289,7 @@  relock:
 	ret = ocfs2_rw_lock(inode, rw_level);
 	if (ret < 0) {
 		mlog_errno(ret);
-		goto out_sems;
+		goto out_mutex;
 	}

 	/*
@@ -2347,7 +2338,6 @@  relock:
 	if (direct_io && !can_do_direct) {
 		ocfs2_rw_unlock(inode, rw_level);

-		have_alloc_sem = 0;
 		rw_level = -1;

 		direct_io = 0;
@@ -2416,7 +2406,6 @@  no_sync:
 	 */
 	if ((ret == -EIOCBQUEUED) || (!ocfs2_iocb_is_rw_locked(iocb))) {
 		rw_level = -1;
-		have_alloc_sem = 0;
 		unaligned_dio = 0;
 	}

@@ -2429,10 +2418,7 @@  out:
 	if (rw_level != -1)
 		ocfs2_rw_unlock(inode, rw_level);

-out_sems:
-	if (have_alloc_sem)
-		ocfs2_iocb_clear_sem_locked(iocb);
-
+out_mutex:
 	mutex_unlock(&inode->i_mutex);

 	if (written)
@@ -2473,7 +2459,7 @@  bail:
 static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
 				   struct iov_iter *to)
 {
-	int ret = 0, rw_level = -1, have_alloc_sem = 0, lock_level = 0;
+	int ret = 0, rw_level = -1, lock_level = 0;
 	struct file *filp = iocb->ki_filp;
 	struct inode *inode = file_inode(filp);

@@ -2490,16 +2476,11 @@  static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
 		goto bail;
 	}

-	ocfs2_iocb_clear_sem_locked(iocb);
-
 	/*
 	 * buffered reads protect themselves in ->readpage().  O_DIRECT reads
 	 * need locks to protect pending reads from racing with truncate.
 	 */
 	if (iocb->ki_flags & IOCB_DIRECT) {
-		have_alloc_sem = 1;
-		ocfs2_iocb_set_sem_locked(iocb);
-
 		ret = ocfs2_rw_lock(inode, 0);
 		if (ret < 0) {
 			mlog_errno(ret);
@@ -2535,13 +2516,9 @@  static ssize_t ocfs2_file_read_iter(struct kiocb *iocb,
 	/* see ocfs2_file_write_iter */
 	if (ret == -EIOCBQUEUED || !ocfs2_iocb_is_rw_locked(iocb)) {
 		rw_level = -1;
-		have_alloc_sem = 0;
 	}

 bail:
-	if (have_alloc_sem)
-		ocfs2_iocb_clear_sem_locked(iocb);
-
 	if (rw_level != -1)
 		ocfs2_rw_unlock(inode, rw_level);