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=-4.0 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_PASS,URIBL_BLOCKED autolearn=ham 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 0497CC282D7 for ; Sat, 2 Feb 2019 17:03:38 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id BCFB920856 for ; Sat, 2 Feb 2019 17:03:37 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=omnibond-com.20150623.gappssmtp.com header.i=@omnibond-com.20150623.gappssmtp.com header.b="FadKxhA8" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726367AbfBBRDh (ORCPT ); Sat, 2 Feb 2019 12:03:37 -0500 Received: from mail-yb1-f193.google.com ([209.85.219.193]:40670 "EHLO mail-yb1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1726055AbfBBRDg (ORCPT ); Sat, 2 Feb 2019 12:03:36 -0500 Received: by mail-yb1-f193.google.com with SMTP id y13so3983551ybr.7 for ; Sat, 02 Feb 2019 09:03:36 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=omnibond-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=3qzk6NU+/jBoioBxN+4HKM3smMnlSU9+MnNt6am4sBc=; b=FadKxhA8O/1GVtFfTlsB8IBAKo5BFiJOmPEjengDtndB/UuvGx2oFFGroRXu+NZD8e d7H0NBE0Zg1Z98ESvx/YyLQkOhfjLcg9DPNsGoyQmmzxoL20SVeKFMFCIXrRm64s33AQ Rhk+PHi6mS/P3+7WV5NSW3dSlC6tWpKzt7rpnQTEjunoaBesAhoqjfOx5RrUQN625Mdx 3GAkGqfrSLb+x0fCKjDHvkzxf55C70255eyUlwq1Wr7ae876R7+Yi7xU1BPuIG4FTRB5 hH6SAYlxCrWfZIk9j/KuL0oZ1isJFQVdUk39rUjIcnvG5AteQEr5U8aMLqslSBXSKyTb LHjA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=3qzk6NU+/jBoioBxN+4HKM3smMnlSU9+MnNt6am4sBc=; b=aThdxK1JHm9vb+h/fLU/It/UyENf5CtINMwFt9+97KoYIQVj5HbvV7/qjM2PVcGN8C XQrPO/rkEI2qtFMW5cjAQoJ1dkcSlvyo+asK5B8GpYqKH9D5r3IguTp/k/UrlRc37Yr9 xt1SSloZqcStEMP/RbeVwBEYFgCCLFhyXaCIIr9sazZ0ZWobYOnQ1Z0b+f6vwvFn8VGz aIMC4ZeoXxqzqYg+CgQcNj5rRfjPDActtme+NkxHCqlGEounfVdEXNJM1qeaEVf9wvwT /X1KdwL/4MFVM2z99UAE6iK9aDguALL8SLWptHWue8Rx5aO56qy5pdg44ZXiTMkd6+se ckTg== X-Gm-Message-State: AHQUAuazDBl6YBFxu4Pf5njjOIWRG2dn1dpw0Fit+si3W8c7Wb7miiHA 27t44zE7KFlB6rmyh6NrnaEyAP73A4VFXQ5SRnvDG5UOf9qaVQ== X-Google-Smtp-Source: AHgI3IasVci8+Cv9QRCkFNTTjqQfr/ROYlCR/o+DkJQS72rRUaD4kvM1ItDG9T5VyDLYXkdPfRHU7ZMZ9jgoADxoyw8= X-Received: by 2002:a25:6b0e:: with SMTP id g14mr9684462ybc.160.1549127015708; Sat, 02 Feb 2019 09:03:35 -0800 (PST) MIME-Version: 1.0 References: <25906140-c699-098f-13aa-fd721bd2e602@kernel.dk> <283717d4-66b4-1c2b-b9f7-955cc0f93555@kernel.dk> In-Reply-To: <283717d4-66b4-1c2b-b9f7-955cc0f93555@kernel.dk> From: Mike Marshall Date: Sat, 2 Feb 2019 12:03:23 -0500 Message-ID: Subject: Re: side effect from "aio: don't zero entire aio_kiocb aio_get_req" To: Jens Axboe Cc: Al Viro , linux-fsdevel , Christoph Hellwig , Martin Brandenburg Content-Type: text/plain; charset="UTF-8" Sender: linux-fsdevel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-fsdevel@vger.kernel.org I'll put it in aio_prep_rw like you suggest and run it through xfstests. I work through gmail, so I'll fiddle with my git-sendmail-foo and send the patch up after that. Thanks! -Mike On Sat, Feb 2, 2019 at 11:37 AM Jens Axboe wrote: > > On 2/2/19 9:35 AM, Jens Axboe wrote: > > On 2/1/19 7:29 PM, Mike Marshall wrote: > >> We've been working on our Orangefs page cache patch with blinders on, > >> and last week I took our patch set which was based on 4.19-rc7 > >> and applied it to 5.0-rc3. > >> > >> In the process I ran vanilla rc3, and rc3 plus an Orangefs related > >> patch set that Christoph Hellwig sent in, through the > >> suite of xfstests. > >> > >> It turns out that a patch from one of Jens Axboe's patch sets > >> that came, I think, in the 5.0 merge window triggered a BUG_ON in Orangefs' > >> file.c. The particular patch is "aio: don't zero entire aio_kiocb aio_get_req". > >> > >> This code is in Orangefs file.c in a couple of places: > >> > >> BUG_ON(iocb->private); > >> > >> Anywho... I can easily fix the Orangefs problem by removing the two > >> BUG_ON statements, I've researched how they got there and they > >> are vestigial, just the kind of thing that Linus hates :-). > >> > >> The bigger question is that maybe there is other code in other filesystems > >> that checks iocb->private without failing in a way that is as obvious > >> as BUG_ON. I don't see any upstream code with grep other than a few > >> lines in ext4/inode.c that might be affected. > >> > >> As a test, I "fixed" the Orangefs problem with this: > >> > >> [hubcap@vm1 linux]# git diff > >> diff --git a/fs/aio.c b/fs/aio.c > >> index b906ff70c90f..2605a4b1a3c9 100644 > >> --- a/fs/aio.c > >> +++ b/fs/aio.c > >> @@ -1020,6 +1020,7 @@ static inline struct aio_kiocb > >> *aio_get_req(struct kioctx *ctx) > >> if (unlikely(!req)) > >> return NULL; > >> > >> + req->rw.private = NULL; > >> percpu_ref_get(&ctx->reqs); > >> req->ki_ctx = ctx; > >> INIT_LIST_HEAD(&req->ki_list); > >> > >> So, the real fix for Orangefs is getting rid of the two BUG_ON lines, > >> and I'll do that, I just wanted to bring this up in case it matters > >> to anyone else... > > > > Let's just bring it back, I think your patch is fine. I don't see > > any other issues with this in a git grep, but better safe than sorry. > > > > Care to send this as a properly formatted patch so it can get > > included? > > BTW, I'd probably initialize it in aio_prep_rw(), that's the more > logical place for it. > > > diff --git a/fs/aio.c b/fs/aio.c > index b906ff70c90f..aaaaf4d12c73 100644 > --- a/fs/aio.c > +++ b/fs/aio.c > @@ -1436,6 +1436,7 @@ static int aio_prep_rw(struct kiocb *req, const struct iocb *iocb) > if (unlikely(!req->ki_filp)) > return -EBADF; > req->ki_complete = aio_complete_rw; > + req->private = NULL; > req->ki_pos = iocb->aio_offset; > req->ki_flags = iocb_flags(req->ki_filp); > if (iocb->aio_flags & IOCB_FLAG_RESFD) > > -- > Jens Axboe >