From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1761148Ab0HFLYI (ORCPT ); Fri, 6 Aug 2010 07:24:08 -0400 Received: from 0122700014.0.fullrate.dk ([95.166.99.235]:36490 "EHLO kernel.dk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752267Ab0HFLYH (ORCPT ); Fri, 6 Aug 2010 07:24:07 -0400 Message-ID: <4C5BF0D5.1080505@kernel.dk> Date: Fri, 06 Aug 2010 13:24:05 +0200 From: Jens Axboe MIME-Version: 1.0 To: Dmitry Monakhov CC: linux-kernel@vger.kernel.org Subject: Re: Resend: [PATCH] blkdev: fix blkdev_issue_zeroout return value References: <87vd7o2f9z.fsf@dmon-lap.sw.ru> <4C5BEA4E.4070107@kernel.dk> <87mxt02dr5.fsf@dmon-lap.sw.ru> In-Reply-To: <87mxt02dr5.fsf@dmon-lap.sw.ru> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2010-08-06 13:15, Dmitry Monakhov wrote: > Jens Axboe writes: > >> On 2010-08-06 12:42, Dmitry Monakhov wrote: >>> Hi Jens, >>> Seems that my first mail was missed somewhere. >>> I've found couple of trivial issues in blkdev_issue_zeroout() >>> implementation. Unfortunately I've miss during initial testing phase >>> because always called it with BARRIER|WAIT flags. >> >> BTW, this: >> >> @@ -218,15 +222,18 @@ submit: >> /* One of bios in the batch was completed with error.*/ >> ret = -EIO; >> >> - if (ret) >> + if (ret && ret != -ENOMEM) >> goto out; >> >> if (test_bit(BIO_EOPNOTSUPP, &bb.flags)) { >> ret = -EOPNOTSUPP; >> goto out; >> } >> - if (nr_sects != 0) >> + if (nr_sects != 0) { >> + if (ret == -ENOMEM) >> + io_schedule(); >> goto submit; >> + } >> out: >> return ret; >> } >> >> is broken. Either the caller sets __GFP_WAIT and then bio_alloc() will >> not fail, or GFP_ATOMIC is used knowing that the call can fail and >> return ENOMEM. Don't code in retry logic like this. > Ok, my fault and in fact i've done in explicitly. I just thought > that blk-layer is some times an exception from general GFP_ATOMIC rule > because in some places in blk-layer we stick to GFP_NOFAIL semantics > regardless to actual gfp flags. > > New version attached. Thanks that looks better, now really added. -- Jens Axboe