From: "Darrick J. Wong" <djwong@kernel.org> To: Pavel Begunkov <asml.silence@gmail.com> Cc: Dave Chinner <david@fromorbit.com>, Christoph Hellwig <hch@infradead.org>, Jane Chu <jane.chu@oracle.com>, "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: [dm-devel] [PATCH 0/6] dax poison recovery with RWF_RECOVERY_DATA flag Date: Fri, 29 Oct 2021 09:57:47 -0700 [thread overview] Message-ID: <20211029165747.GC2237511@magnolia> (raw) In-Reply-To: <22255117-52de-4b2d-822e-b4bc50bbc52b@gmail.com> On Fri, Oct 29, 2021 at 12:46:14PM +0100, Pavel Begunkov wrote: > On 10/28/21 23:59, Dave Chinner wrote: > [...] > > > > Well, my point is doing recovery from bit errors is by definition not > > > > the fast path. Which is why I'd rather keep it away from the pmem > > > > read/write fast path, which also happens to be the (much more important) > > > > non-pmem read/write path. > > > > > > The trouble is, we really /do/ want to be able to (re)write the failed > > > area, and we probably want to try to read whatever we can. Those are > > > reads and writes, not {pre,f}allocation activities. This is where Dave > > > and I arrived at a month ago. > > > > > > Unless you'd be ok with a second IO path for recovery where we're > > > allowed to be slow? That would probably have the same user interface > > > flag, just a different path into the pmem driver. > > > > I just don't see how 4 single line branches to propage RWF_RECOVERY > > down to the hardware is in any way an imposition on the fast path. > > It's no different for passing RWF_HIPRI down to the hardware *in the > > fast path* so that the IO runs the hardware in polling mode because > > it's faster for some hardware. > > Not particularly about this flag, but it is expensive. Surely looks > cheap when it's just one feature, but there are dozens of them with > limited applicability, default config kernels are already sluggish > when it comes to really fast devices and it's not getting better. > Also, pretty often every of them will add a bunch of extra checks > to fix something of whatever it would be. So we can't have data recovery because moving fast the only goal? That's so meta. --D > So let's add a bit of pragmatism to the picture, if there is just one > user of a feature but it adds overhead for millions of machines that > won't ever use it, it's expensive. > > This one doesn't spill yet into paths I care about, but in general > it'd be great if we start thinking more about such stuff instead of > throwing yet another if into the path, e.g. by shifting the overhead > from linear to a constant for cases that don't use it, for instance > with callbacks or bit masks. > > > IOWs, saying that we shouldn't implement RWF_RECOVERY because it > > adds a handful of branches the fast path is like saying that we > > shouldn't implement RWF_HIPRI because it slows down the fast path > > for non-polled IO.... > > > > Just factor the actual recovery operations out into a separate > > function like: > > -- > Pavel Begunkov
WARNING: multiple messages have this Message-ID (diff)
From: "Darrick J. Wong" <djwong@kernel.org> To: Pavel Begunkov <asml.silence@gmail.com> Cc: Jane Chu <jane.chu@oracle.com>, "nvdimm@lists.linux.dev" <nvdimm@lists.linux.dev>, "dave.jiang@intel.com" <dave.jiang@intel.com>, "snitzer@redhat.com" <snitzer@redhat.com>, "vishal.l.verma@intel.com" <vishal.l.verma@intel.com>, Dave Chinner <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>, "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 0/6] dax poison recovery with RWF_RECOVERY_DATA flag Date: Fri, 29 Oct 2021 09:57:47 -0700 [thread overview] Message-ID: <20211029165747.GC2237511@magnolia> (raw) In-Reply-To: <22255117-52de-4b2d-822e-b4bc50bbc52b@gmail.com> On Fri, Oct 29, 2021 at 12:46:14PM +0100, Pavel Begunkov wrote: > On 10/28/21 23:59, Dave Chinner wrote: > [...] > > > > Well, my point is doing recovery from bit errors is by definition not > > > > the fast path. Which is why I'd rather keep it away from the pmem > > > > read/write fast path, which also happens to be the (much more important) > > > > non-pmem read/write path. > > > > > > The trouble is, we really /do/ want to be able to (re)write the failed > > > area, and we probably want to try to read whatever we can. Those are > > > reads and writes, not {pre,f}allocation activities. This is where Dave > > > and I arrived at a month ago. > > > > > > Unless you'd be ok with a second IO path for recovery where we're > > > allowed to be slow? That would probably have the same user interface > > > flag, just a different path into the pmem driver. > > > > I just don't see how 4 single line branches to propage RWF_RECOVERY > > down to the hardware is in any way an imposition on the fast path. > > It's no different for passing RWF_HIPRI down to the hardware *in the > > fast path* so that the IO runs the hardware in polling mode because > > it's faster for some hardware. > > Not particularly about this flag, but it is expensive. Surely looks > cheap when it's just one feature, but there are dozens of them with > limited applicability, default config kernels are already sluggish > when it comes to really fast devices and it's not getting better. > Also, pretty often every of them will add a bunch of extra checks > to fix something of whatever it would be. So we can't have data recovery because moving fast the only goal? That's so meta. --D > So let's add a bit of pragmatism to the picture, if there is just one > user of a feature but it adds overhead for millions of machines that > won't ever use it, it's expensive. > > This one doesn't spill yet into paths I care about, but in general > it'd be great if we start thinking more about such stuff instead of > throwing yet another if into the path, e.g. by shifting the overhead > from linear to a constant for cases that don't use it, for instance > with callbacks or bit masks. > > > IOWs, saying that we shouldn't implement RWF_RECOVERY because it > > adds a handful of branches the fast path is like saying that we > > shouldn't implement RWF_HIPRI because it slows down the fast path > > for non-polled IO.... > > > > Just factor the actual recovery operations out into a separate > > function like: > > -- > Pavel Begunkov -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
next prev parent reply other threads:[~2021-10-29 16:57 UTC|newest] Thread overview: 129+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-10-21 0:10 [PATCH 0/6] dax poison recovery with RWF_RECOVERY_DATA flag Jane Chu 2021-10-21 0:10 ` [dm-devel] " Jane Chu 2021-10-21 0:10 ` [PATCH 1/6] dax: introduce RWF_RECOVERY_DATA flag to preadv2() and pwritev2() Jane Chu 2021-10-21 0:10 ` [dm-devel] " Jane Chu 2021-10-21 0:10 ` [PATCH 2/6] dax: prepare dax_direct_access() API with DAXDEV_F_RECOVERY flag Jane Chu 2021-10-21 0:10 ` [dm-devel] " Jane Chu 2021-10-21 11:20 ` Christoph Hellwig 2021-10-21 11:20 ` [dm-devel] " Christoph Hellwig 2021-10-21 18:19 ` Jane Chu 2021-10-21 18:19 ` [dm-devel] " Jane Chu 2021-10-21 0:10 ` [PATCH 3/6] pmem: pmem_dax_direct_access() to honor the " Jane Chu 2021-10-21 0:10 ` [dm-devel] " Jane Chu 2021-10-21 11:23 ` Christoph Hellwig 2021-10-21 11:23 ` [dm-devel] " Christoph Hellwig 2021-10-21 18:24 ` Jane Chu 2021-10-21 18:24 ` [dm-devel] " Jane Chu 2021-10-21 0:10 ` [PATCH 4/6] dm,dax,pmem: prepare dax_copy_to/from_iter() APIs with DAXDEV_F_RECOVERY Jane Chu 2021-10-21 0:10 ` [dm-devel] [PATCH 4/6] dm, dax, pmem: " Jane Chu 2021-10-21 11:27 ` [PATCH 4/6] dm,dax,pmem: " Christoph Hellwig 2021-10-21 11:27 ` [dm-devel] [PATCH 4/6] dm, dax, pmem: " Christoph Hellwig 2021-10-22 0:49 ` [PATCH 4/6] dm,dax,pmem: " Jane Chu 2021-10-22 0:49 ` [dm-devel] [PATCH 4/6] dm, dax, pmem: " Jane Chu 2021-10-22 1:41 ` correction: Re: [PATCH 4/6] dm,dax,pmem: " Jane Chu 2021-10-22 1:41 ` [dm-devel] correction: Re: [PATCH 4/6] dm, dax, pmem: " Jane Chu 2021-10-22 5:33 ` [PATCH 4/6] dm,dax,pmem: " Christoph Hellwig 2021-10-22 5:33 ` [dm-devel] [PATCH 4/6] dm, dax, pmem: " Christoph Hellwig 2021-10-22 20:30 ` [PATCH 4/6] dm,dax,pmem: " Jane Chu 2021-10-22 20:30 ` [dm-devel] [PATCH 4/6] dm, dax, pmem: " Jane Chu 2021-10-21 0:10 ` [PATCH 5/6] dax,pmem: Add data recovery feature to pmem_copy_to/from_iter() Jane Chu 2021-10-21 0:10 ` [dm-devel] [PATCH 5/6] dax, pmem: " Jane Chu 2021-10-21 11:28 ` [PATCH 5/6] dax,pmem: " Christoph Hellwig 2021-10-21 11:28 ` [dm-devel] [PATCH 5/6] dax, pmem: " Christoph Hellwig 2021-10-22 0:58 ` [PATCH 5/6] dax,pmem: " Jane Chu 2021-10-22 0:58 ` [dm-devel] [PATCH 5/6] dax, pmem: " Jane Chu 2021-10-22 8:03 ` kernel test robot 2021-10-22 8:03 ` kernel test robot 2021-10-26 10:21 ` [PATCH 5/6] dax,pmem: " kernel test robot 2021-10-26 10:21 ` [PATCH 5/6] dax, pmem: " kernel test robot 2021-10-26 10:21 ` [dm-devel] " kernel test robot 2021-10-21 0:10 ` [PATCH 6/6] dm: Ensure dm honors DAXDEV_F_RECOVERY flag on dax only Jane Chu 2021-10-21 0:10 ` [dm-devel] " Jane Chu 2021-10-21 11:31 ` [dm-devel] [PATCH 0/6] dax poison recovery with RWF_RECOVERY_DATA flag Christoph Hellwig 2021-10-21 11:31 ` Christoph Hellwig 2021-10-22 1:37 ` Jane Chu 2021-10-22 1:37 ` Jane Chu 2021-10-22 1:58 ` Darrick J. Wong 2021-10-22 1:58 ` Darrick J. Wong 2021-10-22 5:38 ` Christoph Hellwig 2021-10-22 5:38 ` Christoph Hellwig 2021-10-22 5:36 ` Christoph Hellwig 2021-10-22 5:36 ` Christoph Hellwig 2021-10-22 20:52 ` Jane Chu 2021-10-22 20:52 ` Jane Chu 2021-10-27 6:49 ` Christoph Hellwig 2021-10-27 6:49 ` Christoph Hellwig 2021-10-28 0:24 ` Darrick J. Wong 2021-10-28 0:24 ` Darrick J. Wong 2021-10-28 22:59 ` Dave Chinner 2021-10-28 22:59 ` Dave Chinner 2021-10-29 11:46 ` Pavel Begunkov 2021-10-29 11:46 ` Pavel Begunkov 2021-10-29 16:57 ` Darrick J. Wong [this message] 2021-10-29 16:57 ` Darrick J. Wong 2021-10-29 19:23 ` Pavel Begunkov 2021-10-29 19:23 ` Pavel Begunkov 2021-10-29 20:08 ` Darrick J. Wong 2021-10-29 20:08 ` Darrick J. Wong 2021-10-31 13:27 ` Pavel Begunkov 2021-10-31 13:27 ` Pavel Begunkov 2021-10-29 18:53 ` Jane Chu 2021-10-29 18:53 ` Jane Chu 2021-10-29 22:32 ` Dave Chinner 2021-10-29 22:32 ` Dave Chinner 2021-10-31 13:19 ` Pavel Begunkov 2021-10-31 13:19 ` Pavel Begunkov 2021-11-01 2:31 ` Matthew Wilcox 2021-11-01 2:31 ` Matthew Wilcox 2021-11-02 6:18 ` Christoph Hellwig 2021-11-02 6:18 ` Christoph Hellwig 2021-11-02 19:57 ` Dan Williams 2021-11-02 19:57 ` Dan Williams 2021-11-03 16:58 ` Christoph Hellwig 2021-11-03 16:58 ` Christoph Hellwig 2021-11-03 20:33 ` Dan Williams 2021-11-03 20:33 ` Dan Williams 2021-11-04 8:30 ` Christoph Hellwig 2021-11-04 8:30 ` Christoph Hellwig 2021-11-04 12:29 ` Matthew Wilcox 2021-11-04 12:29 ` Matthew Wilcox 2021-11-04 16:24 ` Dan Williams 2021-11-04 16:24 ` Dan Williams 2021-11-04 17:43 ` Christoph Hellwig 2021-11-04 17:43 ` Christoph Hellwig 2021-11-04 17:50 ` Dan Williams 2021-11-04 17:50 ` Dan Williams 2021-11-04 18:05 ` Matthew Wilcox 2021-11-04 18:05 ` Matthew Wilcox 2021-11-04 18:33 ` Jane Chu 2021-11-04 18:33 ` Jane Chu 2021-11-04 19:00 ` Dan Williams 2021-11-04 19:00 ` Dan Williams 2021-11-04 20:27 ` Jane Chu 2021-11-04 20:27 ` Jane Chu 2021-11-05 0:46 ` Dan Williams 2021-11-05 0:46 ` Dan Williams 2021-11-05 1:35 ` Dan Williams 2021-11-05 1:35 ` Dan Williams 2021-11-05 5:56 ` Christoph Hellwig 2021-11-05 5:56 ` Christoph Hellwig 2021-11-03 18:09 ` Jane Chu 2021-11-03 18:09 ` Jane Chu 2021-11-04 6:21 ` Dan Williams 2021-11-04 6:21 ` Dan Williams 2021-11-04 8:36 ` Christoph Hellwig 2021-11-04 8:36 ` Christoph Hellwig 2021-11-04 16:08 ` Dan Williams 2021-11-04 16:08 ` Dan Williams 2021-11-04 17:46 ` Christoph Hellwig 2021-11-04 17:46 ` Christoph Hellwig 2021-11-04 8:21 ` Christoph Hellwig 2021-11-04 8:21 ` Christoph Hellwig 2021-11-02 16:12 ` Dan Williams 2021-11-02 16:12 ` Dan Williams 2021-11-02 16:03 ` Dan Williams 2021-11-02 16:03 ` Dan Williams 2021-11-03 16:53 ` Christoph Hellwig 2021-11-03 16:53 ` Christoph Hellwig 2021-11-06 7:41 ` Lukas Straub 2021-11-06 7:41 ` Lukas Straub
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=20211029165747.GC2237511@magnolia \ --to=djwong@kernel.org \ --cc=agk@redhat.com \ --cc=asml.silence@gmail.com \ --cc=dan.j.williams@intel.com \ --cc=dave.jiang@intel.com \ --cc=david@fromorbit.com \ --cc=dm-devel@redhat.com \ --cc=hch@infradead.org \ --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.