[LU-4069] bad GOTOs

• Type: Bug
• Resolution: Fixed
• Priority: Minor
• Fix Version/s: Lustre 2.6.0
• Affects Version/s: Lustre 2.5.0
• Labels: coccinelle
• Severity: 3
• Assignee: Dmitry Eremin (Inactive)
• Reporter: John Hammond

Description

Using the coccinelle script below we can easily find statements of the form GOTO(label, -ERRNO):

@@
identifier I;
expression E;
@@

* GOTO(I, -E)

Here are the results:

diff = 
diff -u -p lustre/mdt/mdt_xattr.c /tmp/nothing/mdt/mdt_xattr.c
--- lustre/mdt/mdt_xattr.c
+++ /tmp/nothing/mdt/mdt_xattr.c
@@ -419,7 +419,6 @@ int mdt_reint_setxattr(struct mdt_thread
                             sizeof(XATTR_NAME_ACL_DEFAULT) - 1) == 0)) {
                 /* currently lustre limit acl access size */
                 if (xattr_len > LUSTRE_POSIX_ACL_MAX_SIZE)
-                        GOTO(out, -ERANGE);
         }
 
         lockpart = MDS_INODELOCK_UPDATE;

diff -u -p lustre/quota/qsd_request.c /tmp/nothing/quota/qsd_request.c
--- lustre/quota/qsd_request.c
+++ /tmp/nothing/quota/qsd_request.c
@@ -287,7 +287,6 @@ int qsd_intent_lock(const struct lu_env
 		lock = ldlm_handle2lock(&qti->qti_lockh);
 		if (lock == NULL) {
 			ptlrpc_req_finished(req);
-			GOTO(out, -ENOLCK);
 		}
 		lu_ref_add(&qqi->qqi_reference, "glb_lock", lock);
 		LDLM_LOCK_PUT(lock);
diff -u -p lustre/ldlm/ldlm_inodebits.c /tmp/nothing/ldlm/ldlm_inodebits.c
--- lustre/ldlm/ldlm_inodebits.c
+++ /tmp/nothing/ldlm/ldlm_inodebits.c
@@ -228,7 +228,6 @@ int ldlm_process_inodebits_lock(struct l
                                        LDLM_WORK_BL_AST);
                 lock_res(res);
                 if (rc == -ERESTART)
-                        GOTO(restart, -ERESTART);
                 *flags |= LDLM_FL_BLOCK_GRANTED;
         } else {
                 ldlm_resource_unlink_lock(lock);

diff -u -p lustre/ldlm/ldlm_flock.c /tmp/nothing/ldlm/ldlm_flock.c
--- lustre/ldlm/ldlm_flock.c
+++ /tmp/nothing/ldlm/ldlm_flock.c
@@ -602,7 +602,6 @@ restart:
                                                        LDLM_WORK_CP_AST);
                                 lock_res_and_lock(req);
                                 if (rc == -ERESTART)
-                                        GOTO(restart, -ERESTART);
                        }
                 } else {
                         LASSERT(req->l_completion_ast);

@@ -184,7 +184,6 @@ int ldlm_process_plain_lock(struct ldlm_
                                        LDLM_WORK_BL_AST);
                 lock_res(res);
                 if (rc == -ERESTART)
-                        GOTO(restart, -ERESTART);
                 *flags |= LDLM_FL_BLOCK_GRANTED;
         } else {
                 ldlm_resource_unlink_lock(lock);

@@ -783,7 +783,6 @@ int ldlm_process_extent_lock(struct ldlm
 				GOTO(out, rc = 0);
 			}
 
-			GOTO(restart, -ERESTART);
 		}
 
 		/* this way we force client to wait for the lock


diff -u -p lustre/osd-zfs/osd_io.c /tmp/nothing/osd-zfs/osd_io.c
--- lustre/osd-zfs/osd_io.c
+++ /tmp/nothing/osd-zfs/osd_io.c
@@ -351,7 +351,6 @@ static int osd_bufs_get_write(const stru
 
 			abuf = dmu_request_arcbuf(obj->oo_db, bs);
 			if (unlikely(abuf == NULL))
-				GOTO(out_err, -ENOMEM);
 
 			cfs_atomic_inc(&osd->od_zerocopy_loan);
 
@@ -402,7 +401,6 @@ static int osd_bufs_get_write(const stru
 
 				lnb[i].page = alloc_page(OSD_GFP_IO);
 				if (unlikely(lnb[i].page == NULL))
-					GOTO(out_err, -ENOMEM);
 
 				LASSERT(lnb[i].page->mapping == NULL);
 				lnb[i].page->mapping = (void *)obj;


diff -u -p lustre/obdclass/genops.c /tmp/nothing/obdclass/genops.c
--- lustre/obdclass/genops.c
+++ /tmp/nothing/obdclass/genops.c
@@ -671,26 +671,22 @@ int obd_init_caches(void)
 					      sizeof(struct obd_device),
 					      0, 0, NULL);
 	if (!obd_device_cachep)
-		GOTO(out, -ENOMEM);
 
 	LASSERT(obdo_cachep == NULL);
 	obdo_cachep = kmem_cache_create("ll_obdo_cache", sizeof(struct obdo),
 					0, 0, NULL);
 	if (!obdo_cachep)
-		GOTO(out, -ENOMEM);
 
 	LASSERT(import_cachep == NULL);
 	import_cachep = kmem_cache_create("ll_import_cache",
 					  sizeof(struct obd_import),
 					  0, 0, NULL);
 	if (!import_cachep)
-		GOTO(out, -ENOMEM);
 
 	LASSERT(capa_cachep == NULL);
 	capa_cachep = kmem_cache_create("capa_cache", sizeof(struct obd_capa),
 					0, 0, NULL);
 	if (!capa_cachep)
-		GOTO(out, -ENOMEM);
 
 	RETURN(0);
 out:

Note that despite its format, the coccinelle output is not to be taken as a literal patch. Also the LDLM GOTOs are not bugs per-se.