nvdimm.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
From: Jane Chu <jane.chu@oracle.com>
To: Christoph Hellwig <hch@infradead.org>
Cc: "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 5/6] dax,pmem: Add data recovery feature to pmem_copy_to/from_iter()
Date: Fri, 22 Oct 2021 00:58:04 +0000	[thread overview]
Message-ID: <7dc2445e-4cf0-307c-8ab6-48b886bcf34b@oracle.com> (raw)
In-Reply-To: <YXFO8uCgIDoIqTgC@infradead.org>

On 10/21/2021 4:28 AM, Christoph Hellwig wrote:
>> +	if (flags & DAXDEV_F_RECOVERY) {
>> +		lead_off = (unsigned long)addr & ~PAGE_MASK;
>> +		len = PFN_PHYS(PFN_UP(lead_off + bytes));
>> +		if (is_bad_pmem(&pmem->bb, PFN_PHYS(pgoff) / 512, len)) {
>> +			if (lead_off || !(PAGE_ALIGNED(bytes))) {
>> +				dev_warn(dev, "Found poison, but addr(%p) and/or bytes(%#lx) not page aligned\n",
>> +					addr, bytes);
>> +				return (size_t) -EIO;
>> +			}
>> +			pmem_off = PFN_PHYS(pgoff) + pmem->data_offset;
>> +			if (pmem_clear_poison(pmem, pmem_off, bytes) !=
>> +					BLK_STS_OK)
>> +				return (size_t) -EIO;
>> +		}
> 
> Shouldn't this just go down in a separe ->clear_poison operation
> to make the whole thing a little easier to follow?
> 

Do you mean to lift or refactor the above to a helper function so as
to improve the readability of the code?  I can do that, just to confirm.
On the same note, would you prefer to refactor the read path as well?

thanks!
-jane


  reply	other threads:[~2021-10-22  0:58 UTC|newest]

Thread overview: 62+ 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 ` [PATCH 1/6] dax: introduce RWF_RECOVERY_DATA flag to preadv2() and pwritev2() 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 11:20   ` Christoph Hellwig
2021-10-21 18:19     ` Jane Chu
2021-10-21  0:10 ` [PATCH 3/6] pmem: pmem_dax_direct_access() to honor the " Jane Chu
2021-10-21 11:23   ` Christoph Hellwig
2021-10-21 18:24     ` 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 11:27   ` Christoph Hellwig
2021-10-22  0:49     ` Jane Chu
2021-10-22  1:41       ` correction: " Jane Chu
2021-10-22  5:33       ` Christoph Hellwig
2021-10-22 20:30         ` 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 11:28   ` Christoph Hellwig
2021-10-22  0:58     ` Jane Chu [this message]
2021-10-21  0:10 ` [PATCH 6/6] dm: Ensure dm honors DAXDEV_F_RECOVERY flag on dax only Jane Chu
2021-10-21 11:31 ` [dm-devel] [PATCH 0/6] dax poison recovery with RWF_RECOVERY_DATA flag Christoph Hellwig
2021-10-22  1:37   ` Jane Chu
2021-10-22  1:58     ` Darrick J. Wong
2021-10-22  5:38       ` Christoph Hellwig
2021-10-22  5:36     ` Christoph Hellwig
2021-10-22 20:52       ` Jane Chu
2021-10-27  6:49         ` Christoph Hellwig
2021-10-28  0:24           ` Darrick J. Wong
2021-10-28 22:59             ` Dave Chinner
2021-10-29 11:46               ` Pavel Begunkov
2021-10-29 16:57                 ` Darrick J. Wong
2021-10-29 19:23                   ` Pavel Begunkov
2021-10-29 20:08                     ` Darrick J. Wong
2021-10-31 13:27                       ` Pavel Begunkov
2021-10-29 18:53                 ` Jane Chu
2021-10-29 22:32                 ` Dave Chinner
2021-10-31 13:19                   ` Pavel Begunkov
2021-11-01  2:31                     ` Matthew Wilcox
2021-11-02  6:18             ` Christoph Hellwig
2021-11-02 19:57               ` Dan Williams
2021-11-03 16:58                 ` Christoph Hellwig
2021-11-03 20:33                   ` Dan Williams
2021-11-04  8:30                     ` Christoph Hellwig
2021-11-04 12:29                       ` Matthew Wilcox
2021-11-04 16:24                       ` Dan Williams
2021-11-04 17:43                         ` Christoph Hellwig
2021-11-04 17:50                           ` Dan Williams
2021-11-04 18:05                           ` Matthew Wilcox
2021-11-04 18:33                         ` Jane Chu
2021-11-04 19:00                           ` Dan Williams
2021-11-04 20:27                             ` Jane Chu
2021-11-05  0:46                               ` Dan Williams
2021-11-05  1:35                                 ` Dan Williams
2021-11-05  5:56                             ` Christoph Hellwig
2021-11-03 18:09               ` Jane Chu
2021-11-04  6:21                 ` Dan Williams
2021-11-04  8:36                   ` Christoph Hellwig
2021-11-04 16:08                     ` Dan Williams
2021-11-04 17:46                       ` Christoph Hellwig
2021-11-04  8:21                 ` Christoph Hellwig
2021-11-02 16:12             ` Dan Williams
2021-11-02 16:03           ` Dan Williams
2021-11-03 16:53             ` Christoph Hellwig
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=7dc2445e-4cf0-307c-8ab6-48b886bcf34b@oracle.com \
    --to=jane.chu@oracle.com \
    --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=hch@infradead.org \
    --cc=ira.weiny@intel.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: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).