Re: [PATCH 01/10] mm: make cleancache.c explicitly non-modular

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

 



[Re: [PATCH 01/10] mm: make cleancache.c explicitly non-modular] On 22/09/2015 (Tue 11:20) Konrad Rzeszutek Wilk wrote:

> On Mon, Aug 24, 2015 at 09:10:40PM -0400, Paul Gortmaker wrote:
> > [Re: [PATCH 01/10] mm: make cleancache.c explicitly non-modular] On 24/08/2015 (Mon 20:10) Konrad Rzeszutek Wilk wrote:
> > 
> > > On August 24, 2015 6:14:33 PM EDT, Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx> wrote:
> > > >The Kconfig currently controlling compilation of this code is:
> > > >
> > > >config CLEANCACHE
> > > >bool "Enable cleancache driver to cache clean pages if tmem is present"
> > > >
> > > >...meaning that it currently is not being built as a module by anyone.
> > > 
> > > Why not make it a tristate?
> > 
> > Simple.  I'm making the code consistent with its current behaviour.
> > I'm not looking to extend functionality in code that I don't know
> > intimately.  I can't do that and do it reliably and guarantee it
> > works as a module when it has never been used as such before.
> > 
> > I've got about 130 of these and counting.  Some of them have been bool
> > since before git history ; before the turn of the century.  If there was
> > demand for them to be tristate, then it would have happened by now.  So
> > clearly there is no point in looking at making _those_ tristate.
> > 
> > I did have one uart driver author indicate that he _meant_ his code to
> > be tristate, and he tested it as such, and asked if I would convert it
> > to tristate on his behalf.  And that was fine and I did exactly that.
> > 
> > But unless there are interested users who want their code tristate and
> > can vouch that their code works OK as such, I can only make the code
> > consistent with the implicit non-modular behaviour that the Kconfig and
> > Makefiles have dictated up to now.  Are there such users for CLEANCACHE?
> 
> Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> 
> Sorry for taking so long. It really cannot be tri-state (I tried
> making it an module) as the cleancache hooks are tied in the mm/filemap.c.

No problem, AFAIK these aren't queued anywhere and after having done a
wider sweep, there are some 300+ of them (that give a net removal of ~5k
lines of dead code) with some in maintainer-less code; so it looks like
I'll inevitably have residuals that I'll have to ask Linus to pull directly.

Anyway, thanks for trying to modularize it. I'll add the reviewed tags
and update the commit log on the other one to add your comment.

Paul.
--

> 
> > 
> > Paul.
> > --
> > 
> > > 
> > > 
> > > >
> > > >Lets remove the couple traces of modularity so that when reading the
> > > >driver there is no doubt it is builtin-only.
> > > >
> > > >Since module_init translates to device_initcall in the non-modular
> > > >case, the init ordering remains unchanged with this commit.
> > > >
> > > >Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> > > >Cc: linux-mm@xxxxxxxxx
> > > >Signed-off-by: Paul Gortmaker <paul.gortmaker@xxxxxxxxxxxxx>
> > > >---
> > > > mm/cleancache.c | 4 ++--
> > > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > >diff --git a/mm/cleancache.c b/mm/cleancache.c
> > > >index 8fc50811119b..ee0646d1c2fa 100644
> > > >--- a/mm/cleancache.c
> > > >+++ b/mm/cleancache.c
> > > >@@ -11,7 +11,7 @@
> > > >  * This work is licensed under the terms of the GNU GPL, version 2.
> > > >  */
> > > > 
> > > >-#include <linux/module.h>
> > > >+#include <linux/init.h>
> > > > #include <linux/fs.h>
> > > > #include <linux/exportfs.h>
> > > > #include <linux/mm.h>
> > > >@@ -316,4 +316,4 @@ static int __init init_cleancache(void)
> > > > #endif
> > > > 	return 0;
> > > > }
> > > >-module_init(init_cleancache)
> > > >+device_initcall(init_cleancache)
> > > 
> > > 

--
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]