From: Christoph Hellwig <hch@lst.de>
To: Dan Williams <dan.j.williams@intel.com>
Cc: "Michal Hocko" <mhocko@suse.com>,
"kbuild test robot" <lkp@intel.com>,
linux-nvdimm@lists.01.org,
"Heiko Carstens" <heiko.carstens@de.ibm.com>,
linux-mm@kvack.org, linux-fsdevel@vger.kernel.org,
"Jérôme Glisse" <jglisse@redhat.com>,
"Martin Schwidefsky" <schwidefsky@de.ibm.com>,
"Jan Kara" <jack@suse.cz>, "Christoph Hellwig" <hch@lst.de>,
"Thomas Meyer" <thomas@m3y3r.de>
Subject: Re: [PATCH v10] mm: introduce MEMORY_DEVICE_FS_DAX and CONFIG_DEV_PAGEMAP_OPS
Date: Fri, 18 May 2018 11:46:16 +0200 [thread overview]
Message-ID: <20180518094616.GA25838@lst.de> (raw)
In-Reply-To: <152658753673.26786.16458605771414761966.stgit@dwillia2-desk3.amr.corp.intel.com>
This looks reasonable to me. A few more comments below.
> This patch replaces and consolidates patch 2 [1] and 4 [2] from the v9
> series [3] for "dax: fix dma vs truncate/hole-punch".
Can you repost the whole series? Otherwise things might get a little
too confusing.
> WARN_ON(IS_ENABLED(CONFIG_ARCH_HAS_PMEM_API));
> + return 0;
> } else if (pfn_t_devmap(pfn)) {
> + struct dev_pagemap *pgmap;
This should probably become something like:
bool supported = false;
...
if (IS_ENABLED(CONFIG_FS_DAX_LIMITED) && pfn_t_special(pfn)) {
...
supported = true;
} else if (pfn_t_devmap(pfn)) {
pgmap = get_dev_pagemap(pfn_t_to_pfn(pfn), NULL);
if (pgmap && pgmap->type == MEMORY_DEVICE_FS_DAX)
supported = true;
put_dev_pagemap(pgmap);
}
if (!supported) {
pr_debug("VFS (%s): error: dax support not enabled\n",
sb->s_id);
return -EOPNOTSUPP;
}
return 0;
> + select DEV_PAGEMAP_OPS if (ZONE_DEVICE && !FS_DAX_LIMITED)
Btw, what was the reason again we couldn't get rid of FS_DAX_LIMITED?
> +void generic_dax_pagefree(struct page *page, void *data)
> +{
> + wake_up_var(&page->_refcount);
> +}
> +EXPORT_SYMBOL_GPL(generic_dax_pagefree);
Why is this here and exported instead of static in drivers/nvdimm/pmem.c?
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm
WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@lst.de>
To: Dan Williams <dan.j.williams@intel.com>
Cc: linux-nvdimm@lists.01.org,
"Martin Schwidefsky" <schwidefsky@de.ibm.com>,
"Heiko Carstens" <heiko.carstens@de.ibm.com>,
"Michal Hocko" <mhocko@suse.com>,
"kbuild test robot" <lkp@intel.com>,
"Thomas Meyer" <thomas@m3y3r.de>,
"Dave Jiang" <dave.jiang@intel.com>,
"Christoph Hellwig" <hch@lst.de>,
"Jérôme Glisse" <jglisse@redhat.com>, "Jan Kara" <jack@suse.cz>,
linux-fsdevel@vger.kernel.org, linux-mm@kvack.org
Subject: Re: [PATCH v10] mm: introduce MEMORY_DEVICE_FS_DAX and CONFIG_DEV_PAGEMAP_OPS
Date: Fri, 18 May 2018 11:46:16 +0200 [thread overview]
Message-ID: <20180518094616.GA25838@lst.de> (raw)
In-Reply-To: <152658753673.26786.16458605771414761966.stgit@dwillia2-desk3.amr.corp.intel.com>
This looks reasonable to me. A few more comments below.
> This patch replaces and consolidates patch 2 [1] and 4 [2] from the v9
> series [3] for "dax: fix dma vs truncate/hole-punch".
Can you repost the whole series? Otherwise things might get a little
too confusing.
> WARN_ON(IS_ENABLED(CONFIG_ARCH_HAS_PMEM_API));
> + return 0;
> } else if (pfn_t_devmap(pfn)) {
> + struct dev_pagemap *pgmap;
This should probably become something like:
bool supported = false;
...
if (IS_ENABLED(CONFIG_FS_DAX_LIMITED) && pfn_t_special(pfn)) {
...
supported = true;
} else if (pfn_t_devmap(pfn)) {
pgmap = get_dev_pagemap(pfn_t_to_pfn(pfn), NULL);
if (pgmap && pgmap->type == MEMORY_DEVICE_FS_DAX)
supported = true;
put_dev_pagemap(pgmap);
}
if (!supported) {
pr_debug("VFS (%s): error: dax support not enabled\n",
sb->s_id);
return -EOPNOTSUPP;
}
return 0;
> + select DEV_PAGEMAP_OPS if (ZONE_DEVICE && !FS_DAX_LIMITED)
Btw, what was the reason again we couldn't get rid of FS_DAX_LIMITED?
> +void generic_dax_pagefree(struct page *page, void *data)
> +{
> + wake_up_var(&page->_refcount);
> +}
> +EXPORT_SYMBOL_GPL(generic_dax_pagefree);
Why is this here and exported instead of static in drivers/nvdimm/pmem.c?
next prev parent reply other threads:[~2018-05-18 9:41 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-05-17 20:06 [PATCH v10] mm: introduce MEMORY_DEVICE_FS_DAX and CONFIG_DEV_PAGEMAP_OPS Dan Williams
2018-05-17 20:06 ` Dan Williams
2018-05-17 20:06 ` Dan Williams
2018-05-18 9:00 ` Jan Kara
2018-05-18 9:00 ` Jan Kara
2018-05-18 9:46 ` Christoph Hellwig [this message]
2018-05-18 9:46 ` Christoph Hellwig
2018-05-18 16:00 ` Dan Williams
2018-05-18 16:00 ` Dan Williams
2018-05-21 9:04 ` Jan Kara
2018-05-22 6:28 ` Christoph Hellwig
2018-05-22 6:28 ` Christoph Hellwig
2018-05-23 18:50 ` Gerald Schaefer
2018-05-23 18:50 ` Gerald Schaefer
2018-05-29 20:26 ` Dan Williams
2018-05-29 20:26 ` Dan Williams
2018-06-01 15:01 ` Gerald Schaefer
2018-06-01 15:01 ` Gerald Schaefer
2018-06-02 0:03 ` Dan Williams
2018-06-02 0:03 ` Dan Williams
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=20180518094616.GA25838@lst.de \
--to=hch@lst.de \
--cc=dan.j.williams@intel.com \
--cc=heiko.carstens@de.ibm.com \
--cc=jack@suse.cz \
--cc=jglisse@redhat.com \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux-nvdimm@lists.01.org \
--cc=lkp@intel.com \
--cc=mhocko@suse.com \
--cc=schwidefsky@de.ibm.com \
--cc=thomas@m3y3r.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.