From: Ira Weiny <ira.weiny@intel.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Christoph Hellwig <hch@lst.de>, Jens Axboe <axboe@kernel.dk>,
Thomas Gleixner <tglx@linutronix.de>,
Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
Geoff Levand <geoff@infradead.org>,
Ilya Dryomov <idryomov@gmail.com>,
Dongsheng Yang <dongsheng.yang@easystack.cn>,
Mike Snitzer <snitzer@redhat.com>,
"James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>,
dm-devel@redhat.com, linux-mips@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-block@vger.kernel.org,
linuxppc-dev@lists.ozlabs.org, ceph-devel@vger.kernel.org,
linux-arch@vger.kernel.org,
"David S. Miller" <davem@davemloft.net>,
Christoph Lameter <cl@gentwo.de>
Subject: Re: [PATCH 01/18] mm: add a kunmap_local_dirty helper
Date: Fri, 18 Jun 2021 11:12:58 -0700 [thread overview]
Message-ID: <20210618181258.GC1905674@iweiny-DESK2.sc.intel.com> (raw)
In-Reply-To: <20210618033728.GA16787@gondor.apana.org.au>
On Fri, Jun 18, 2021 at 11:37:28AM +0800, Herbert Xu wrote:
> On Thu, Jun 17, 2021 at 08:01:57PM -0700, Ira Weiny wrote:
> >
> > > + flush_kernel_dcache_page(__page); \
> >
> > Is this required on 32bit systems? Why is kunmap_flush_on_unmap() not
> > sufficient on 64bit systems? The normal kunmap_local() path does that.
> >
> > I'm sorry but I did not see a conclusion to my query on V1. Herbert implied the
> > he just copied from the crypto code.[1] I'm concerned that this _dirty() call
> > is just going to confuse the users of kmap even more. So why can't we get to
> > the bottom of why flush_kernel_dcache_page() needs so much logic around it
> > before complicating the general kernel users.
> >
> > I would like to see it go away if possible.
>
> This thread may be related:
>
> https://lwn.net/Articles/240249/
Interesting! Thanks!
Digging around a bit more I found:
https://lore.kernel.org/patchwork/patch/439637/
Auditing all the flush_dcache_page() arch code reveals that the mapping field
is either unused, or is checked for NULL. Furthermore, all the implementations
call page_mapping_file() which further limits the page to not be a swap page.
All flush_kernel_dcache_page() implementations appears to operate the same way
in all arch's which define that call.
So I'm confident now that additional !PageSlab(__page) checks are not needed
and this patch is unnecessary. Christoph, can we leave this out of the kmap
API and just fold the flush_kernel_dcache_page() calls back into the bvec code?
Unfortunately, I'm not convinced this can be handled completely by
kunmap_local() nor the mem*_page() calls because there is a difference between
flush_dcache_page() and flush_kernel_dcache_page() in most archs... [parisc
being an exception which falls back to flush_kernel_dcache_page()]...
It seems like the generic unmap path _should_ be able to determine which call
to make based on the page but I'd have to look at that more.
Ira
WARNING: multiple messages have this Message-ID (diff)
From: Ira Weiny <ira.weiny@intel.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Jens Axboe <axboe@kernel.dk>,
linux-block@vger.kernel.org,
Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
Mike Snitzer <snitzer@redhat.com>,
"David S. Miller" <davem@davemloft.net>,
Geoff Levand <geoff@infradead.org>,
Christoph Lameter <cl@gentwo.de>,
ceph-devel@vger.kernel.org, linux-mips@vger.kernel.org,
Dongsheng Yang <dongsheng.yang@easystack.cn>,
linux-kernel@vger.kernel.org,
"James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>,
dm-devel@redhat.com, linux-arch@vger.kernel.org,
Thomas Gleixner <tglx@linutronix.de>,
Ilya Dryomov <idryomov@gmail.com>,
linuxppc-dev@lists.ozlabs.org, Christoph Hellwig <hch@lst.de>
Subject: Re: [PATCH 01/18] mm: add a kunmap_local_dirty helper
Date: Fri, 18 Jun 2021 11:12:58 -0700 [thread overview]
Message-ID: <20210618181258.GC1905674@iweiny-DESK2.sc.intel.com> (raw)
In-Reply-To: <20210618033728.GA16787@gondor.apana.org.au>
On Fri, Jun 18, 2021 at 11:37:28AM +0800, Herbert Xu wrote:
> On Thu, Jun 17, 2021 at 08:01:57PM -0700, Ira Weiny wrote:
> >
> > > + flush_kernel_dcache_page(__page); \
> >
> > Is this required on 32bit systems? Why is kunmap_flush_on_unmap() not
> > sufficient on 64bit systems? The normal kunmap_local() path does that.
> >
> > I'm sorry but I did not see a conclusion to my query on V1. Herbert implied the
> > he just copied from the crypto code.[1] I'm concerned that this _dirty() call
> > is just going to confuse the users of kmap even more. So why can't we get to
> > the bottom of why flush_kernel_dcache_page() needs so much logic around it
> > before complicating the general kernel users.
> >
> > I would like to see it go away if possible.
>
> This thread may be related:
>
> https://lwn.net/Articles/240249/
Interesting! Thanks!
Digging around a bit more I found:
https://lore.kernel.org/patchwork/patch/439637/
Auditing all the flush_dcache_page() arch code reveals that the mapping field
is either unused, or is checked for NULL. Furthermore, all the implementations
call page_mapping_file() which further limits the page to not be a swap page.
All flush_kernel_dcache_page() implementations appears to operate the same way
in all arch's which define that call.
So I'm confident now that additional !PageSlab(__page) checks are not needed
and this patch is unnecessary. Christoph, can we leave this out of the kmap
API and just fold the flush_kernel_dcache_page() calls back into the bvec code?
Unfortunately, I'm not convinced this can be handled completely by
kunmap_local() nor the mem*_page() calls because there is a difference between
flush_dcache_page() and flush_kernel_dcache_page() in most archs... [parisc
being an exception which falls back to flush_kernel_dcache_page()]...
It seems like the generic unmap path _should_ be able to determine which call
to make based on the page but I'd have to look at that more.
Ira
WARNING: multiple messages have this Message-ID (diff)
From: Ira Weiny <ira.weiny@intel.com>
To: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Jens Axboe <axboe@kernel.dk>,
linux-block@vger.kernel.org,
Thomas Bogendoerfer <tsbogend@alpha.franken.de>,
Mike Snitzer <snitzer@redhat.com>,
"David S. Miller" <davem@davemloft.net>,
Geoff Levand <geoff@infradead.org>,
Christoph Lameter <cl@gentwo.de>,
ceph-devel@vger.kernel.org, linux-mips@vger.kernel.org,
Dongsheng Yang <dongsheng.yang@easystack.cn>,
linux-kernel@vger.kernel.org,
"James E.J. Bottomley" <James.Bottomley@HansenPartnership.com>,
dm-devel@redhat.com, linux-arch@vger.kernel.org,
Thomas Gleixner <tglx@linutronix.de>,
Ilya Dryomov <idryomov@gmail.com>,
linuxppc-dev@lists.ozlabs.org, Christoph Hellwig <hch@lst.de>
Subject: Re: [dm-devel] [PATCH 01/18] mm: add a kunmap_local_dirty helper
Date: Fri, 18 Jun 2021 11:12:58 -0700 [thread overview]
Message-ID: <20210618181258.GC1905674@iweiny-DESK2.sc.intel.com> (raw)
In-Reply-To: <20210618033728.GA16787@gondor.apana.org.au>
On Fri, Jun 18, 2021 at 11:37:28AM +0800, Herbert Xu wrote:
> On Thu, Jun 17, 2021 at 08:01:57PM -0700, Ira Weiny wrote:
> >
> > > + flush_kernel_dcache_page(__page); \
> >
> > Is this required on 32bit systems? Why is kunmap_flush_on_unmap() not
> > sufficient on 64bit systems? The normal kunmap_local() path does that.
> >
> > I'm sorry but I did not see a conclusion to my query on V1. Herbert implied the
> > he just copied from the crypto code.[1] I'm concerned that this _dirty() call
> > is just going to confuse the users of kmap even more. So why can't we get to
> > the bottom of why flush_kernel_dcache_page() needs so much logic around it
> > before complicating the general kernel users.
> >
> > I would like to see it go away if possible.
>
> This thread may be related:
>
> https://lwn.net/Articles/240249/
Interesting! Thanks!
Digging around a bit more I found:
https://lore.kernel.org/patchwork/patch/439637/
Auditing all the flush_dcache_page() arch code reveals that the mapping field
is either unused, or is checked for NULL. Furthermore, all the implementations
call page_mapping_file() which further limits the page to not be a swap page.
All flush_kernel_dcache_page() implementations appears to operate the same way
in all arch's which define that call.
So I'm confident now that additional !PageSlab(__page) checks are not needed
and this patch is unnecessary. Christoph, can we leave this out of the kmap
API and just fold the flush_kernel_dcache_page() calls back into the bvec code?
Unfortunately, I'm not convinced this can be handled completely by
kunmap_local() nor the mem*_page() calls because there is a difference between
flush_dcache_page() and flush_kernel_dcache_page() in most archs... [parisc
being an exception which falls back to flush_kernel_dcache_page()]...
It seems like the generic unmap path _should_ be able to determine which call
to make based on the page but I'd have to look at that more.
Ira
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel
next prev parent reply other threads:[~2021-06-18 18:13 UTC|newest]
Thread overview: 78+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-06-15 13:24 switch the block layer to use kmap_local_page v2 Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-15 13:24 ` [PATCH 01/18] mm: add a kunmap_local_dirty helper Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-18 3:01 ` Ira Weiny
2021-06-18 3:01 ` [dm-devel] " Ira Weiny
2021-06-18 3:01 ` Ira Weiny
2021-06-18 3:37 ` Herbert Xu
2021-06-18 3:37 ` [dm-devel] " Herbert Xu
2021-06-18 3:37 ` Herbert Xu
2021-06-18 18:12 ` Ira Weiny [this message]
2021-06-18 18:12 ` [dm-devel] " Ira Weiny
2021-06-18 18:12 ` Ira Weiny
2021-06-24 6:32 ` [PATCH] crypto: scatterwalk - Remove obsolete PageSlab check Herbert Xu
2021-06-24 6:32 ` [dm-devel] " Herbert Xu
2021-06-24 6:32 ` Herbert Xu
2021-06-15 13:24 ` [PATCH 02/18] mm: use kunmap_local_dirty in memcpy_to_page Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-15 13:24 ` [PATCH 03/18] mm: use kmap_local_page in memzero_page Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-15 13:24 ` [PATCH 04/18] MIPS: don't include <linux/genhd.h> in <asm/mach-rc32434/rb.h> Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-15 13:24 ` [PATCH 05/18] bvec: fix the include guards for bvec.h Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-15 13:24 ` [PATCH 06/18] bvec: add a bvec_kmap_local helper Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-16 16:52 ` [dm-devel] " Bart Van Assche
2021-06-16 16:52 ` Bart Van Assche
2021-06-16 16:52 ` Bart Van Assche
2021-06-15 13:24 ` [PATCH 07/18] bvec: add memcpy_{from,to}_bvec and memzero_bvec helper Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] [PATCH 07/18] bvec: add memcpy_{from, to}_bvec " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-15 13:24 ` [PATCH 08/18] block: use memzero_page in zero_fill_bio Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-15 13:24 ` [PATCH 09/18] rbd: use memzero_bvec Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-15 13:24 ` [PATCH 10/18] dm-writecache: use bvec_kmap_local instead of bvec_kmap_irq Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-15 13:24 ` [PATCH 11/18] ps3disk: use memcpy_{from,to}_bvec Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-16 16:36 ` Geoff Levand
2021-06-16 16:36 ` [dm-devel] " Geoff Levand
2021-06-16 16:36 ` Geoff Levand
2021-06-15 13:24 ` [PATCH 12/18] block: remove bvec_kmap_irq and bvec_kunmap_irq Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-15 13:24 ` [PATCH 13/18] block: rewrite bio_copy_data_iter to use bvec_kmap_local and memcpy_to_bvec Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-15 13:24 ` [PATCH 14/18] block: use memcpy_to_bvec in copy_to_high_bio_irq Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-15 13:24 ` [PATCH 15/18] block: use memcpy_from_bvec in bio_copy_kern_endio_read Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-15 13:24 ` [PATCH 16/18] block: use memcpy_from_bvec in __blk_queue_bounce Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-15 13:24 ` [PATCH 17/18] block: use bvec_kmap_local in t10_pi_type1_{prepare,complete} Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] [PATCH 17/18] block: use bvec_kmap_local in t10_pi_type1_{prepare, complete} Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-15 13:24 ` [PATCH 18/18] block: use bvec_kmap_local in bio_integrity_process Christoph Hellwig
2021-06-15 13:24 ` [dm-devel] " Christoph Hellwig
2021-06-15 13:24 ` Christoph Hellwig
2021-06-16 16:03 ` switch the block layer to use kmap_local_page v2 Martin K. Petersen
2021-06-16 16:03 ` [dm-devel] " Martin K. Petersen
2021-06-16 16:03 ` Martin K. Petersen
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=20210618181258.GC1905674@iweiny-DESK2.sc.intel.com \
--to=ira.weiny@intel.com \
--cc=James.Bottomley@HansenPartnership.com \
--cc=axboe@kernel.dk \
--cc=ceph-devel@vger.kernel.org \
--cc=cl@gentwo.de \
--cc=davem@davemloft.net \
--cc=dm-devel@redhat.com \
--cc=dongsheng.yang@easystack.cn \
--cc=geoff@infradead.org \
--cc=hch@lst.de \
--cc=herbert@gondor.apana.org.au \
--cc=idryomov@gmail.com \
--cc=linux-arch@vger.kernel.org \
--cc=linux-block@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mips@vger.kernel.org \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=snitzer@redhat.com \
--cc=tglx@linutronix.de \
--cc=tsbogend@alpha.franken.de \
/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.