From: Byungchul Park <byungchul.park@lge.com>
To: Ingo Molnar <mingo@kernel.org>
Cc: peterz@infradead.org, axboe@kernel.dk, johan@kernel.org,
tglx@linutronix.de, linux-kernel@vger.kernel.org,
linux-mm@kvack.org, tj@kernel.org, johannes.berg@intel.com,
oleg@redhat.com, amir73il@gmail.com, david@fromorbit.com,
darrick.wong@oracle.com, linux-xfs@vger.kernel.org,
linux-fsdevel@vger.kernel.org, linux-block@vger.kernel.org,
hch@infradead.org, idryomov@gmail.com, kernel-team@lge.com
Subject: Re: [PATCH v4 7/7] block: Assign a lock_class per gendisk used for wait_for_completion()
Date: Wed, 25 Oct 2017 15:34:35 +0900 [thread overview]
Message-ID: <20171025063435.GQ3310@X58A-UD3R> (raw)
In-Reply-To: <20171025060123.6mugpdpje6hx32nx@gmail.com>
On Wed, Oct 25, 2017 at 08:01:24AM +0200, Ingo Molnar wrote:
> Beyond the #ifdef reduction I mentioned in the other thread, there's four other
> things I noticed that need to be fixed in this patch:
>
> - Please write out 'minor' instead of the 'm' abbreviation that is meaningless.
> 'm' is only used for trivial wrappers, but this wrapper is not trivial - so
> proper canonical variable names should be used.
>
> - Since __key and __name is already double underscores that is customary for
> macros to avoid variable name shadowing, why is 'ret' not such a name?
>
> - But, 'ret' is the typical name used for integer returns, not for pointers!
> Please check the gendisk code for what the typical name for gendisk pointers
> is and use that instead of making up new, weird patterns ...
>
> - The "(complete)"#minor"("#id")" generated name is pretty bad. Firstly
> "complete" is a verb (or adjective), while lock(dep) symbol names should be
> nouns! But even "completion" is pretty opaque, how about "gendisk_completion"?
>
> More careful patches please!
I am sorry for that. I will do my best not to repeat them.
And I re-spined ASAP to make you able to review with the latest version
only including all your suggestions at the previous version.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
prev parent reply other threads:[~2017-10-25 6:34 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-25 5:11 [PATCH v4 0/7] cross-release: Enhence performance and fix false positives Byungchul Park
2017-10-25 5:11 ` [PATCH v4 1/7] block: use DECLARE_COMPLETION_ONSTACK in submit_bio_wait Byungchul Park
2017-10-25 5:11 ` [PATCH v4 2/7] locking/lockdep: Add a boot parameter allowing unwind in cross-release and disable it by default Byungchul Park
2017-10-25 5:11 ` [PATCH v4 3/7] locking/lockdep: Remove the BROKEN flag from CONFIG_LOCKDEP_CROSSRELEASE and CONFIG_LOCKDEP_COMPLETIONS Byungchul Park
2017-10-25 5:11 ` [PATCH v4 4/7] locking/lockdep: Introduce CONFIG_BOOTPARAM_LOCKDEP_CROSSRELEASE_FULLSTACK Byungchul Park
2017-10-25 5:11 ` [PATCH v4 5/7] completion: Add support for initializing completion with lockdep_map Byungchul Park
2017-10-25 5:11 ` [PATCH v4 6/7] workqueue: Remove unnecessary acquisitions wrt workqueue flush Byungchul Park
2017-10-25 6:35 ` Ingo Molnar
2017-10-25 5:11 ` [PATCH v4 7/7] block: Assign a lock_class per gendisk used for wait_for_completion() Byungchul Park
2017-10-25 6:01 ` Ingo Molnar
2017-10-25 6:34 ` Byungchul Park [this message]
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=20171025063435.GQ3310@X58A-UD3R \
--to=byungchul.park@lge.com \
--cc=amir73il@gmail.com \
--cc=axboe@kernel.dk \
--cc=darrick.wong@oracle.com \
--cc=david@fromorbit.com \
--cc=hch@infradead.org \
--cc=idryomov@gmail.com \
--cc=johan@kernel.org \
--cc=johannes.berg@intel.com \
--cc=kernel-team@lge.com \
--cc=linux-block@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-xfs@vger.kernel.org \
--cc=mingo@kernel.org \
--cc=oleg@redhat.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=tj@kernel.org \
/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).