All of lore.kernel.org
 help / color / mirror / Atom feed
From: Russell King <rmk@arm.linux.org.uk>
To: James Bottomley <James.Bottomley@HansenPartnership.com>
Cc: Ilya Loginov <isloginov@gmail.com>,
	Jens Axboe <jens.axboe@oracle.com>,
	linux-arch@vger.kernel.org
Subject: Re: problems in commit 2d4dc890b5c8 (block: add helpers to run flush_dcache_page() against a bio and a request's pages)
Date: Thu, 10 Dec 2009 17:48:23 +0000	[thread overview]
Message-ID: <20091210174823.GA20884@flint.arm.linux.org.uk> (raw)
In-Reply-To: <1260464851.2457.98.camel@mulgrave.site>

On Thu, Dec 10, 2009 at 11:07:31AM -0600, James Bottomley wrote:
> For PIO we have this kmap/operate/flush_kernel_dcache_page()/kunmap
> complexity (see for example drivers/mmc/mmc_spi.c).  However, it could
> all be taken care of in a similar way to the DMA API via an
> abstraction ... although I suspect the best abstraction is to pull the
> flush into kunmap.

I assume you actually mean kmap_atomic() / kunmap_atomic(), since that
would be used in interrupt driven PIO drivers rather than kmap()/kunmap().

Well, it would mean every kunmap_atomic() gains an expensive cache flush
no matter what it's doing.  That would be very bad for things like
copy_user_highpage(), where we kmap the source and destination pages,
and then kunmap both.

However, there are cases where we do want to flush on kunmap_atomic() -
again, taking the copy_user_highpage() example, we want to ensure that
data written to the page is going to be visible in some way.  IOW, we
already have this:

        kfrom = kmap_atomic(from, KM_USER0);
        kto = kmap_atomic(to, KM_USER1);
        copy_page(kto, kfrom);
#ifdef CONFIG_HIGHMEM
        /*
         * kmap_atomic() doesn't set the page virtual address, and
         * kunmap_atomic() takes care of cache flushing already.
         */
        if (page_address(to) != NULL)
#endif
                __cpuc_flush_dcache_page(kto);
        kunmap_atomic(kto, KM_USER1);
        kunmap_atomic(kfrom, KM_USER0);

would become something like:

	...
	kunmap_atomic_flush(kto, KM_USER1);
	kunmap_atomic(kfrom, KM_USER0);

So I think what we want to add is kunmap_atomic_flush() for the cases
where we need the additional coherency, or maybe a kunmap_atomic_noflush()
for those which we don't.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

  reply	other threads:[~2009-12-10 17:48 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-12-09 22:39 problems in commit 2d4dc890b5c8 (block: add helpers to run flush_dcache_page() against a bio and a request's pages) James Bottomley
2009-12-09 22:45 ` Russell King
2009-12-09 22:56   ` James Bottomley
2009-12-09 23:03 ` Ilya Loginov
2009-12-09 23:11   ` James Bottomley
2009-12-09 23:36     ` Ilya Loginov
2009-12-09 23:47       ` James Bottomley
2009-12-10  0:06         ` Ilya Loginov
2009-12-10  0:19           ` James Bottomley
2009-12-10  4:40             ` Ilya Loginov
2009-12-10 17:07               ` James Bottomley
2009-12-10 17:48                 ` Russell King [this message]
2009-12-10 17:59                   ` James Bottomley
2009-12-10 18:06                     ` Russell King
2009-12-10 18:20                       ` James Bottomley
2009-12-10 19:05                         ` Russell King
2009-12-10 20:29                           ` James Bottomley
2009-12-10 20:39                             ` Russell King
2009-12-10 19:42                         ` Ilya Loginov
2009-12-10 19:43                           ` Russell King
2009-12-10 19:48                             ` Ilya Loginov
2009-12-10 19:46                 ` Ilya Loginov
2009-12-10 20:28                   ` James Bottomley
2009-12-10 20:41                     ` Ilya Loginov
2009-12-10 20:48                     ` Ilya Loginov
2009-12-10 20:59                       ` James Bottomley
2009-12-10 21:27                         ` Ilya Loginov
2009-12-10 21:43                           ` Ilya Loginov
2009-12-10 22:00                           ` James Bottomley
2009-12-10 22:03                             ` David Miller
2009-12-10 22:33                             ` 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=20091210174823.GA20884@flint.arm.linux.org.uk \
    --to=rmk@arm.linux.org.uk \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=isloginov@gmail.com \
    --cc=jens.axboe@oracle.com \
    --cc=linux-arch@vger.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 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.