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 phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id D0D5FC77B75 for ; Tue, 18 Apr 2023 14:39:11 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 3E825860ED; Tue, 18 Apr 2023 16:37:50 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=atmark-techno.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 9F6ED86034; Tue, 18 Apr 2023 04:05:43 +0200 (CEST) Received: from gw.atmark-techno.com (gw.atmark-techno.com [13.115.124.170]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 2EDE1856E5 for ; Tue, 18 Apr 2023 04:05:39 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=atmark-techno.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=dominique.martinet@atmark-techno.com Received: from mail-pf1-f197.google.com (mail-pf1-f197.google.com [209.85.210.197]) by gw.atmark-techno.com (Postfix) with ESMTPS id 359616019C for ; Tue, 18 Apr 2023 11:05:36 +0900 (JST) Received: by mail-pf1-f197.google.com with SMTP id d2e1a72fcca58-63b79d8043eso3634330b3a.0 for ; Mon, 17 Apr 2023 19:05:36 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1681783535; x=1684375535; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=qAPiAIU5HgclSinkp6F/Z6yHela0V5UFfJjRmjfpsOQ=; b=ZI4U0P2+rSCmRnFTSS5b3ng4pQ2c/KleX6CgEYzyXa0+pCx8SjuEMbAxT/3rNe3BQ0 xeCFYS07ZSlYkCZYUdr3Wr31YPSrN9D4cmuTl+y+q4CX4l3U3bTvhvs6TjKS1b72oFD3 mUpBPBOfqD+7/K+Ind18jK/x2LuMZhf5nuU7PZb5/MwMmgBt1n5+f1EfZYjDpUFsmjsx GmqQ9/rNnoxlg821DVB5ryFKlGBueFUT5wTCMmukDc75zNh1ukHhGoIHHBxq7uIq4uG+ 35JIEz2X20KRdDsqPlblfMHHXmazihXGsEKFdc6yzPyoU7oBzkAFwzGaAqJZ8xsG9oiq +Gvg== X-Gm-Message-State: AAQBX9c4tIBR+3aXWKf4jLWjIoZA7ZCjmBREfZHhUAH/h/JNDFVXiUXx muC8VrJAb9PPlG9BA2/tylx1N0i+OHicobrMqEfVVhEgMtpDd1G4MgrNGOnqoWmegZXxqV5VSVS hXLun6FnjkPEZkrJ23g== X-Received: by 2002:a17:902:ea0b:b0:1a3:dcc1:307d with SMTP id s11-20020a170902ea0b00b001a3dcc1307dmr603723plg.23.1681783535780; Mon, 17 Apr 2023 19:05:35 -0700 (PDT) X-Google-Smtp-Source: AKy350bLwCPS7GbsopYCgKlj2OCNPofnolvAojawj4enlZlARH69d5NLtIUeHL5qeDYsjEyvBVlyfQ== X-Received: by 2002:a17:902:ea0b:b0:1a3:dcc1:307d with SMTP id s11-20020a170902ea0b00b001a3dcc1307dmr603696plg.23.1681783535420; Mon, 17 Apr 2023 19:05:35 -0700 (PDT) Received: from pc-zest.atmarktech (76.125.194.35.bc.googleusercontent.com. [35.194.125.76]) by smtp.gmail.com with ESMTPSA id jl12-20020a170903134c00b0019fea4d61c9sm8332812plb.198.2023.04.17.19.05.35 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Mon, 17 Apr 2023 19:05:35 -0700 (PDT) Received: from martinet by pc-zest.atmarktech with local (Exim 4.96) (envelope-from ) id 1poair-00C6xf-3C; Tue, 18 Apr 2023 11:05:33 +0900 Date: Tue, 18 Apr 2023 11:05:23 +0900 From: Dominique Martinet To: Qu Wenruo Cc: Dominique Martinet , Marek =?utf-8?B?QmVow7pu?= , Qu Wenruo , linux-btrfs@vger.kernel.org, u-boot@lists.denx.de Subject: Re: [PATCH U-BOOT 1/3] btrfs: fix offset within btrfs_read_extent_reg() Message-ID: References: <20230418-btrfs-extent-reads-v1-0-47ba9839f0cc@codewreck.org> <20230418-btrfs-extent-reads-v1-1-47ba9839f0cc@codewreck.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: X-Mailman-Approved-At: Tue, 18 Apr 2023 16:37:46 +0200 X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.8 at phobos.denx.de X-Virus-Status: Clean Qu Wenruo wrote on Tue, Apr 18, 2023 at 09:58:37AM +0800: > The subject can be changed to "btrfs: fix offset when reading compressed > extents". > The original one is a little too generic. Ok. > > btrfs_file_read() > > -> btrfs_read_extent_reg > > (aligned part loop from 0x40480000 to 0x40ba0000, 128KB at a time) > > last read had 0x4000 bytes in extent, but aligned_end - cur limited > > the read to 0x3000 (offset 0x720000) > > -> read_and_truncate_page > > -> btrfs_read_extent_reg > > reading the last 0x1000 bytes from offset 0x73000 (end of the > > previous 0x4000) into 0x40ba3000 > > here key.offset = 0x70000 so we need to use it to recover the > > 0x3000 offset. > > You can use "btrfs ins dump-tree" to provide a much easier to read on-disk > data layout. key.offset is the offset within the read call, not the offset on disk. The file on disk looks perfectly normal, the condition to trigger the bug is to have a file which size is not sector-aligned and where the last extent is bigger than a sector. I had a look at dump-tree early on and couldn't actually find my file in there, now the problem is understood it should be easy to make a reproducer so I'll add this info and commands needed to reproduce (+ a link to a fs image just in case) in v2 > > /* Preallocated or hole , fill @dest with zero */ > > if (btrfs_file_extent_type(leaf, fi) == BTRFS_FILE_EXTENT_PREALLOC || > > @@ -454,9 +456,7 @@ int btrfs_read_extent_reg(struct btrfs_path *path, > > if (btrfs_file_extent_compression(leaf, fi) == BTRFS_COMPRESS_NONE) { > > u64 logical; > > - logical = btrfs_file_extent_disk_bytenr(leaf, fi) + > > - btrfs_file_extent_offset(leaf, fi) + > > - offset - key.offset; > > + logical = btrfs_file_extent_disk_bytenr(leaf, fi) + offset; > > This is touching non-compressed path, which is very weird as your commit > message said this part is correct. my rationale is that since this was considered once but forgotten the other time it'll be easy to add yet another code path that forgets it later, but I guess it won't change much anyway -- I'll fix the patch making it explicit again. Thanks, -- Dominique