Re: [Intel-gfx] [PATCH v2 1/2] mm: Export nr_swap_pages

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon 07-12-15 14:13:46, Johannes Weiner wrote:
> On Mon, Dec 07, 2015 at 06:10:00PM +0000, Dave Gordon wrote:
> > Exporting random uncontrolled variables from the kernel to loaded modules is
> > not really considered best practice. It would be preferable to provide an
> > accessor function - which is just what the declaration says we have; the
> > implementation as a static inline (and/or macro) is what causes the problem
> > here.
> 
> No, what causes the problem is thinking we can't trust in-kernel code.

This is not about the trust. It is about a clear API and separation.

> If somebody screws up, we can fix it easily enough. Sure, we shouldn't
> be laying traps and create easy-to-misuse interfaces, but that's not
> what's happening here. There is no reason to add function overhead to
> what should be a single 'mov' instruction.

The mere fact that the current implementation is a simple atomic_long_read
is a detail and not important for the API. The function is not used
in any hot path where a single function call overhead would be a
performance killer. Exporting implementation details to random users
tends to add maintenance burden in future.

I think it is natural to export symbols which are consumed by modules
and that will be get_nr_swap_pages(). I do not even understand the
resistance against that. Anyway I am not going to argue about it more.
I have raised my review comment and leave the decision to Chris/Andrew.
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]