From: Christoph Hellwig <hch@infradead.org> To: Jane Chu <jane.chu@oracle.com> Cc: Christoph Hellwig <hch@infradead.org>, "david@fromorbit.com" <david@fromorbit.com>, "djwong@kernel.org" <djwong@kernel.org>, "dan.j.williams@intel.com" <dan.j.williams@intel.com>, "vishal.l.verma@intel.com" <vishal.l.verma@intel.com>, "dave.jiang@intel.com" <dave.jiang@intel.com>, "agk@redhat.com" <agk@redhat.com>, "snitzer@redhat.com" <snitzer@redhat.com>, "dm-devel@redhat.com" <dm-devel@redhat.com>, "ira.weiny@intel.com" <ira.weiny@intel.com>, "willy@infradead.org" <willy@infradead.org>, "vgoyal@redhat.com" <vgoyal@redhat.com>, "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>, "nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org> Subject: Re: [PATCH v6 4/6] dax: add DAX_RECOVERY flag and .recovery_write dev_pgmap_ops Date: Tue, 22 Mar 2022 22:45:58 -0700 [thread overview] Message-ID: <Yjq0FspfsLrN/mrx@infradead.org> (raw) In-Reply-To: <3dabd58b-70f2-12af-419f-a7dfc07fbb1c@oracle.com> On Tue, Mar 22, 2022 at 11:05:09PM +0000, Jane Chu wrote: > > This DAX_RECOVERY doesn't actually seem to be used anywhere here or > > in the subsequent patches. Did I miss something? > > dax_iomap_iter() uses the flag in the same patch, > + if ((map_len == -EIO) && (iov_iter_rw(iter) == WRITE)) { > + flags |= DAX_RECOVERY; > + map_len = dax_direct_access(dax_dev, pgoff, nrpg, > + flags, &kaddr, NULL); Yes, it passes it on to dax_direct_access, and dax_direct_access passes it onto ->direct_access. But nothing in this series actually checks for it as far as I can tell. > >> Also introduce a new dev_pagemap_ops .recovery_write function. > >> The function is applicable to FSDAX device only. The device > >> page backend driver provides .recovery_write function if the > >> device has underlying mechanism to clear the uncorrectable > >> errors on the fly. > > > > Why is this not in struct dax_operations? > > Per Dan's comments to the v5 series, adding .recovery_write to > dax_operations causes a number of trivial dm targets changes. > Dan suggested that adding .recovery_write to pagemap_ops could > cut short the logistics of figuring out whether the driver backing > up a page is indeed capable of clearing poison. Please see > https://lkml.org/lkml/2022/2/4/31 But at least in this series there is 1:1 association between the pgmap and the dax_device so that scheme won't work. It would have to lookup the pgmap based on the return physical address from dax_direct_access. Which sounds more complicated than just adding the (annoying) boilerplate code to DM. > include/linux/memremap.h doesn't know struct iov_iter which is defined > in include/linux/uio.h, would you prefer to adding include/linux/uio.h > to include/linux/memremap.h ? As it is not derefences just adding a struct iov_iter; line to memremap.h below the includes should be all that is needed.
WARNING: multiple messages have this Message-ID (diff)
From: Christoph Hellwig <hch@infradead.org> To: Jane Chu <jane.chu@oracle.com> Cc: "nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>, "dave.jiang@intel.com" <dave.jiang@intel.com>, "snitzer@redhat.com" <snitzer@redhat.com>, "djwong@kernel.org" <djwong@kernel.org>, "david@fromorbit.com" <david@fromorbit.com>, "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>, "willy@infradead.org" <willy@infradead.org>, Christoph Hellwig <hch@infradead.org>, "dm-devel@redhat.com" <dm-devel@redhat.com>, "vgoyal@redhat.com" <vgoyal@redhat.com>, "vishal.l.verma@intel.com" <vishal.l.verma@intel.com>, "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>, "dan.j.williams@intel.com" <dan.j.williams@intel.com>, "ira.weiny@intel.com" <ira.weiny@intel.com>, "linux-xfs@vger.kernel.org" <linux-xfs@vger.kernel.org>, "agk@redhat.com" <agk@redhat.com> Subject: Re: [dm-devel] [PATCH v6 4/6] dax: add DAX_RECOVERY flag and .recovery_write dev_pgmap_ops Date: Tue, 22 Mar 2022 22:45:58 -0700 [thread overview] Message-ID: <Yjq0FspfsLrN/mrx@infradead.org> (raw) In-Reply-To: <3dabd58b-70f2-12af-419f-a7dfc07fbb1c@oracle.com> On Tue, Mar 22, 2022 at 11:05:09PM +0000, Jane Chu wrote: > > This DAX_RECOVERY doesn't actually seem to be used anywhere here or > > in the subsequent patches. Did I miss something? > > dax_iomap_iter() uses the flag in the same patch, > + if ((map_len == -EIO) && (iov_iter_rw(iter) == WRITE)) { > + flags |= DAX_RECOVERY; > + map_len = dax_direct_access(dax_dev, pgoff, nrpg, > + flags, &kaddr, NULL); Yes, it passes it on to dax_direct_access, and dax_direct_access passes it onto ->direct_access. But nothing in this series actually checks for it as far as I can tell. > >> Also introduce a new dev_pagemap_ops .recovery_write function. > >> The function is applicable to FSDAX device only. The device > >> page backend driver provides .recovery_write function if the > >> device has underlying mechanism to clear the uncorrectable > >> errors on the fly. > > > > Why is this not in struct dax_operations? > > Per Dan's comments to the v5 series, adding .recovery_write to > dax_operations causes a number of trivial dm targets changes. > Dan suggested that adding .recovery_write to pagemap_ops could > cut short the logistics of figuring out whether the driver backing > up a page is indeed capable of clearing poison. Please see > https://lkml.org/lkml/2022/2/4/31 But at least in this series there is 1:1 association between the pgmap and the dax_device so that scheme won't work. It would have to lookup the pgmap based on the return physical address from dax_direct_access. Which sounds more complicated than just adding the (annoying) boilerplate code to DM. > include/linux/memremap.h doesn't know struct iov_iter which is defined > in include/linux/uio.h, would you prefer to adding include/linux/uio.h > to include/linux/memremap.h ? As it is not derefences just adding a struct iov_iter; line to memremap.h below the includes should be all that is needed. -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
next prev parent reply other threads:[~2022-03-23 5:46 UTC|newest] Thread overview: 52+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-03-19 6:28 [PATCH v6 0/6] DAX poison recovery Jane Chu 2022-03-19 6:28 ` [dm-devel] " Jane Chu 2022-03-19 6:28 ` [PATCH v6 1/6] x86/mm: fix comment Jane Chu 2022-03-19 6:28 ` [dm-devel] " Jane Chu 2022-03-22 8:40 ` Christoph Hellwig 2022-03-22 8:40 ` [dm-devel] " Christoph Hellwig 2022-03-19 6:28 ` [PATCH v6 2/6] x86/mce: relocate set{clear}_mce_nospec() functions Jane Chu 2022-03-19 6:28 ` [dm-devel] " Jane Chu 2022-03-19 8:13 ` kernel test robot 2022-03-19 8:13 ` [dm-devel] " kernel test robot 2022-03-19 8:24 ` kernel test robot 2022-03-19 8:24 ` [dm-devel] " kernel test robot 2022-03-22 8:42 ` Christoph Hellwig 2022-03-22 8:42 ` [dm-devel] " Christoph Hellwig 2022-03-22 22:19 ` Jane Chu 2022-03-22 22:19 ` [dm-devel] " Jane Chu 2022-03-22 22:41 ` Borislav Petkov 2022-03-22 22:41 ` [dm-devel] " Borislav Petkov 2022-03-22 23:48 ` Jane Chu 2022-03-22 23:48 ` [dm-devel] " Jane Chu 2022-03-19 6:28 ` [PATCH v6 3/6] mce: fix set_mce_nospec to always unmap the whole page Jane Chu 2022-03-19 6:28 ` [dm-devel] " Jane Chu 2022-03-22 8:44 ` Christoph Hellwig 2022-03-22 8:44 ` [dm-devel] " Christoph Hellwig 2022-03-22 22:45 ` Jane Chu 2022-03-22 22:45 ` [dm-devel] " Jane Chu 2022-03-19 6:28 ` [PATCH v6 4/6] dax: add DAX_RECOVERY flag and .recovery_write dev_pgmap_ops Jane Chu 2022-03-19 6:28 ` [dm-devel] " Jane Chu 2022-03-19 8:24 ` kernel test robot 2022-03-19 8:24 ` [dm-devel] " kernel test robot 2022-03-19 8:44 ` kernel test robot 2022-03-19 8:44 ` [dm-devel] " kernel test robot 2022-03-22 9:01 ` Christoph Hellwig 2022-03-22 9:01 ` [dm-devel] " Christoph Hellwig 2022-03-22 23:05 ` Jane Chu 2022-03-22 23:05 ` [dm-devel] " Jane Chu 2022-03-23 5:45 ` Christoph Hellwig [this message] 2022-03-23 5:45 ` Christoph Hellwig 2022-03-23 18:43 ` Jane Chu 2022-03-23 18:43 ` [dm-devel] " Jane Chu 2022-03-24 6:37 ` Christoph Hellwig 2022-03-24 6:37 ` [dm-devel] " Christoph Hellwig 2022-03-26 6:31 ` Jane Chu 2022-03-26 6:31 ` [dm-devel] " Jane Chu 2022-03-19 6:28 ` [PATCH v6 5/6] pmem: refactor pmem_clear_poison() Jane Chu 2022-03-19 6:28 ` [dm-devel] " Jane Chu 2022-03-22 8:53 ` Christoph Hellwig 2022-03-22 8:53 ` [dm-devel] " Christoph Hellwig 2022-03-22 23:45 ` Jane Chu 2022-03-22 23:45 ` [dm-devel] " Jane Chu 2022-03-19 6:28 ` [PATCH v6 6/6] pmem: implement pmem_recovery_write() Jane Chu 2022-03-19 6:28 ` [dm-devel] " Jane Chu
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=Yjq0FspfsLrN/mrx@infradead.org \ --to=hch@infradead.org \ --cc=agk@redhat.com \ --cc=dan.j.williams@intel.com \ --cc=dave.jiang@intel.com \ --cc=david@fromorbit.com \ --cc=djwong@kernel.org \ --cc=dm-devel@redhat.com \ --cc=ira.weiny@intel.com \ --cc=jane.chu@oracle.com \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-xfs@vger.kernel.org \ --cc=nvdimm@lists.linux.dev \ --cc=snitzer@redhat.com \ --cc=vgoyal@redhat.com \ --cc=vishal.l.verma@intel.com \ --cc=willy@infradead.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: linkBe 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.