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 DCE7DC3DA4A for ; Thu, 22 Aug 2024 06:46:50 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 3D7606B0188; Thu, 22 Aug 2024 02:46:50 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 387D56B018A; Thu, 22 Aug 2024 02:46:50 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 228FA6B0189; Thu, 22 Aug 2024 02:46:50 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0011.hostedemail.com [216.40.44.11]) by kanga.kvack.org (Postfix) with ESMTP id 05E056B02B2 for ; Thu, 22 Aug 2024 02:46:49 -0400 (EDT) Received: from smtpin14.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay09.hostedemail.com (Postfix) with ESMTP id A3ABD812F7 for ; Thu, 22 Aug 2024 06:46:49 +0000 (UTC) X-FDA: 82478948538.14.A326906 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) by imf28.hostedemail.com (Postfix) with ESMTP id 05B77C000C for ; Thu, 22 Aug 2024 06:46:47 +0000 (UTC) Authentication-Results: imf28.hostedemail.com; dkim=pass header.d=infradead.org header.s=bombadil.20210309 header.b=e++10MiQ; dmarc=none; spf=none (imf28.hostedemail.com: domain of BATV+dfc8a69a7fb43479e7c6+7669+infradead.org+hch@bombadil.srs.infradead.org has no SPF policy when checking 198.137.202.133) smtp.mailfrom=BATV+dfc8a69a7fb43479e7c6+7669+infradead.org+hch@bombadil.srs.infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1724309148; a=rsa-sha256; cv=none; b=Flgvmo/Sn3w+kpboqxb/W//cym4oFMt7MlR8Zd6pufylfUeRC4XiZNpu3wl1TxHy8+Rc5Y DclSCQ6Z1peAvg7iHFHSH1BYSpGh0+PuJKEJ/dIXiJnQTWKjqHbHbLFF56PRKjMtPkwXYo g7u9RhakFsjoeVuu0ttgpY/EyekoK9k= ARC-Authentication-Results: i=1; imf28.hostedemail.com; dkim=pass header.d=infradead.org header.s=bombadil.20210309 header.b=e++10MiQ; dmarc=none; spf=none (imf28.hostedemail.com: domain of BATV+dfc8a69a7fb43479e7c6+7669+infradead.org+hch@bombadil.srs.infradead.org has no SPF policy when checking 198.137.202.133) smtp.mailfrom=BATV+dfc8a69a7fb43479e7c6+7669+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=1724309148; 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=lYAyybBx9ag/sgO6l6j7CoDzfNN3dtp7xr87Twy9vgo=; b=L862mFFrdetAi7KN5FHr6XJiwCb6kKs75oF3bmxHrNmyjeof7Z1q23Xco87xMxVVjztMw/ TCYylM/aIQtFXHuEKiuHrZFcaN7O+/gcM7+b31k29fhCaFeMe7REA4Ei++H5RbBSTIn9eg p9EnU+NKFTrFhOHo8lo1FWXcE2IZEyA= 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=lYAyybBx9ag/sgO6l6j7CoDzfNN3dtp7xr87Twy9vgo=; b=e++10MiQQU8L/mwLY9/zR05cAI rj5vgR16euUPRPLFT4nDyaZPb2tKnxn6f7cJGJIVurOsGc/YNGZ7juDf5fcYgjOZy+ZsV8cUi39nY wQZraOTm5gchN84agQlBTP9NgsGVhgTY9cy0BK0oohTjzlesbcJSYAktvieQk7FbLzoY2mt27vSe2 n074yDSXLJC0WknqjmCayiZhbGbudZis9DaOb5dMdXtxjEBWBi18eyDZ3c240Zp1cwczZty2ZCa+W ZbmyYp7tziRx2baAzFAfA7hcBXnizyqTzUFubp2wW85s1bBfUYxz+jRXbqAWiDzoM0btw5bammL8d Fcv2+ZWg==; Received: from hch by bombadil.infradead.org with local (Exim 4.97.1 #2 (Red Hat Linux)) id 1sh1ao-0000000Bg00-0efc; Thu, 22 Aug 2024 06:46:46 +0000 Date: Wed, 21 Aug 2024 23:46:46 -0700 From: Christoph Hellwig To: Pavel Begunkov Cc: io-uring@vger.kernel.org, Jens Axboe , Conrad Meyer , linux-block@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v2 5/7] block: implement async discard as io_uring cmd Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org. See http://www.infradead.org/rpr.html X-Rspamd-Queue-Id: 05B77C000C X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: hs74qc9opjndq8zgzdnh9wes6koxc8s8 X-HE-Tag: 1724309207-352221 X-HE-Meta: U2FsdGVkX19IRlIloC57sLCq3D3vX+9NgRf53762NaQ/hsHZKNL+wPHWZ8rotSznTDhnQT0frr8r+q4vH2N82hYfXYdmTYd73oNqDZzfeLA7d1AU2oI7nreJtuecMlpCuRy+kylA9xC83c4xlYTT5+eXN+MgYmRRTSRIj2tVfAzW715Ww5AWJt2BirTGVKtgVmRif/YJAYRoUctqWNcyKcVABluq8N2k1AWlgwS8ZKGIpufaR3N3AAK7Rau11HB8l6kORCyNLFO+NLUDRwa30U0Mwlhb0ShB/39agU3eLM1rMWa32g21N9BKbWMuqnVgk7V1VsiuSCF/2pDDyooisutnFicBwe9wJ54aBHEUtLdapAgdEVgMiqT9Y4Oj1L9KNjUVErXU61fzclEmraO3L2KmEeRG253wjMTNlWF9gQMXoNbK6UH4KedQs9CEFenc3xb3MFzgm5rvfTR0xqAfEpAYOkIceh9Sq+MQYcqFeLM85am23sy0MgegAEXUGAP1pO9HNSWB53/2330IhpIc8OuKXWUuiu8AiATg+b9hzcYl4AcHgESiBbwWiXR6PyVhtnGxSZC3oV9j/d98tkoEDje59Gqiv0ttzRPVuJoz2f4dGLOQ2dT4U6Wmx1D3VCWZDI0zU7NRpDWMJ1IO/MNlbz/q4B94D1ex/o9lHODGupGV670Tk8nc81p1StofvrymaW9PX3uPL9LsGC/5dQbAkliLlxq35qfvZhDJj5zFpuQOCWd2OowEZ/SSxMybX+nzpDtPidPIOP6I/qDl6xs+93xqMnrygqO/jVAfurDjLwsdV6/fCc79Zxslij/5ScN+593nQMtPtKFujYnnXIdm7zAS23UbyIECN+Y4lUobOrTBm9r0qd0ph38jpa9mBIvQwOfc7onpq9170S3rpHfOqz+rvGBdkvkX6C6noqbawow5e+uoOo2I+rSvQU+kqr/KfH4V2dSxkEAiETw++Gq vHZrAJh1 c4dfJTgunnWLtcNN82mb5v3DM4Yz6pvwb/ljAqqTzN/phOiOeVxJDwp4Qmf5GHlLXiyY6RV+Lctk1ZW+UCEIZ8cxOnai2Arlvb/xmQD02Hgd4WNKD9BtDTIxEiIV1oWVY5MUAc1ZC8KjZoZ8xcB1WV9rwBO/XoHiUiNuTbr/SwSgy5TgsHfgyrU1JLQxSGZUu5tLzdg0+4kovCbw16QVl6YG9yAeiBXGkLhh8q+1iCnHe1cZyZ7QeHaUX1Pjvy3koweB2TK8Lmcvc3c2H+xRFNDBa+1I4W5hK/79UaEh1TRNo9F1fRMAJ4A/KLpiuWjw+X+S76gVR56ivX5nZd8k7pF6BJFLp+O8+iW+yi2DwUYsX7G9CDe3mVTxTiuhC+K+cjpCcSqTi6UWNt2DGN6Tz+aKdmqqbDKbp5M6mpSQmK/XaN+GPS654L8V/kYkxAb5kAXiKhcX2fR/Zgglese+Q2vx3RHkn+LRi9/IzPypVgT44MmtvSXRLTzpBP60Guu5Q4lLPDYM9TxMHZ1VCBzmM2VUkKBgHjhCy80S30zWHFi3E+V86qIvSDfIAzyB2aqTbSG3z X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, 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 Thu, Aug 22, 2024 at 04:35:55AM +0100, Pavel Begunkov wrote: > io_uring allows to implement custom file specific operations via > fops->uring_cmd callback. Use it to wire up asynchronous discard > commands. Normally, first it tries to do a non-blocking issue, and if > fails we'd retry from a blocking context by returning -EAGAIN to > core io_uring. > > Note, unlike ioctl(BLKDISCARD) with stronger guarantees against races, > we only do a best effort attempt to invalidate page cache, and it can > race with any writes and reads and leave page cache stale. It's the > same kind of races we allow to direct writes. Can you please write up a man page for this that clear documents the expecvted semantics? > +static void bio_cmd_end(struct bio *bio) This is really weird function name. blk_cmd_end_io or blk_cmd_bio_end_io would be the usual choices. > + while ((bio = blk_alloc_discard_bio(bdev, §or, &nr_sects, > + GFP_KERNEL))) { GFP_KERNEL can often will block. You'll probably want a GFP_NOWAIT allocation here for the nowait case. > + if (nowait) { > + /* > + * Don't allow multi-bio non-blocking submissions as > + * subsequent bios may fail but we won't get direct > + * feedback about that. Normally, the caller should > + * retry from a blocking context. > + */ > + if (unlikely(nr_sects)) { > + bio_put(bio); > + return -EAGAIN; > + } > + bio->bi_opf |= REQ_NOWAIT; > + } And this really looks weird. It first allocates a bio, potentially blocking, and then gives up? I think you're much better off with something like: if (nowait) { if (nr_sects > bio_discard_limit(bdev, sector)) return -EAGAIN; bio = blk_alloc_discard_bio(bdev, §or, &nr_sects, GFP_NOWAIT); if (!bio) return -EAGAIN bio->bi_opf |= REQ_NOWAIT; goto submit; } /* submission loop here */ > diff --git a/include/uapi/linux/fs.h b/include/uapi/linux/fs.h > index 753971770733..0016e38ed33c 100644 > --- a/include/uapi/linux/fs.h > +++ b/include/uapi/linux/fs.h > @@ -208,6 +208,8 @@ struct fsxattr { > * (see uapi/linux/blkzoned.h) > */ > > +#define BLOCK_URING_CMD_DISCARD 0 Is fs.h the reight place for this? Curious: how to we deal with conflicting uring cmds on different device and how do we probe for them? The NVMe uring_cmds use the ioctl-style _IO* encoding which at least helps a bit with that and which seem like a good idea. Maybe someone needs to write up a few lose rules on uring commands?