From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1CED1EB64DE for ; Tue, 10 Sep 2024 14:17:19 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id A5CF68D007A; Tue, 10 Sep 2024 10:17:18 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 9E59E8D0002; Tue, 10 Sep 2024 10:17:18 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 886C08D007A; Tue, 10 Sep 2024 10:17:18 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 613E98D0002 for ; Tue, 10 Sep 2024 10:17:18 -0400 (EDT) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id 216C4A0597 for ; Tue, 10 Sep 2024 14:17:18 +0000 (UTC) X-FDA: 82549030956.08.C7074D8 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) by imf22.hostedemail.com (Postfix) with ESMTP id 7A19FC0014 for ; Tue, 10 Sep 2024 14:17:14 +0000 (UTC) Authentication-Results: imf22.hostedemail.com; dkim=pass header.d=infradead.org header.s=bombadil.20210309 header.b=hnUz3AjZ; dmarc=none; spf=none (imf22.hostedemail.com: domain of BATV+33ed81108a7d293f2f12+7688+infradead.org+hch@bombadil.srs.infradead.org has no SPF policy when checking 198.137.202.133) smtp.mailfrom=BATV+33ed81108a7d293f2f12+7688+infradead.org+hch@bombadil.srs.infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1725977808; a=rsa-sha256; cv=none; b=BR9KSZID7x8ezYRzRrTS/pgx+nGwF7g/LsXTPVlKa3AriI1a6W8wIhu0X7/AWx3D6K6wWP 0Vl9ZVEKwkFXTvKDOuLyBiYwadvvBNSHtu2BJc4r4eFnBSRmtu3S1vhaFchJRdnpy9UzF6 R0+NFiD5DZbhL9qfY9g7q+ZFjTZcEgM= ARC-Authentication-Results: i=1; imf22.hostedemail.com; dkim=pass header.d=infradead.org header.s=bombadil.20210309 header.b=hnUz3AjZ; dmarc=none; spf=none (imf22.hostedemail.com: domain of BATV+33ed81108a7d293f2f12+7688+infradead.org+hch@bombadil.srs.infradead.org has no SPF policy when checking 198.137.202.133) smtp.mailfrom=BATV+33ed81108a7d293f2f12+7688+infradead.org+hch@bombadil.srs.infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1725977808; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=DkOrZ1Suf79nRPSASZN/zy8KeU8qA5QdhlPDiSeKVDE=; b=g2RMSPxfxTzO2A7WAiaiRLV2vTi//WfLEKCyqJm9+PNCxmuvuCgTjNID+03Yyv3icCI+dr keEddecasfacwH7b9Eux6yL9xB+k3XqFpypwehWNkjPhJxair7laqPDPsoOc6wEh6NgaQ+ shcABH4Dqu6PSleTuW19R/48HTQKJHo= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=DkOrZ1Suf79nRPSASZN/zy8KeU8qA5QdhlPDiSeKVDE=; b=hnUz3AjZ8oQEKvbkyJ9wzk2EEY nibfCyItU0Jxw+x8oUp8v+zTkmkSv+FSCms/YGS4Oxb3TmrbX6G1O2AD2JDs6G5dq0zle2ObC5N1Q vl16jF51PffGlGNmWN4EkiJgAIAlw5jFpQvnXXFK7P872FNZv1d71/ygdSXDdlR+4eHjj+62l/qL2 6PrNaa6rFOHARsNyAclXgdAAIHfL1Su6JERU4L97IQ1vIPYfH+dWvlATUVfk/tnRAIMJfg+68vywC iX5MYNmNjLcqkqhHB3UlMK69aQC1z4kEpR3c8aZW7eMTGzxpgKENYy9/wtQ3M/ccVnGW+u/RHPc9B +zNySoBA==; Received: from hch by bombadil.infradead.org with local (Exim 4.97.1 #2 (Red Hat Linux)) id 1so1g8-00000005qEM-0NM3; Tue, 10 Sep 2024 14:17:12 +0000 Date: Tue, 10 Sep 2024 07:17:12 -0700 From: Christoph Hellwig To: Pavel Begunkov Cc: Christoph Hellwig , io-uring@vger.kernel.org, Jens Axboe , Conrad Meyer , linux-block@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v4 5/8] block: implement async discard as io_uring cmd Message-ID: References: <7fc0a61ae29190a42e958eddfefd6d44cdf372ad.1725621577.git.asml.silence@gmail.com> <430ca5b3-6ee1-463b-9e4e-5d0b934578cc@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <430ca5b3-6ee1-463b-9e4e-5d0b934578cc@gmail.com> X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org. See http://www.infradead.org/rpr.html X-Rspam-User: X-Rspamd-Queue-Id: 7A19FC0014 X-Rspamd-Server: rspam01 X-Stat-Signature: j89fwcrkuots3ok1bxtn4m3iasasfkd3 X-HE-Tag: 1725977834-768821 X-HE-Meta: U2FsdGVkX18GYTJaRbl+H0bl+O36rUgIvv5Wa0203YIeK30Tirx4yTkrJ84D2K4odT3rHqGyPMZeyTww3/q5DrwrM+dYE1JP6EhPR6fp1sOb1RTmMSpQ65Ok86bv6M/1MiJInkBViq4p730r9tWwHJoNn7imuXdEJ1A8I7qpyIdEM3UwXz38Lc71tuEaVWtTaEujlV3dYditdoQu5zAXt2m+8ZdoH0oT6zHHjjggnx/GCJQ2lsMLiqkI8Fm+YuG5EyaTrK2vJTeZ3edktmdftTniFte7fXM9JYQ3P1bMINjocT3dAL6S7RRqKu5TVhRKPCCW0m4KS+Mpf+VVg/ZfZ2GJ0N/YHqv6/D9R6bLGmKSxktQUIZbQW1fRr0SYrplr3JvUx28GJP2L/k33PolQH/CfFbWMyllLJ/l3SqZdkCYWLQrgQk5SGmwQH2r9GskNq5w77kzzn5lq2JrZ7clktLGMeS6VGd7LAxUE2Ix4ooMaTfUU7vwYlwcV6mUS2o5eBVzzxZ5cDDXj0BsmZgd0g9FiMBl2TF5zNQvlrpgUCCM3sLfrm8cAHBJQJNBU72e8BDn/Xbiaz5F6aGmi2Kmjm9Y2Dkz0GxNOkN9ToHcl05oOd3AgC8J9+UAHNydmEi3pLvar/ptzPSL/yx5gBS/86WQXjViIdFufJVq7YIBjMRV/9XfB4DtvJFdaI55J4+b0gEG9p6YnFQ2WYR2c7eKSzoG9oIBXvGzJG8nsh9gMx4wsfyoSykv3H6JnxAMSMihtNCUYTbpop9wsNf/vxd5oUcI+OksOEMXtRywNw6viUi/SB9jxBkC7oYH2JHbxsX03hkGb6Nw3SwRcj44gGV+ZRlu13l4xWyP3Zn7CHKjmil4uQ0+sbLBCwV4+Si1+/lMCigUnCPO0AhLkwprtiJxJPJgUY1UBbvZ1nTIGAiBCgvdEC3iUMCxLCQ2Noe5mueacUBVvf6PfVhykNhJv48H sR5Lcr79 CsdJBurLaAVXPSlmP11ArZKu8+RmgOf8QYheF144zFED6MDOoII302l+9yjfukBlgKZqrzp9szQ7+5fZFQAwd4PXPKr3QsZzH93W5JMp5FE9HeCkPfKqZAdUDDDCAqWOPAKeG4+P0ckChY7XabE42DAaFyQWem/++1AnXIl4k+JGp9iXilObrV1Bg5ewM4TWojCQ5Y9wod88sqo8O0xryfFoapYLh7xgqzgT8MDMq20pCYre83Nicyn2PfbySXz+1I7QfK1SaYUIZeFRzJcs6CQKbBbuF+zwsJXxPEUGmLckFjt1AGAH1q7JK+v1umkckoPybRJWPAgxBQ4l7mcR5zmeac3VMxRMn4Hiv3fA23NPlUoqoeviagkIvGR/ArKjfDbEQsAolcmDx4zSadr61/UhEJEhCYaTzNYjD8bp8RDwhTNCucORmhqCyoxR2a8ewY77LIjrCY+kiZb/Fv5xJkQ2is2DAi79mYKL3tT0GW3sx+wJC4gr0rg5lCjQMdb0n5Bi3czKpHxdcmr3LJcoyOr02mNJWaq8ynxXDU8FXN7tP+3x34xTWY4AtejOpLAbU5ygd/oeCqnoS+FShviooe0bQWJyEqTIme317 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000037, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Tue, Sep 10, 2024 at 11:58:23AM +0100, Pavel Begunkov wrote: > > Based on the above this function is misnamed, as it validates sector_t > > range and not a byte range. > > Start and len here are in bytes. What do you mean? You are right, sorry. > > > + > > > + err = filemap_invalidate_pages(bdev->bd_mapping, start, > > > + start + len - 1, nowait); > > > + if (err) > > > + return err; > > > + > > > + while ((bio = blk_alloc_discard_bio(bdev, §or, &nr_sects, gfp))) { > > > + if (nowait) > > > + bio->bi_opf |= REQ_NOWAIT; > > > + prev = bio_chain_and_submit(prev, bio); > > > + } > > > + if (!prev) > > > + return -EAGAIN; > > > > If a user changes the max_discard value between the check above and > > the loop here this is racy. > > If the driver randomly changes it, it's racy either way. What do > you want to protect against? The discard limit shrinking and now this successfully returning while not actually discarding the range. The fix is pretty simple in that the nowait case should simply break out of the loop after the first bio. > > > +sector_t bio_discard_limit(struct block_device *bdev, sector_t sector); > > > > And to be honest, I'd really prefer to not have bio_discard_limit > > exposed. Certainly not outside a header private to block/. > > Which is the other reason why first versions were putting down > a bio seeing that there is more to be done for nowait, which > you didn't like. I can return back to it or narrow the scopre. The above should also take care of that. > > > Also why start at 137? A comment > > would generally be pretty useful as well. > > There is a comment, 2 lines above the new define. > > /* > * A jump here: 130-136 are reserved for zoned block devices > * (see uapi/linux/blkzoned.h) > */ > > Is that your concern? But those are ioctls, this is not an ioctl and uses a different number space. Take a look at e.g. nvme uring cmds which also don't try to use the same number space as the ioctl. > > Also can we have a include/uapi/linux/blkdev.h for this instead of > > bloating fs.h that gets included just about everywhere? > I don't think it belongs to this series. How would adding a proper header instead of bloating fs.h not be part of the series adding the first ever block layer uring_cmds? Just in case I wasn't clear - this isn't asking you to move anything existing as we can't do that without breaking existing applications. It is about adding the new command to the proper place.