From: Dan Carpenter <dan.carpenter@oracle.com>
To: Oleg Drokin <green@linuxhacker.ru>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org,
Oleg Drokin <oleg.drokin@intel.com>,
"John L. Hammond" <john.hammond@intel.com>
Subject: Re: [PATCH 41/47] staging/lustre/llite: remove dead code
Date: Tue, 29 Apr 2014 14:02:52 +0300 [thread overview]
Message-ID: <20140429110251.GC26890@mwanda> (raw)
In-Reply-To: <1398618431-29757-42-git-send-email-green@linuxhacker.ru>
On Sun, Apr 27, 2014 at 01:07:05PM -0400, Oleg Drokin wrote:
> From: "John L. Hammond" <john.hammond@intel.com>
>
> In llite remove unused declarations, parameters, types, and unused,
> get-only, or set-only structure members. Add static and const
> qualifiers to declarations where possible.
>
> Signed-off-by: John L. Hammond <john.hammond@intel.com>
> Reviewed-on: http://review.whamcloud.com/9767
> Intel-bug-id: https://jira.hpdd.intel.com/browse/LU-2675
> Reviewed-by: Lai Siyao <lai.siyao@intel.com>
> Reviewed-by: Jinshan Xiong <jinshan.xiong@intel.com>
> Signed-off-by: Oleg Drokin <oleg.drokin@intel.com>
This is a random grab bag of changes to lots of files. One thing per
patch, etc, next time.
> diff --git a/drivers/staging/lustre/lustre/llite/dcache.c b/drivers/staging/lustre/lustre/llite/dcache.c
> index 8b55080..7d520d8 100644
> --- a/drivers/staging/lustre/lustre/llite/dcache.c
> +++ b/drivers/staging/lustre/lustre/llite/dcache.c
> @@ -69,8 +69,7 @@ static void ll_release(struct dentry *de)
> ll_intent_release(lld->lld_it);
> OBD_FREE(lld->lld_it, sizeof(*lld->lld_it));
> }
> - LASSERT(lld->lld_cwd_count == 0);
> - LASSERT(lld->lld_mnt_count == 0);
> +
I'm totally in favour of removing LASSERT() calls... But is this a
"set only" struct member? It's totally unclear from the patch
description.
> diff --git a/drivers/staging/lustre/lustre/llite/statahead.c b/drivers/staging/lustre/lustre/llite/statahead.c
> index 51c5327..1b47774 100644
> --- a/drivers/staging/lustre/lustre/llite/statahead.c
> +++ b/drivers/staging/lustre/lustre/llite/statahead.c
> @@ -1230,9 +1230,7 @@ do_it:
> */
> ll_release_page(page, le32_to_cpu(dp->ldp_flags) &
> LDF_COLLIDE);
> - sai->sai_in_readpage = 1;
> page = ll_get_dir_page(dir, pos, &chain);
> - sai->sai_in_readpage = 0;
> } else {
> LASSERT(le32_to_cpu(dp->ldp_flags) & LDF_COLLIDE);
> ll_release_page(page, 1);
> @@ -1563,12 +1561,6 @@ int do_statahead_enter(struct inode *dir, struct dentry **dentryp,
> return entry ? 1 : -EAGAIN;
> }
>
> - /* if statahead is busy in readdir, help it do post-work */
> - while (!ll_sa_entry_stated(entry) &&
> - sai->sai_in_readpage &&
> - !sa_received_empty(sai))
> - ll_post_statahead(sai);
> -
> if (!ll_sa_entry_stated(entry)) {
> sai->sai_index_wait = entry->se_index;
> lwi = LWI_TIMEOUT_INTR(cfs_time_seconds(30), NULL,
What is this change about really? I've already waded through 1271 lines
of random changes at this point and now I have to figure out what
ll_post_statahead() does and why we don't need to call it now?
Anyways, please explain this change.
Btw, it's very easy to break up a patch into separate patches. Use
git citool. Highlight the lines of code which make things static.
Right click and select "Add highlighted lines to commit." This patch
has three Reviewed-by tags so you guys need to be pushing back on this
kind of stuff, it's obviously outside of the "one thing per patch" rule.
regards,
dan carpenter
next prev parent reply other threads:[~2014-04-29 11:03 UTC|newest]
Thread overview: 65+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-04-27 17:06 [PATCH 00/47] Lustre fixes and cleanups Oleg Drokin
2014-04-27 17:06 ` [PATCH 01/47] staging/lustre/ptlrpc: Fix assertion failure of null_alloc_rs() Oleg Drokin
2014-04-27 17:06 ` [PATCH 02/47] staging/lustre/ptlrpc: Remove log message about export timer update Oleg Drokin
2014-04-27 17:06 ` [PATCH 03/47] staging/lustre/gss: gssnull security flavor Oleg Drokin
2014-04-27 17:06 ` [PATCH 04/47] staging/lustre/gss: Shared key mechanism & flavors Oleg Drokin
2014-04-27 17:20 ` Greg Kroah-Hartman
2014-04-27 17:06 ` [PATCH 05/47] staging/lustre/osc: don't activate deactivated obd_import Oleg Drokin
2014-04-27 17:06 ` [PATCH 06/47] staging/lustre/lnet: Dropped messages are not accounted correctly Oleg Drokin
2014-04-27 17:06 ` [PATCH 07/47] staging/lustre/ldlm: Hold lock when clearing flag Oleg Drokin
2014-04-27 17:06 ` [PATCH 08/47] staging/lustre/clio: clear nowait flag agl lock re-enqueue Oleg Drokin
2014-04-27 17:06 ` [PATCH 09/47] staging/lustre/ptlrpc: don't try to recover no_recov connection Oleg Drokin
2014-04-27 17:06 ` [PATCH 10/47] staging/lustre/gss: fix few issues found by Klocwork Insight tool Oleg Drokin
2014-04-27 17:06 ` [PATCH 11/47] staging/lustre/ptlrpc: add rpc_cache Oleg Drokin
2014-04-29 9:46 ` Dan Carpenter
2014-04-30 3:22 ` Oleg Drokin
2014-04-27 17:06 ` [PATCH 12/47] staging/lustre: restore __GFP_WAIT flag to memalloc calls Oleg Drokin
2014-04-27 17:06 ` [PATCH 13/47] staging/lustre/gss: fix uninitialized variable Oleg Drokin
2014-04-27 17:06 ` [PATCH 14/47] staging/lustre: quiet console permission error messages Oleg Drokin
2014-04-27 17:06 ` [PATCH 15/47] staging/lustre/lov: remove unused lov llog code Oleg Drokin
2014-04-27 17:06 ` [PATCH 16/47] staging/lustre/obdclass: remove uses of lov_stripe_md Oleg Drokin
2014-04-27 17:06 ` [PATCH 17/47] staging/lustre/hsm: count NULL terminator in hai_zero/hal_size Oleg Drokin
2014-04-27 17:06 ` [PATCH 18/47] staging/lustre/hsm: HSM requests not delivered Oleg Drokin
2014-04-29 9:08 ` Dan Carpenter
2014-04-30 3:31 ` Oleg Drokin
2014-04-27 17:06 ` [PATCH 19/47] staging/lustre: fix permission problem of setfacl Oleg Drokin
2014-04-27 17:06 ` [PATCH 20/47] staging/lustre/llite: issue OST_SYNC for fsync() Oleg Drokin
2014-04-27 17:06 ` [PATCH 21/47] staging/lustre/llite: deadlock taking lli_trunc_sem during file write Oleg Drokin
2014-04-27 17:06 ` [PATCH 22/47] staging/lustre/lov: to not hold sub locks at initialization Oleg Drokin
2014-04-27 17:06 ` [PATCH 23/47] staging/lustre: Limit reply buffer size Oleg Drokin
2014-04-27 17:06 ` [PATCH 24/47] staging/lustre/llite: Avoid statahead thread start/stop deadlocks Oleg Drokin
2014-04-27 17:06 ` [PATCH 25/47] stagaing/lustre: Improve statahead debug messages Oleg Drokin
2014-04-27 17:06 ` [PATCH 26/47] staging/lustre/llite: access layout version under a lock Oleg Drokin
2014-04-27 17:06 ` [PATCH 27/47] staging/lustre: shrink lu_object_header by 8 bytes on x86_64 Oleg Drokin
2014-04-27 17:06 ` [PATCH 28/47] staging/lustre/ldlm: fix NULL pointer dereference Oleg Drokin
2014-04-27 17:06 ` [PATCH 29/47] staging/lustre/lnet: lnet: fix issues found by Klocwork Insight tool Oleg Drokin
2014-04-27 17:25 ` Greg Kroah-Hartman
2014-04-27 17:06 ` [PATCH 30/47] staging/lustre/mdc: fix issue " Oleg Drokin
2014-04-29 10:20 ` Dan Carpenter
2014-04-27 17:06 ` [PATCH 31/47] staging/lustre/libcfs: fix issues " Oleg Drokin
2014-04-27 17:06 ` [PATCH 32/47] staging/lustre/lnet: NI shutdown may loop forever Oleg Drokin
2014-04-27 17:06 ` [PATCH 33/47] staging/lustre: remove lustre/include/ioctl.h Oleg Drokin
2014-04-27 17:06 ` [PATCH 34/47] staging/lustre/libcfs: add CPU table functions for uniprocessor Oleg Drokin
2014-04-29 10:35 ` Dan Carpenter
2014-04-27 17:06 ` [PATCH 35/47] staging/lustre: replace semaphores with mutexes Oleg Drokin
2014-04-27 17:07 ` [PATCH 36/47] staging/lustre/clio: replace semaphore with mutex Oleg Drokin
2014-04-27 17:07 ` [PATCH 37/47] staging/lustre/llite: Do not rate limit dirty page discard warning Oleg Drokin
2014-04-27 17:07 ` [PATCH 38/47] staging/lustre/lloop: avoid panic during blockdev_info Oleg Drokin
2014-04-27 17:07 ` [PATCH 39/47] staging/lustre/clio: Solve a race in cl_lock_put Oleg Drokin
2014-04-27 17:07 ` [PATCH 40/47] staging/lustre/mdc: use cl_max_mds_md to pack getattr RPC Oleg Drokin
2014-04-27 17:07 ` [PATCH 41/47] staging/lustre/llite: remove dead code Oleg Drokin
2014-04-29 11:02 ` Dan Carpenter [this message]
2014-04-29 19:16 ` Hammond, John
2014-04-29 20:17 ` Dan Carpenter
2014-04-30 3:21 ` Oleg Drokin
2014-04-30 8:01 ` Dan Carpenter
2014-04-29 11:12 ` Richard Weinberger
2014-04-27 17:07 ` [PATCH 42/47] staging/lustre: remove assertion of spin_is_locked() Oleg Drokin
2014-04-27 17:07 ` [PATCH 43/47] staging/lustre/osc: Update inode timestamp for lockless IO as well Oleg Drokin
2014-04-27 17:07 ` [PATCH 44/47] staging/lustre: Always clamp cdls_delay between min and max Oleg Drokin
2014-04-27 17:07 ` [PATCH 45/47] staging/lustre: pass fsync() range through RPC/IO stack Oleg Drokin
2014-04-27 17:07 ` [PATCH 46/47] staging/lustre: Fix unsafe userspace access in many proc files Oleg Drokin
2014-04-27 17:30 ` Greg Kroah-Hartman
2014-04-27 17:07 ` [PATCH 47/47] staging/lustre/llite: prevent buffer overflow in fiemap Oleg Drokin
2014-04-27 17:33 ` [PATCH 00/47] Lustre fixes and cleanups Greg Kroah-Hartman
2014-04-27 18:28 ` Oleg Drokin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20140429110251.GC26890@mwanda \
--to=dan.carpenter@oracle.com \
--cc=devel@driverdev.osuosl.org \
--cc=green@linuxhacker.ru \
--cc=gregkh@linuxfoundation.org \
--cc=john.hammond@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=oleg.drokin@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).