From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752802AbcDOQyy (ORCPT ); Fri, 15 Apr 2016 12:54:54 -0400 Received: from mga01.intel.com ([192.55.52.88]:45054 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751574AbcDOQyw (ORCPT ); Fri, 15 Apr 2016 12:54:52 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,488,1455004800"; d="scan'208";a="785720859" From: "Verma, Vishal L" To: "jmoyer@redhat.com" CC: "Wilcox, Matthew R" , "linux-kernel@vger.kernel.org" , "linux-block@vger.kernel.org" , "hch@infradead.org" , "linux-nvdimm@ml01.01.org" , "xfs@oss.sgi.com" , "linux-mm@kvack.org" , "viro@zeniv.linux.org.uk" , "akpm@linux-foundation.org" , "axboe@fb.com" , "linux-fsdevel@vger.kernel.org" , "linux-ext4@vger.kernel.org" , "david@fromorbit.com" , "jack@suse.cz" Subject: Re: [PATCH v2 5/5] dax: handle media errors in dax_do_io Thread-Topic: [PATCH v2 5/5] dax: handle media errors in dax_do_io Thread-Index: AQHRlzGOmdVyB28MyEyyQ/rUxU8Y+Z+Ltf8A Date: Fri, 15 Apr 2016 16:54:48 +0000 Message-ID: <1460739288.3012.3.camel@intel.com> References: <1459303190-20072-1-git-send-email-vishal.l.verma@intel.com> <1459303190-20072-6-git-send-email-vishal.l.verma@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.254.176.69] Content-Type: text/plain; charset="utf-8" Content-ID: <16063EC031B1C2489B91594D65C22F50@intel.com> MIME-Version: 1.0 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Transfer-Encoding: 8bit X-MIME-Autoconverted: from base64 to 8bit by mail.home.local id u3FGsxkX003746 On Fri, 2016-04-15 at 12:11 -0400, Jeff Moyer wrote: > Vishal Verma writes: > > > > > dax_do_io (called for read() or write() for a dax file system) may > > fail > > in the presence of bad blocks or media errors. Since we expect that > > a > > write should clear media errors on nvdimms, make dax_do_io fall > > back to > > the direct_IO path, which will send down a bio to the driver, which > > can > > then attempt to clear the error. > [snip] > > > > > + if (IS_DAX(inode)) { > > + ret = dax_do_io(iocb, inode, iter, offset, > > blkdev_get_block, > >   NULL, DIO_SKIP_DIO_COUNT); > > - return __blockdev_direct_IO(iocb, inode, I_BDEV(inode), > > iter, offset, > > + if (ret == -EIO && (iov_iter_rw(iter) == WRITE)) > > + ret_saved = ret; > > + else > > + return ret; > > + } > > + > > + ret = __blockdev_direct_IO(iocb, inode, I_BDEV(inode), > > iter, offset, > >       blkdev_get_block, NULL, NULL, > >       DIO_SKIP_DIO_COUNT); > > + if (ret < 0 && ret_saved) > > + return ret_saved; > > + > Hmm, did you just break async DIO?  I think you did!  :) > __blockdev_direct_IO can return -EIOCBQUEUED, and you've now turned > that > into -EIO.  Really, I don't see a reason to save that first > -EIO.  The > same applies to all instances in this patch. The reason I saved it was if __blockdev_direct_IO fails for some reason, we should return the original cause o the error, which was an EIO.. i.e. we shouldn't be hiding the EIO if the direct_IO fails with something else.. But, how does _EIOCBQUEUED work? Maybe we need an exception for it?  Thanks, -Vishal