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 X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS,USER_AGENT_SANE_1 autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A4430C433E2 for ; Thu, 3 Sep 2020 07:13:41 +0000 (UTC) Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 5DC9C2071B for ; Thu, 3 Sep 2020 07:13:41 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 5DC9C2071B Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=lst.de Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Received: from bilbo.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 4BhsVC3M7dzDr3V for ; Thu, 3 Sep 2020 17:13:39 +1000 (AEST) Authentication-Results: lists.ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lst.de (client-ip=213.95.11.211; helo=verein.lst.de; envelope-from=hch@lst.de; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=lst.de Received: from verein.lst.de (verein.lst.de [213.95.11.211]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 4BhsS71SnwzDr3V for ; Thu, 3 Sep 2020 17:11:50 +1000 (AEST) Received: by verein.lst.de (Postfix, from userid 2407) id 0164568BEB; Thu, 3 Sep 2020 09:11:44 +0200 (CEST) Date: Thu, 3 Sep 2020 09:11:44 +0200 From: Christoph Hellwig To: Linus Torvalds Subject: Re: [PATCH 10/10] powerpc: remove address space overrides using set_fs() Message-ID: <20200903071144.GA19247@lst.de> References: <20200827150030.282762-1-hch@lst.de> <20200827150030.282762-11-hch@lst.de> <8974838a-a0b1-1806-4a3a-e983deda67ca@csgroup.eu> <20200902123646.GA31184@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.17 (2007-11-01) X-BeenThere: linuxppc-dev@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Linux on PowerPC Developers Mail List List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-arch , Kees Cook , the arch/x86 maintainers , Linux Kernel Mailing List , Al Viro , linux-fsdevel , linuxppc-dev , Christoph Hellwig Errors-To: linuxppc-dev-bounces+linuxppc-dev=archiver.kernel.org@lists.ozlabs.org Sender: "Linuxppc-dev" On Wed, Sep 02, 2020 at 11:02:22AM -0700, Linus Torvalds wrote: > I don't see why this change would make any difference. Me neither, but while looking at a different project I did spot places that actually do an access_ok with len 0, that's why I wanted him to try. That being said: Christophe are these number stables? Do you get similar numbers with multiple runs? > And btw, why do the 32-bit and 64-bit checks even differ? It's not > like the extra (single) instruction should even matter. I think the > main reason is that the simpler 64-bit case could stay as a macro > (because it only uses "addr" and "size" once), but honestly, that > "simplification" doesn't help when you then need to have that #ifdef > for the 32-bit case and an inline function anyway. I'll have to leave that to the powerpc folks. The intent was to not change the behavior (and I even fucked that up for the the size == 0 case). > However, I suspect a bigger reason for the actual performance > degradation would be the patch that makes things use "write_iter()" > for writing, even when a simpler "write()" exists. Except that we do not actually have such a patch. For normal user writes we only use ->write_iter if ->write is not present. But what shows up in the profile is that /dev/zero only has a read_iter op and not a normal read. I've added a patch below that implements a normal read which might help a tad with this workload, but should not be part of a regression. Also Christophe: can you bisect which patch starts this? Is it really this last patch in the series? --- diff --git a/drivers/char/mem.c b/drivers/char/mem.c index abd4ffdc8cdebc..1dc99ab158457a 100644 --- a/drivers/char/mem.c +++ b/drivers/char/mem.c @@ -726,6 +726,27 @@ static ssize_t read_iter_zero(struct kiocb *iocb, struct iov_iter *iter) return written; } +static ssize_t read_zero(struct file *file, char __user *buf, + size_t count, loff_t *ppos) +{ + size_t cleared = 0; + + while (count) { + size_t chunk = min_t(size_t, count, PAGE_SIZE); + + if (clear_user(buf + cleared, chunk)) + return cleared ? cleared : -EFAULT; + cleared += chunk; + count -= chunk; + + if (signal_pending(current)) + return cleared ? cleared : -ERESTARTSYS; + cond_resched(); + } + + return cleared; +} + static int mmap_zero(struct file *file, struct vm_area_struct *vma) { #ifndef CONFIG_MMU @@ -921,6 +942,7 @@ static const struct file_operations zero_fops = { .llseek = zero_lseek, .write = write_zero, .read_iter = read_iter_zero, + .read = read_zero, .write_iter = write_iter_zero, .mmap = mmap_zero, .get_unmapped_area = get_unmapped_area_zero,