Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752161AbcDWSIn (ORCPT ); Sat, 23 Apr 2016 14:08:43 -0400 Received: from mga04.intel.com ([192.55.52.120]:18194 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752000AbcDWSIk (ORCPT ); Sat, 23 Apr 2016 14:08:40 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,523,1455004800"; d="scan'208";a="938742937" From: "Verma, Vishal L" To: "hch@infradead.org" , "jmoyer@redhat.com" CC: "Wilcox, Matthew R" , "linux-kernel@vger.kernel.org" , "linux-block@vger.kernel.org" , "xfs@oss.sgi.com" , "linux-nvdimm@ml01.01.org" , "linux-mm@kvack.org" , "viro@zeniv.linux.org.uk" , "axboe@fb.com" , "akpm@linux-foundation.org" , "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+T1f2AgASHRwA= Date: Sat, 23 Apr 2016 18:08:37 +0000 Message-ID: <1461434916.3695.7.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> <20160420205923.GA24797@infradead.org> In-Reply-To: <20160420205923.GA24797@infradead.org> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.254.113.52] Content-Type: text/plain; charset="utf-8" Content-ID: <627BD94691203943AE9F3F8DCFC1876B@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 u3NI8msi022674 Content-Length: 1422 Lines: 45 On Wed, 2016-04-20 at 13:59 -0700, Christoph Hellwig wrote: > On Fri, Apr 15, 2016 at 12:11:36PM -0400, Jeff Moyer wrote: > > > > > > > > + if (IS_DAX(inode)) { > > > + ret = dax_do_io(iocb, inode, iter, offset, > > > blkdev_get_block, > > >   NULL, DIO_SKIP_DIO_COUNT); > > > + 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. > Yes, there is no point in saving the earlier error - just return the > second error all the time. Is it ok to do that? direct_IO might fail with -EINVAL due to misalignment, or -ENOMEM due to some allocation failing, and I thought we should return the original -EIO in such cases so that the application doesn't lose the information that the bad block is actually causing the error. > > E.g. > > ret = dax_io(); > if (dax_need_dio_retry(ret)) > ret = direct_IO(); >