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 E04F2C47422 for ; Fri, 26 Jan 2024 09:36:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 411D16B00B9; Fri, 26 Jan 2024 04:36:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 39AE86B00C0; Fri, 26 Jan 2024 04:36:57 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 23B386B00C1; Fri, 26 Jan 2024 04:36:57 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0012.hostedemail.com [216.40.44.12]) by kanga.kvack.org (Postfix) with ESMTP id 101DE6B00B9 for ; Fri, 26 Jan 2024 04:36:57 -0500 (EST) Received: from smtpin15.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id A802740181 for ; Fri, 26 Jan 2024 09:36:56 +0000 (UTC) X-FDA: 81720958032.15.584A24B Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) by imf30.hostedemail.com (Postfix) with ESMTP id 43EC98001E for ; Fri, 26 Jan 2024 09:36:53 +0000 (UTC) Authentication-Results: imf30.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=gOuwyWAt; dmarc=none; spf=none (imf30.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1706261815; 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:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Fscjw5+02nWcqj9YGEOz7mXAdbzE5tCE48cyS+dnnGE=; b=qvTKVrTHYamuVAwY27G13WqjOX99/S3i1CD5h+Yp48rvPlqsDdEVpIF3pGnbLz7zAO0aAl FIBMRDDQbQB8SQe0eTXYdPUd062gMe2cJgEilgu2Avxqzt8Ji/TUWQslsqQ631x6DkmIDD BDaGn0w+dg54TBolxC8W0HQ57KcQWOA= ARC-Authentication-Results: i=1; imf30.hostedemail.com; dkim=pass header.d=infradead.org header.s=casper.20170209 header.b=gOuwyWAt; dmarc=none; spf=none (imf30.hostedemail.com: domain of willy@infradead.org has no SPF policy when checking 90.155.50.34) smtp.mailfrom=willy@infradead.org ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1706261815; a=rsa-sha256; cv=none; b=hOlpskI3Rp6x1NZrDtVZJKnXMc6cGeFEP++7SGFW84zeHcPExQ/9shq7B+elmsaGEinGSA SDXmxoPLQDq4pkir9bffOf+JXnZFCCODBZPucUx9zxbJnPai2lABaAFcncwWqmBYyEWHFi FSm4RCmyRnJOvJg0P48F9y1bE8hYo+U= DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Transfer-Encoding: Content-Type:MIME-Version:References:Message-ID:Subject:Cc:To:From:Date: Sender:Reply-To:Content-ID:Content-Description; bh=Fscjw5+02nWcqj9YGEOz7mXAdbzE5tCE48cyS+dnnGE=; b=gOuwyWAtOHJYZZB8kDVsEgfvNa tzk+b/d/l2FYsERlAfsujmb8rKjohGn4n+Bjerm1Nu7gIas8kU4qd1vl82CdJSFqXa7a7TwZvpl2E OJHMXMGJ1HLGeFpsSeuP+rp+sjh7BCtodOy1dwYLVP8zU6Ulx8F3lb+AslA+ZfZ3vkVmeiJ5m8xDo leXDzaEjSp7e1eJfAjjruhDlU8oyRIKOV/IYs65et9NcfRBY7K79zrMREaTPQlZu1sVc7ZtQHWONm 1GV/jWHI8v3hXvaje41swjLKtQeaESAwcfX5HYtX0rrFvpERUrGeI4fzSiqEEBY1By36pBnx56t6t x9B7Ww9Q==; Received: from willy by casper.infradead.org with local (Exim 4.97.1 #2 (Red Hat Linux)) id 1rTIdY-0000000DBIw-3Qwx; Fri, 26 Jan 2024 09:36:36 +0000 Date: Fri, 26 Jan 2024 09:36:36 +0000 From: Matthew Wilcox To: Zhaoyang Huang Cc: "zhaoyang.huang" , Alexander Viro , Christian Brauner , linux-fsdevel@vger.kernel.org, Andrew Morton , Jens Axboe , Yu Zhao , Damien Le Moal , Niklas Cassel , "Martin K . Petersen" , Hannes Reinecke , Linus Walleij , linux-mm@kvack.org, linux-block@vger.kernel.org, linux-kernel@vger.kernel.org, steve.kang@unisoc.com Subject: Re: [PATCHv3 1/1] block: introduce content activity based ioprio Message-ID: References: <20240125071901.3223188-1-zhaoyang.huang@unisoc.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Rspamd-Queue-Id: 43EC98001E X-Rspam-User: X-Rspamd-Server: rspam05 X-Stat-Signature: rfdx58frfkb7c7nh9o5chihb5d13d6uw X-HE-Tag: 1706261813-937139 X-HE-Meta: U2FsdGVkX18Zg5hIbRKyWqH8TyLXYIXPneyvs/nyadE67Aaq6M1sm8hAWca9Digp15WP1ynXpmJegkXQJETGhcS4P9yxzHecBGXm3ZbVewS2RvA8M5ex5c+JQs5q17y9Q8MKnUx1G52HTLZydLTL/s+y3Nlk/uDnU0qq8X6eIfF3+L42qRekCXFIZZ005kWfjjx925rx1ddxxIbiP4fKC62R6ll3w2F/xO6uWw3ujmI/w7ZGYvSGFpy7QtpFxoqPJXL/YiMUzHJRNbAyM/uLK62TdkN8JfOHZQsgv7JYvagX0Qzmmral+Iwq/5LeW3tFxknNYAWq5xPugrabnYYwy4nluq3eNG80fSF5NKONgckz5dcisWOdybT69BazzmQB13bcsWzMb5X87bpFjfkOYv1Qzoce/Y/nthrk3TX1bl3U7uTXmT3ivV/yzTa439SctR/lNuZh5sBlIOvcloP1/n67RSHr7e34knya0q7MO3n89dCVcWK3pB06MvNGyOy4dHxwuO/PRk1DWhPjfEWI+qQgSueDMaV8X0hes9y9zqfUvvBOg4WGsaZ1o0p/o9/srRMUCzswSGSTbgb2g+BH6YX4PqLcL73n5L115x0p1bEjMR1VIECu6jTB82BgYI2wEpoFvZT0MozJOaUyXdvfewAGaJ2xZv/shJ9/LPLIOvJhbpTcRuaGJzMrBO8JGZxoqMcF57nsIlvhV8rCvJ7whSLwRZ33LGmK3I/kijNXnUK9vKd0Mv1fkyuNo7h0A429YL5hJdHdlRJOXu8JVT+sa5pieBph1cA1xK8tNcjERN4K4QK0L5P/HsLWCD6bTzTpKYj5iiaMxi/lbPyvinDgQr/fp41IOiDeHOmQc9e7HuU6OV81Rh7lJau8celTsoDkjpJ/eSFdIRVRw79ZxtCKMCcf2ELrOJDZPRmuu2YOMthe0M3i9gjG6TZFK+H7a7u1dTB9igX2HaBu++L4Czt WiW42QUI BSlzFQHmq7ioSWqCG9NF/zPRM+qIslVfvx3HZFVsX2M8TJEsTw1MihM4EKeIYE6SQEiPg3HL3T4F99ZP4XK99Uswz4B7J+bBVFu+EQf52B2a37SSQMAOmFlBKourovvTznCrDHQ621Wrj24QufhPoGeq0nYUWUy9Zq+A1sBNItDxZbvpF7Ekd3HQ3pvlZwtV0X6DppfzWxzwc5/PeCXq6qSNl95B3lc24HMbPkBT4hy9vYxU= 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 Fri, Jan 26, 2024 at 05:28:58PM +0800, Zhaoyang Huang wrote: > On Fri, Jan 26, 2024 at 4:55 PM Matthew Wilcox wrote: > > > > On Fri, Jan 26, 2024 at 03:59:48PM +0800, Zhaoyang Huang wrote: > > > loop more mm and fs guys for more comments > > > > I agree with everything Damien said. But also ... > ok, I will find a way to solve this problem. > > > > > > +bool BIO_ADD_FOLIO(struct bio *bio, struct folio *folio, size_t len, > > > > + size_t off) > > > > You don't add any users of these functions. It's hard to assess whether > > this is the right API when there are no example users. > Actually, the code has been tested on ext4 and f2fs by patchv2 on a > v6.6 6GB android system where I get the test result posted on the > commit message. These APIs is to keep block layer clean and wrap > things up for fs. well, where's patch v2? i don't see it in my inbox. i'm not going to go hunting around the email lists for it. this is not good enough. > > why are BIO_ADD_PAGE and BIO_ADD_FOLIO so very different from each > > other? > These two API just repeat the same thing that bio_add_page and > bio_add_folio do. what? here's the patch you sent. these two functions do wildly different things: +bool BIO_ADD_FOLIO(struct bio *bio, struct folio *folio, size_t len, + size_t off) +{ + int class, level, hint, activity; + + if (len > UINT_MAX || off > UINT_MAX) + return false; + + class = IOPRIO_PRIO_CLASS(bio->bi_ioprio); + level = IOPRIO_PRIO_LEVEL(bio->bi_ioprio); + hint = IOPRIO_PRIO_HINT(bio->bi_ioprio); + activity = IOPRIO_PRIO_ACTIVITY(bio->bi_ioprio); + + activity += (bio->bi_vcnt + 1 <= IOPRIO_NR_ACTIVITY && + PageWorkingset(&folio->page)) ? 1 : 0; + if (activity >= bio->bi_vcnt / 2) + class = IOPRIO_CLASS_RT; + else if (activity >= bio->bi_vcnt / 4) + class = max(IOPRIO_PRIO_CLASS(get_current_ioprio()), IOPRIO_CLASS_BE); + + bio->bi_ioprio = IOPRIO_PRIO_VALUE_ACTIVITY(class, level, hint, activity); + + return bio_add_page(bio, &folio->page, len, off) > 0; +} + +int BIO_ADD_PAGE(struct bio *bio, struct page *page, + unsigned int len, unsigned int offset) +{ + int class, level, hint, activity; + + if (bio_add_page(bio, page, len, offset) > 0) { + class = IOPRIO_PRIO_CLASS(bio->bi_ioprio); + level = IOPRIO_PRIO_LEVEL(bio->bi_ioprio); + hint = IOPRIO_PRIO_HINT(bio->bi_ioprio); + activity = IOPRIO_PRIO_ACTIVITY(bio->bi_ioprio); + activity += (bio->bi_vcnt <= IOPRIO_NR_ACTIVITY && PageWorkingset(page)) ? 1 : 0; + bio->bi_ioprio = IOPRIO_PRIO_VALUE_ACTIVITY(class, level, hint, activity); + } + + return len; +} did you change one and forget to change the other? > These white spaces are trimmed by vim, I will change them back in next version. vim doesn't do that by default.