From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 30 Nov 2000 23:17:16 +0100 From: Andrea Arcangeli Message-ID: <20001130231716.H18804@athlon.random> References: <20001130214121.D18804@athlon.random> <200011302154.eAULsJZ02315@webber.adilger.net> Mime-Version: 1.0 Content-Disposition: inline In-Reply-To: <200011302154.eAULsJZ02315@webber.adilger.net>; from adilger@turbolinux.com on Thu, Nov 30, 2000 at 02:54:19PM -0700 Subject: [linux-lvm] Re: [PATCH] Re: [bug] infinite loop in generic_make_request() Sender: linux-lvm-admin@sistina.com Errors-To: linux-lvm-admin@sistina.com Reply-To: linux-lvm@sistina.com List-Help: List-Post: List-Subscribe: , List-Unsubscribe: , List-Archive: List-Id: Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: Andreas Dilger Cc: Neil Brown , Linus Torvalds , V Ganesh , linux-kernel@vger.kernel.org, linux-LVM@sistina.com, mingo@redhat.com On Thu, Nov 30, 2000 at 02:54:19PM -0700, Andreas Dilger wrote: > Andrea writes: > > On Thu, Nov 30, 2000 at 01:05:53PM -0700, Andreas Dilger wrote: > > > the RAID and LVM make_request functions should be changed to do that > > > instead (i.e. 0 on success, -ve on error, and maybe "1" if they do their > > > own recursion to break the loop)? > > > > We preferred to let the lowlevel drivers to handle error themselfs to > > simplify the interface. The lowlevel driver needs to call buffer_IO_error > > before returning in case of error. > > Even if the lowlevel driver handles the error case, it would still > make more sense to stick with the normal kernel practise of -ERROR, > and 0 for success. Then, if in the future we can do something with the > error codes, at least we don't have to change the interface yet again. You shouldn't see the fact that a storage management driver returns 0 as an error. It has a different semantic, it only means: "the make_request callback completed the request, it wasn't a remap". That's all. As said the highlevel layer doesn't know anything about errors anymore, it only need to know when the request is completed. Of course if there's an error during a remap you can't continue so you have to say "this request is completed" and to tell this you currently have to return 0. But 0 from the point of view of the highlevel layer doesn't mean "error". I'd suggest to take a third way that is to define only two retvals: BLKDEV_IO_REQ_COMPLETED BLKDEV_IO_REQ_REMAPPED Then it doesn't matter anymore what number they're defined to. generic_make_request simply keeps looping as far as it gets BLKDEV_IO_REQ_REMAPPED as retval. Andrea