From: Vivek Goyal <vgoyal@redhat.com> To: Christoph Hellwig <hch@infradead.org> Cc: linux-fsdevel@vger.kernel.org, linux-nvdimm@lists.01.org, dm-devel@redhat.com Subject: Re: [PATCH v3 3/7] dax, pmem: Add a dax operation zero_page_range Date: Mon, 17 Feb 2020 13:08:07 -0500 [thread overview] Message-ID: <20200217180807.GC24816@redhat.com> (raw) In-Reply-To: <20200217132607.GD14490@infradead.org> On Mon, Feb 17, 2020 at 05:26:07AM -0800, Christoph Hellwig wrote: > > + int rc; > > + struct pmem_device *pmem = dax_get_private(dax_dev); > > + struct page *page = ZERO_PAGE(0); > > Nit: I tend to find code easier to read if variable declarations > with assignments are above those without. Fixed in V4. > > Also I don't think we need the page variable here. Fixed in V4. > > > + rc = pmem_do_write(pmem, page, 0, offset, len); > > + if (rc > 0) > > + return -EIO; > > pmem_do_write returns a blk_status_t, so the type of rc and the > check > seem odd. But I think pmem_do_write (and pmem_do_read) might be better > off returning a normal errno anyway. Now I am using blk_status_to_errno() to convert error in V4. rc = pmem_do_write(pmem, ZERO_PAGE(0), 0, offset, len); return blk_status_to_errno(rc); Did not modify pmem_do_read()/pmem_do_write() to return errno as there is still one caller which expects to return blk_status_t and then that caller will have to do the converstion. Having said that, it probably is good idea to clean up functions called by pmem_do_read()/pmem_do_write() to return errno. I prefer not to take that work in that patch series as that seems like a nice to have thing and can be handled in a separate patch series. Thanks Vivek _______________________________________________ Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org To unsubscribe send an email to linux-nvdimm-leave@lists.01.org
WARNING: multiple messages have this Message-ID (diff)
From: Vivek Goyal <vgoyal@redhat.com> To: Christoph Hellwig <hch@infradead.org> Cc: linux-fsdevel@vger.kernel.org, linux-nvdimm@lists.01.org, dan.j.williams@intel.com, dm-devel@redhat.com, vishal.l.verma@intel.com Subject: Re: [PATCH v3 3/7] dax, pmem: Add a dax operation zero_page_range Date: Mon, 17 Feb 2020 13:08:07 -0500 [thread overview] Message-ID: <20200217180807.GC24816@redhat.com> (raw) In-Reply-To: <20200217132607.GD14490@infradead.org> On Mon, Feb 17, 2020 at 05:26:07AM -0800, Christoph Hellwig wrote: > > + int rc; > > + struct pmem_device *pmem = dax_get_private(dax_dev); > > + struct page *page = ZERO_PAGE(0); > > Nit: I tend to find code easier to read if variable declarations > with assignments are above those without. Fixed in V4. > > Also I don't think we need the page variable here. Fixed in V4. > > > + rc = pmem_do_write(pmem, page, 0, offset, len); > > + if (rc > 0) > > + return -EIO; > > pmem_do_write returns a blk_status_t, so the type of rc and the > check > seem odd. But I think pmem_do_write (and pmem_do_read) might be better > off returning a normal errno anyway. Now I am using blk_status_to_errno() to convert error in V4. rc = pmem_do_write(pmem, ZERO_PAGE(0), 0, offset, len); return blk_status_to_errno(rc); Did not modify pmem_do_read()/pmem_do_write() to return errno as there is still one caller which expects to return blk_status_t and then that caller will have to do the converstion. Having said that, it probably is good idea to clean up functions called by pmem_do_read()/pmem_do_write() to return errno. I prefer not to take that work in that patch series as that seems like a nice to have thing and can be handled in a separate patch series. Thanks Vivek
next prev parent reply other threads:[~2020-02-17 18:08 UTC|newest] Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-02-07 20:26 [PATCH v3 0/7] dax,pmem: Provide a dax operation to zero range of memory Vivek Goyal 2020-02-07 20:26 ` Vivek Goyal 2020-02-07 20:26 ` [PATCH v3 1/7] pmem: Add functions for reading/writing page to/from pmem Vivek Goyal 2020-02-07 20:26 ` Vivek Goyal 2020-02-17 13:21 ` Christoph Hellwig 2020-02-17 13:21 ` Christoph Hellwig 2020-02-17 18:04 ` Vivek Goyal 2020-02-17 18:04 ` Vivek Goyal 2020-02-07 20:26 ` [PATCH v3 2/7] pmem: Enable pmem_do_write() to deal with arbitrary ranges Vivek Goyal 2020-02-07 20:26 ` Vivek Goyal 2020-02-17 13:23 ` Christoph Hellwig 2020-02-17 13:23 ` Christoph Hellwig 2020-02-17 14:59 ` Vivek Goyal 2020-02-17 14:59 ` Vivek Goyal 2020-02-07 20:26 ` [PATCH v3 3/7] dax, pmem: Add a dax operation zero_page_range Vivek Goyal 2020-02-07 20:26 ` Vivek Goyal 2020-02-17 13:26 ` Christoph Hellwig 2020-02-17 13:26 ` Christoph Hellwig 2020-02-17 18:08 ` Vivek Goyal [this message] 2020-02-17 18:08 ` Vivek Goyal 2020-02-07 20:26 ` [PATCH v3 4/7] s390,dcssblk,dax: Add dax zero_page_range operation to dcssblk driver Vivek Goyal 2020-02-07 20:26 ` Vivek Goyal 2020-02-10 20:53 ` Gerald Schaefer 2020-02-10 20:53 ` Gerald Schaefer 2020-02-11 15:11 ` Vivek Goyal 2020-02-11 15:11 ` Vivek Goyal 2020-02-11 15:49 ` Gerald Schaefer 2020-02-11 15:49 ` Gerald Schaefer 2020-02-07 20:26 ` [PATCH v3 5/7] dm,dax: Add dax zero_page_range operation Vivek Goyal 2020-02-07 20:26 ` Vivek Goyal 2020-02-07 20:26 ` [PATCH v3 6/7] dax,iomap: Start using dax native zero_page_range() Vivek Goyal 2020-02-07 20:26 ` Vivek Goyal 2020-02-17 13:26 ` Christoph Hellwig 2020-02-17 13:26 ` Christoph Hellwig 2020-02-07 20:26 ` [PATCH v3 7/7] dax,iomap: Add helper dax_iomap_zero() to zero a range Vivek Goyal 2020-02-07 20:26 ` Vivek Goyal 2020-02-17 13:27 ` Christoph Hellwig 2020-02-17 13:27 ` Christoph Hellwig 2020-02-14 12:57 ` [PATCH v3 0/7] dax,pmem: Provide a dax operation to zero range of memory Vivek Goyal 2020-02-14 12:57 ` Vivek Goyal
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=20200217180807.GC24816@redhat.com \ --to=vgoyal@redhat.com \ --cc=dm-devel@redhat.com \ --cc=hch@infradead.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-nvdimm@lists.01.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.