All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Morton <akpm@linux-foundation.org>
To: Ilya Loginov <isloginov@gmail.com>
Cc: David Woodhouse <dwmw2@infradead.org>,
	linux-kernel@vger.kernel.org, Peter Horton <phorton@bitbox.co.uk>,
	"Ed L. Cashin" <ecashin@coraid.com>,
	Jens Axboe <jens.axboe@oracle.com>
Subject: Re: [PATCH] mtd: fix mtd_blkdevs problem with caches on some architectures (2.6.31)
Date: Sat, 21 Nov 2009 15:26:33 -0800	[thread overview]
Message-ID: <20091121152633.8c79e341.akpm@linux-foundation.org> (raw)
In-Reply-To: <20091122021128.db47e202.isloginov@gmail.com>

On Sun, 22 Nov 2009 02:11:28 +0300 Ilya Loginov <isloginov@gmail.com> wrote:

> I think we should select SYS_HAS_FLUSH_DCACHE_PAGE for those
> architectures which requires it(to fix the bug) (and implement empty
> flush_dcache_page throught inline like in x86(to avoid pointless do
> {} while(0))).
> 
> What do you think about this?

Well, rather that defining CONFIG_SYS_HAS_FLUSH_DCACHE_PAGE in
Kconfig (which is what I think your were thinking of), we could do:

--- a/arch/x86/include/asm/cacheflush.h~a
+++ a/arch/x86/include/asm/cacheflush.h
@@ -13,6 +13,7 @@ static inline void flush_cache_range(str
 static inline void flush_cache_page(struct vm_area_struct *vma,
 				    unsigned long vmaddr, unsigned long pfn) { }
 static inline void flush_dcache_page(struct page *page) { }
+#define ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE 0
 static inline void flush_dcache_mmap_lock(struct address_space *mapping) { }
 static inline void flush_dcache_mmap_unlock(struct address_space *mapping) { }
 static inline void flush_icache_range(unsigned long start,
--- a/arch/arm/include/asm/cacheflush.h~a
+++ a/arch/arm/include/asm/cacheflush.h
@@ -409,6 +409,7 @@ extern void flush_ptrace_access(struct v
  * See update_mmu_cache for the user space part.
  */
 extern void flush_dcache_page(struct page *);
+#define ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE 1
 
 extern void __flush_dcache_page(struct address_space *mapping, struct page *page);
 
(etc)

And then, in a .c file:

#ifndef ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE
#error "you lose"
#endif

and, of course:

#if ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE
<implement real code>
#else
<implement empty stubs>
#endif


This way

a) the definition site for ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE is
   right next to the definition site for flush_dcache_page(), instead
   of being in some random remote file and

b) people can't forget to implement ARCH_IMPLEMENTS_FLUSH_DCACHE_PAGE.


Generally we prefer to avoid defining ARCH_HAS_FOO in header files and
we prefer to control the definition in Kconfig.  But it sounds like we
have a special case here..


  reply	other threads:[~2009-11-21 23:26 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-18 14:08 [PATCH] mtd: fix mtd_blkdevs problem with caches on some architectures (2.6.31) Ilya Loginov
2009-11-21  0:37 ` Andrew Morton
2009-11-21 14:04   ` Ilya Loginov
2009-11-21 17:54     ` Andrew Morton
2009-11-21 23:11       ` Ilya Loginov
2009-11-21 23:26         ` Andrew Morton [this message]
2009-11-21 23:36           ` Ilya Loginov
2009-11-22  9:46           ` Ilya Loginov
2009-11-22  9:53             ` David Woodhouse
2009-11-22 18:49               ` Ilya Loginov
2009-11-22 13:29             ` Ingo Molnar
2009-11-22 13:55               ` Ilya Loginov
2009-11-22 18:48                 ` Andrew Morton
2009-11-22 19:18                   ` Ilya Loginov
2009-11-22 19:51                     ` Andrew Morton
2009-11-22 20:55                       ` Ilya Loginov
2009-11-24 20:48                         ` Andrew Morton
2009-11-25  1:01                           ` Ilya Loginov

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=20091121152633.8c79e341.akpm@linux-foundation.org \
    --to=akpm@linux-foundation.org \
    --cc=dwmw2@infradead.org \
    --cc=ecashin@coraid.com \
    --cc=isloginov@gmail.com \
    --cc=jens.axboe@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=phorton@bitbox.co.uk \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.