From 5cd7c3f2ab53a07cf98aef83685dafddb13077ed Mon Sep 17 00:00:00 2001 From: "John L. Hammond" Date: Mon, 24 Jun 2013 15:39:51 -0500 Subject: [PATCH] LU-3498 kernel: remove uses of IS_ERR_VALUE() Remove most uses of IS_ERR_VALUE(). This macro was often given an int argument coming from PTR_ERR(). This invokes implementation defined behavior since the long value gotten by applying PTR_ERR() to a kernel pointer will usually not be representable as an int. Moreover it may be just plain wrong to do this since the expressions IS_ERR(p) and IS_ERR_VALUE((int) PTR_ERR(p)) are not equivalent for a general pointer p. Signed-off-by: John L. Hammond Change-Id: Ib76592a1fc491539d28a51668c7f06347e988f9f Reviewed-on: http://review.whamcloud.com/6759 Reviewed-by: Andreas Dilger Tested-by: Jenkins Reviewed-by: Dmitry Eremin Tested-by: Maloo Reviewed-by: Oleg Drokin --- lnet/lnet/acceptor.c | 9 +++++---- lnet/lnet/router.c | 22 ++++++++++++---------- lnet/ulnds/socklnd/usocklnd.c | 11 ++++++----- lustre/ldlm/ldlm_lockd.c | 15 ++++++++++----- lustre/llite/statahead.c | 10 ++++++---- lustre/mdc/mdc_request.c | 25 +++++++++++++++---------- lustre/mgc/mgc_request.c | 13 +++++++------ lustre/obdclass/llog.c | 22 ++++++++++++++-------- lustre/osd-ldiskfs/osd_scrub.c | 8 +++++--- lustre/osp/osp_sync.c | 10 ++++++---- lustre/ptlrpc/pinger.c | 11 +++++++---- lustre/ptlrpc/service.c | 42 +++++++++++++++++++++++++----------------- lustre/quota/qsd_reint.c | 6 ++++-- 13 files changed, 122 insertions(+), 82 deletions(-) diff --git a/lnet/lnet/acceptor.c b/lnet/lnet/acceptor.c index 9bdac05..654918b 100644 --- a/lnet/lnet/acceptor.c +++ b/lnet/lnet/acceptor.c @@ -503,6 +503,7 @@ accept2secure(const char *acc, long *sec) int lnet_acceptor_start(void) { + struct task_struct *task; int rc; long rc2; long secure; @@ -529,10 +530,10 @@ lnet_acceptor_start(void) if (lnet_count_acceptor_nis() == 0) /* not required */ return 0; - rc2 = PTR_ERR(kthread_run(lnet_acceptor, - (void *)(ulong_ptr_t)secure, - "acceptor_%03ld", secure)); - if (IS_ERR_VALUE(rc2)) { + task = kthread_run(lnet_acceptor, (void *)(ulong_ptr_t)secure, + "acceptor_%03ld", secure); + if (IS_ERR(task)) { + rc2 = PTR_ERR(task); CERROR("Can't start acceptor thread: %ld\n", rc2); fini_completion(&lnet_acceptor_state.pta_signal); diff --git a/lnet/lnet/router.c b/lnet/lnet/router.c index a560db8..effd48a 100644 --- a/lnet/lnet/router.c +++ b/lnet/lnet/router.c @@ -998,13 +998,15 @@ lnet_ping_router_locked (lnet_peer_t *rtr) int lnet_router_checker_start(void) { - int rc; - int eqsz; -#ifndef __KERNEL__ - lnet_peer_t *rtr; - __u64 version; - int nrtr = 0; - int router_checker_max_eqsize = 10240; + int rc; + int eqsz; +#ifdef __KERNEL__ + struct task_struct *task; +#else /* __KERNEL__ */ + lnet_peer_t *rtr; + __u64 version; + int nrtr = 0; + int router_checker_max_eqsize = 10240; LASSERT (check_routers_before_use); LASSERT (dead_router_check_interval > 0); @@ -1090,9 +1092,9 @@ lnet_router_checker_start(void) the_lnet.ln_rc_state = LNET_RC_STATE_RUNNING; #ifdef __KERNEL__ - rc = PTR_ERR(kthread_run(lnet_router_checker, - NULL, "router_checker")); - if (IS_ERR_VALUE(rc)) { + task = kthread_run(lnet_router_checker, NULL, "router_checker"); + if (IS_ERR(task)) { + rc = PTR_ERR(task); CERROR("Can't start router checker thread: %d\n", rc); /* block until event callback signals exit */ down(&the_lnet.ln_rc_signal); diff --git a/lnet/ulnds/socklnd/usocklnd.c b/lnet/ulnds/socklnd/usocklnd.c index d4d5027..17f546c 100644 --- a/lnet/ulnds/socklnd/usocklnd.c +++ b/lnet/ulnds/socklnd/usocklnd.c @@ -297,12 +297,13 @@ usocklnd_base_startup() /* Spawn poll threads */ for (i = 0; i < usock_data.ud_npollthreads; i++) { - rc = PTR_ERR(kthread_run(usocklnd_poll_thread, - &usock_data.ud_pollthreads[i], - "")); - if (IS_ERR_VALUE(rc)) { + struct task_struct *task; + + task = kthread_run(usocklnd_poll_thread, + &usock_data.ud_pollthreads[i], ""); + if (IS_ERR(task)) { usocklnd_base_shutdown(i); - return rc; + return PTR_ERR(task); } } diff --git a/lustre/ldlm/ldlm_lockd.c b/lustre/ldlm/ldlm_lockd.c index 6580b8d..1e3bc7a 100644 --- a/lustre/ldlm/ldlm_lockd.c +++ b/lustre/ldlm/ldlm_lockd.c @@ -2771,11 +2771,15 @@ EXPORT_SYMBOL(ldlm_destroy_export); static int ldlm_setup(void) { static struct ptlrpc_service_conf conf; - struct ldlm_bl_pool *blp = NULL; - int rc = 0; + struct ldlm_bl_pool *blp = NULL; #ifdef __KERNEL__ - int i; +# ifdef HAVE_SERVER_SUPPORT + struct task_struct *task; +# endif + int i; #endif + int rc = 0; + ENTRY; if (ldlm_state != NULL) @@ -2910,8 +2914,9 @@ static int ldlm_setup(void) spin_lock_init(&waiting_locks_spinlock); cfs_timer_init(&waiting_locks_timer, waiting_locks_callback, 0); - rc = PTR_ERR(kthread_run(expired_lock_main, NULL, "ldlm_elt")); - if (IS_ERR_VALUE(rc)) { + task = kthread_run(expired_lock_main, NULL, "ldlm_elt"); + if (IS_ERR(task)) { + rc = PTR_ERR(task); CERROR("Cannot start ldlm expired-lock thread: %d\n", rc); GOTO(out, rc); } diff --git a/lustre/llite/statahead.c b/lustre/llite/statahead.c index 49cd27c..c21bae3 100644 --- a/lustre/llite/statahead.c +++ b/lustre/llite/statahead.c @@ -1498,6 +1498,7 @@ int do_statahead_enter(struct inode *dir, struct dentry **dentryp, struct ll_sa_entry *entry; struct ptlrpc_thread *thread; struct l_wait_info lwi = { 0 }; + struct task_struct *task; int rc = 0; struct ll_inode_info *plli; ENTRY; @@ -1655,11 +1656,12 @@ int do_statahead_enter(struct inode *dir, struct dentry **dentryp, lli->lli_sai = sai; plli = ll_i2info(parent->d_inode); - rc = PTR_ERR(kthread_run(ll_statahead_thread, parent, - "ll_sa_%u", plli->lli_opendir_pid)); + task = kthread_run(ll_statahead_thread, parent, "ll_sa_%u", + plli->lli_opendir_pid); thread = &sai->sai_thread; - if (IS_ERR_VALUE(rc)) { - CERROR("can't start ll_sa thread, rc: %d\n", rc); + if (IS_ERR(task)) { + rc = PTR_ERR(task); + CERROR("cannot start ll_sa thread: rc = %d\n", rc); dput(parent); lli->lli_opendir_key = NULL; thread_set_flags(thread, SVC_STOPPED); diff --git a/lustre/mdc/mdc_request.c b/lustre/mdc/mdc_request.c index 7833532..d5c9701 100644 --- a/lustre/mdc/mdc_request.c +++ b/lustre/mdc/mdc_request.c @@ -2281,8 +2281,9 @@ out: static int mdc_ioc_changelog_send(struct obd_device *obd, struct ioc_changelog *icc) { - struct changelog_show *cs; - int rc; + struct changelog_show *cs; + struct task_struct *task; + int rc; /* Freed in mdc_changelog_send_thread */ OBD_ALLOC_PTR(cs); @@ -2299,16 +2300,20 @@ static int mdc_ioc_changelog_send(struct obd_device *obd, * New thread because we should return to user app before * writing into our pipe */ - rc = PTR_ERR(kthread_run(mdc_changelog_send_thread, cs, - "mdc_clg_send_thread")); - if (!IS_ERR_VALUE(rc)) { - CDEBUG(D_CHANGELOG, "start changelog thread\n"); - return 0; + task = kthread_run(mdc_changelog_send_thread, cs, + "mdc_clg_send_thread"); + if (IS_ERR(task)) { + rc = PTR_ERR(task); + CERROR("%s: cannot start changelog thread: rc = %d\n", + obd->obd_name, rc); + OBD_FREE_PTR(cs); + } else { + rc = 0; + CDEBUG(D_CHANGELOG, "%s: started changelog thread\n", + obd->obd_name); } - CERROR("Failed to start changelog thread: %d\n", rc); - OBD_FREE_PTR(cs); - return rc; + return rc; } static int mdc_ioc_hsm_ct_start(struct obd_export *exp, diff --git a/lustre/mgc/mgc_request.c b/lustre/mgc/mgc_request.c index 7741bbf..09a6bce 100644 --- a/lustre/mgc/mgc_request.c +++ b/lustre/mgc/mgc_request.c @@ -891,7 +891,8 @@ static int mgc_cleanup(struct obd_device *obd) static int mgc_setup(struct obd_device *obd, struct lustre_cfg *lcfg) { - int rc; + struct task_struct *task; + int rc; ENTRY; rc = ptlrpcd_addref(); @@ -919,11 +920,11 @@ static int mgc_setup(struct obd_device *obd, struct lustre_cfg *lcfg) init_waitqueue_head(&rq_waitq); /* start requeue thread */ - rc = PTR_ERR(kthread_run(mgc_requeue_thread, NULL, - "ll_cfg_requeue")); - if (IS_ERR_VALUE(rc)) { - CERROR("%s: Cannot start requeue thread (%d)," - "no more log updates!\n", + task = kthread_run(mgc_requeue_thread, NULL, "ll_cfg_requeue"); + if (IS_ERR(task)) { + rc = PTR_ERR(task); + CERROR("%s: cannot start requeue thread: rc = %d; " + "no more log updates\n", obd->obd_name, rc); GOTO(err_cleanup, rc); } diff --git a/lustre/obdclass/llog.c b/lustre/obdclass/llog.c index e972fcd..3a3f430 100644 --- a/lustre/obdclass/llog.c +++ b/lustre/obdclass/llog.c @@ -463,17 +463,19 @@ int llog_process_or_fork(const struct lu_env *env, #ifdef __KERNEL__ if (fork) { + struct task_struct *task; + /* The new thread can't use parent env, * init the new one in llog_process_thread_daemonize. */ lpi->lpi_env = NULL; init_completion(&lpi->lpi_completion); - rc = PTR_ERR(kthread_run(llog_process_thread_daemonize, lpi, - "llog_process_thread")); - if (IS_ERR_VALUE(rc)) { + task = kthread_run(llog_process_thread_daemonize, lpi, + "llog_process_thread"); + if (IS_ERR(task)) { + rc = PTR_ERR(task); CERROR("%s: cannot start thread: rc = %d\n", loghandle->lgh_ctxt->loc_obd->obd_name, rc); - OBD_FREE_PTR(lpi); - RETURN(rc); + GOTO(out_lpi, rc); } wait_for_completion(&lpi->lpi_completion); } else { @@ -484,9 +486,13 @@ int llog_process_or_fork(const struct lu_env *env, lpi->lpi_env = env; llog_process_thread(lpi); #endif - rc = lpi->lpi_rc; - OBD_FREE_PTR(lpi); - RETURN(rc); + rc = lpi->lpi_rc; + +#ifdef __KERNEL__ +out_lpi: +#endif + OBD_FREE_PTR(lpi); + RETURN(rc); } EXPORT_SYMBOL(llog_process_or_fork); diff --git a/lustre/osd-ldiskfs/osd_scrub.c b/lustre/osd-ldiskfs/osd_scrub.c index 7d6c011..054239b 100644 --- a/lustre/osd-ldiskfs/osd_scrub.c +++ b/lustre/osd-ldiskfs/osd_scrub.c @@ -1994,6 +1994,7 @@ static int do_osd_scrub_start(struct osd_device *dev, __u32 flags) struct osd_scrub *scrub = &dev->od_scrub; struct ptlrpc_thread *thread = &scrub->os_thread; struct l_wait_info lwi = { 0 }; + struct task_struct *task; int rc; ENTRY; @@ -2017,9 +2018,10 @@ again: scrub->os_start_flags = flags; thread_set_flags(thread, 0); - rc = PTR_ERR(kthread_run(osd_scrub_main, dev, "OI_scrub")); - if (IS_ERR_VALUE(rc)) { - CERROR("%.16s: cannot start iteration thread, rc = %d\n", + task = kthread_run(osd_scrub_main, dev, "OI_scrub"); + if (IS_ERR(task)) { + rc = PTR_ERR(task); + CERROR("%.16s: cannot start iteration thread: rc = %d\n", LDISKFS_SB(osd_sb(dev))->s_es->s_volume_name, rc); RETURN(rc); } diff --git a/lustre/osp/osp_sync.c b/lustre/osp/osp_sync.c index 298626d..07e3a41 100644 --- a/lustre/osp/osp_sync.c +++ b/lustre/osp/osp_sync.c @@ -1106,6 +1106,7 @@ static void osp_sync_llog_fini(const struct lu_env *env, struct osp_device *d) int osp_sync_init(const struct lu_env *env, struct osp_device *d) { struct l_wait_info lwi = { 0 }; + struct task_struct *task; int rc; ENTRY; @@ -1134,10 +1135,11 @@ int osp_sync_init(const struct lu_env *env, struct osp_device *d) init_waitqueue_head(&d->opd_syn_thread.t_ctl_waitq); CFS_INIT_LIST_HEAD(&d->opd_syn_committed_there); - rc = PTR_ERR(kthread_run(osp_sync_thread, d, - "osp-syn-%u-%u", d->opd_index, d->opd_group)); - if (IS_ERR_VALUE(rc)) { - CERROR("%s: can't start sync thread: rc = %d\n", + task = kthread_run(osp_sync_thread, d, "osp-syn-%u-%u", + d->opd_index, d->opd_group); + if (IS_ERR(task)) { + rc = PTR_ERR(task); + CERROR("%s: cannot start sync thread: rc = %d\n", d->opd_obd->obd_name, rc); GOTO(err_llog, rc); } diff --git a/lustre/ptlrpc/pinger.c b/lustre/ptlrpc/pinger.c index 7606e69..daa8aa2 100644 --- a/lustre/ptlrpc/pinger.c +++ b/lustre/ptlrpc/pinger.c @@ -318,6 +318,7 @@ static struct ptlrpc_thread pinger_thread; int ptlrpc_start_pinger(void) { struct l_wait_info lwi = { 0 }; + struct task_struct *task; int rc; #ifndef ENABLE_PINGER return 0; @@ -334,12 +335,14 @@ int ptlrpc_start_pinger(void) /* CLONE_VM and CLONE_FILES just avoid a needless copy, because we * just drop the VM and FILES in kthread_run() right away. */ - rc = PTR_ERR(kthread_run(ptlrpc_pinger_main, - &pinger_thread, pinger_thread.t_name)); - if (IS_ERR_VALUE(rc)) { - CERROR("cannot start thread: %d\n", rc); + task = kthread_run(ptlrpc_pinger_main, &pinger_thread, + pinger_thread.t_name); + if (IS_ERR(task)) { + rc = PTR_ERR(task); + CERROR("cannot start pinger thread: rc = %d\n", rc); RETURN(rc); } + l_wait_event(pinger_thread.t_ctl_waitq, thread_is_running(&pinger_thread), &lwi); diff --git a/lustre/ptlrpc/service.c b/lustre/ptlrpc/service.c index b312f8b4..d9476b4 100644 --- a/lustre/ptlrpc/service.c +++ b/lustre/ptlrpc/service.c @@ -2742,25 +2742,31 @@ static int ptlrpc_start_hr_threads(void) int rc = 0; for (j = 0; j < hrp->hrp_nthrs; j++) { - struct ptlrpc_hr_thread *hrt = &hrp->hrp_thrs[j]; - rc = PTR_ERR(kthread_run(ptlrpc_hr_main, - &hrp->hrp_thrs[j], - "ptlrpc_hr%02d_%03d", - hrp->hrp_cpt, - hrt->hrt_id)); - if (IS_ERR_VALUE(rc)) + struct ptlrpc_hr_thread *hrt = &hrp->hrp_thrs[j]; + struct task_struct *task; + + task = kthread_run(ptlrpc_hr_main, + &hrp->hrp_thrs[j], + "ptlrpc_hr%02d_%03d", + hrp->hrp_cpt, + hrt->hrt_id); + if (IS_ERR(task)) { + rc = PTR_ERR(task); break; + } } + wait_event(ptlrpc_hr.hr_waitq, - atomic_read(&hrp->hrp_nstarted) == j); - if (!IS_ERR_VALUE(rc)) - continue; + atomic_read(&hrp->hrp_nstarted) == j); - CERROR("Reply handling thread %d:%d Failed on starting: " - "rc = %d\n", i, j, rc); - ptlrpc_stop_hr_threads(); - RETURN(rc); + if (rc < 0) { + CERROR("cannot start reply handler thread %d:%d: " + "rc = %d\n", i, j, rc); + ptlrpc_stop_hr_threads(); + RETURN(rc); + } } + RETURN(0); } @@ -2869,6 +2875,7 @@ int ptlrpc_start_thread(struct ptlrpc_service_part *svcpt, int wait) struct l_wait_info lwi = { 0 }; struct ptlrpc_thread *thread; struct ptlrpc_service *svc; + struct task_struct *task; int rc; ENTRY; @@ -2936,9 +2943,10 @@ int ptlrpc_start_thread(struct ptlrpc_service_part *svcpt, int wait) } CDEBUG(D_RPCTRACE, "starting thread '%s'\n", thread->t_name); - rc = PTR_ERR(kthread_run(ptlrpc_main, thread, thread->t_name)); - if (IS_ERR_VALUE(rc)) { - CERROR("cannot start thread '%s': rc %d\n", + task = kthread_run(ptlrpc_main, thread, "%s", thread->t_name); + if (IS_ERR(task)) { + rc = PTR_ERR(task); + CERROR("cannot start thread '%s': rc = %d\n", thread->t_name, rc); spin_lock(&svcpt->scp_lock); --svcpt->scp_nthrs_starting; diff --git a/lustre/quota/qsd_reint.c b/lustre/quota/qsd_reint.c index 08e8f93..fea082f 100644 --- a/lustre/quota/qsd_reint.c +++ b/lustre/quota/qsd_reint.c @@ -623,6 +623,7 @@ int qsd_start_reint_thread(struct qsd_qtype_info *qqi) struct ptlrpc_thread *thread = &qqi->qqi_reint_thread; struct qsd_instance *qsd = qqi->qqi_qsd; struct l_wait_info lwi = { 0 }; + struct task_struct *task; int rc; char *name; ENTRY; @@ -664,10 +665,11 @@ int qsd_start_reint_thread(struct qsd_qtype_info *qqi) snprintf(name, MTI_NAME_MAXLEN, "qsd_reint_%d.%s", qqi->qqi_qtype, qsd->qsd_svname); - rc = PTR_ERR(kthread_run(qsd_reint_main, (void *)qqi, name)); + task = kthread_run(qsd_reint_main, qqi, name); OBD_FREE(name, MTI_NAME_MAXLEN); - if (IS_ERR_VALUE(rc)) { + if (IS_ERR(task)) { + rc = PTR_ERR(task); thread_set_flags(thread, SVC_STOPPED); write_lock(&qsd->qsd_lock); qqi->qqi_reint = 0; -- 1.8.3.1