Thread (79 messages) 79 messages, 6 authors, 2018-09-19

Re: [RFC PATCH 07/29] memblock: remove _virt from APIs returning virtual address

From: Michal Hocko <mhocko@kernel.org>
Date: 2018-09-07 08:48:03
Also in: linux-mm, linuxppc-dev, lkml, sparclinux

On Fri 07-09-18 11:42:12, Mike Rapoport wrote:
On Thu, Sep 06, 2018 at 03:46:27PM +0200, Michal Hocko wrote:
quoted
On Thu 06-09-18 16:39:58, Mike Rapoport wrote:
quoted
On Thu, Sep 06, 2018 at 03:01:02PM +0200, Michal Hocko wrote:
quoted
On Thu 06-09-18 15:43:21, Mike Rapoport wrote:
quoted
On Thu, Sep 06, 2018 at 09:28:00AM +0200, Michal Hocko wrote:
quoted
On Wed 05-09-18 20:20:18, Mike Rapoport wrote:
quoted
On Wed, Sep 05, 2018 at 12:04:36PM -0500, Rob Herring wrote:
quoted
On Wed, Sep 5, 2018 at 11:00 AM Mike Rapoport [off-list ref] wrote:
quoted
The conversion is done using

sed -i 's@memblock_virt_alloc@memblock_alloc@g' \
        $(git grep -l memblock_virt_alloc)
What's the reason to do this? It seems like a lot of churn even if a
mechanical change.
I felt that memblock_virt_alloc_ is too long for a prefix, e.g:
memblock_virt_alloc_node_nopanic, memblock_virt_alloc_low_nopanic.

And for consistency I've changed the memblock_virt_alloc as well.
I would keep the current API unless the name is terribly misleading or
it can be improved a lot. Neither seems to be the case here. So I would
rather stick with the status quo.
I'm ok with the memblock_virt_alloc by itself, but having 'virt' in
'memblock_virt_alloc_try_nid_nopanic' and 'memblock_virt_alloc_low_nopanic'
reduces code readability in my opinion.
Well, is _nopanic really really useful in the name. Do we even need/want
implicit panic/nopanic semantic? The code should rather check for the
return value and decide depending on the code path. I suspect removing
panic/nopanic would make the API slightly lighter.
 
I agree that panic/nopanic should be removed. But I prefer to start with
equivalent replacement to make it as automated as possible and update
memblock API when the dust settles a bit.
Yes, I agree with that approach. But that also doesn't justify the
renaming
Well, the renaming is automated :)
Yes, it is. It also adds churn to the code so I tend to prefer an
existing naming unless it is completely misleading or incomprehensible.

Is this something to lose sleep over. Absolutely not! Does it make sense
to discuss further? I do not think so. If you strongly believe that the
renaming is a good thing then just do it.
-- 
Michal Hocko
SUSE Labs
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help