From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f43.google.com (mail-pj1-f43.google.com [209.85.216.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 794BE30DD11 for ; Fri, 6 Mar 2026 02:05:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.43 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772762710; cv=none; b=SohZVGCCuXYlacJSfYI+JZC4ukxROuHdB8NN4OeSjvppec4ly+45Yfb4T2hvGJ8jKFJwXWrJnYvL69M/PE9v6q+6FGyZ4YeHeaYxHcinoV6H43p4YKSABerkvt1JdyV9oVEh+CkGC1+C3LYnkJWTYbVX0iPJ5BxdOkP0/GcAjWM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772762710; c=relaxed/simple; bh=ZolZG20xlR9hrqrydDArlydwOBb0Hn40AqcbjCpEHMs=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=i//Kl47DMFh6Cw6XIjzOxuAUeR/0vXBJ6iD2TEDiOhgOibBreX9DhR7BMhl+xNr3Zbi1dyL/1XPl9yQvtYl/tcB7kEcHNcFIFiK3TwlL90s7L6i7oe/4wGj6r20SUlCQ6tnhdm4gp+6JlSKivU2Q/gu4QMFgxwPvwiOug7EBunw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ihLWS+Iz; arc=none smtp.client-ip=209.85.216.43 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ihLWS+Iz" Received: by mail-pj1-f43.google.com with SMTP id 98e67ed59e1d1-35995cb33a8so2671263a91.0 for ; Thu, 05 Mar 2026 18:05:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1772762709; x=1773367509; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=K6hWoJU5NFg6LYKfkwpfVqwiXWe8S4He0GFrCPJwzgU=; b=ihLWS+IzPrj1U7wuqA5J4pD1nn4TFhJbBBNnRU96OsATCevjQLpTq1byNq/s1+OUda dxnLK5deShRvU9LukIOyMWpCJwRL/R4pAyGfxCDqYS6znNagDNshvJBE9ZTaH3UWPFS1 /A37Fps2hEz8Hg8VLJKd7cxOShHTbf6xBWCNjt9s4JX7Wy0fFQuls6OmpCd1oye2lgJZ FvAE+/5VPuUyocPru/Fruu/LUy2BCIE2OfJYNwCpY8EBuHLYdvuJ3JfXEFgeceH45GQB 5poGkwHbOyQqyB0hpUts9CaKXSu3Q7XeRRxa2A7d0babud2Op99sI+94oktJDRzflRq5 YMAg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1772762709; x=1773367509; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=K6hWoJU5NFg6LYKfkwpfVqwiXWe8S4He0GFrCPJwzgU=; b=igDSAoJIumQxBoYkR6XBKN++KwQsvs53CrUyO6eeZWX2TYXwXLwGM7bM4IACBZEjiv zBHfffBfABt5U9Tvo94SlAwBOt2+okBuLAFD3P/TivFTZP33Gjr+AUbmXh1YpvAmAFxn PpBnx/Zt1IyPp2P4ALTZfKNr0UZtu80mSye0xMd8prC9epUwqg6M1ueLRR2+4sz4Fu8P hg2140d8qaIT6FjPDscmPRLXnEAh/+jesBNP9LC0pf5YOpcRfohwcFkRfCcbdx+ejZ8V YK1CAqTS2rCK0zIcyf5Vrb6yEUoTzGlzlo3DHglCSOVkabiBqn9XiZ1L0KiPxjuFhmlf fEPQ== X-Forwarded-Encrypted: i=1; AJvYcCVLIVz7+AnTXNCotnYB5Tjhy2hLed7BTH1Pg+QE+VZ5/K8+t28PjnO8Z9utELKPnfFaH8szSRy/npZRPBg=@vger.kernel.org X-Gm-Message-State: AOJu0YxrW9gfsG6RQ0q7li77bB9o5/F8e2KbVL0r4zqHAIxfIfkLgRVB GzbpSHs/b4uPFCrK8mIRWCfB47HQtUBAiPEK3LnBhjNKVdYDaR+D+TDq X-Gm-Gg: ATEYQzx2YmojEE3P2X6FRdil9tVgloDCEJ0KNtnnio1OyrzgqFaZkQHaBAxfeHHYA8T VB567lnNbue1fjZO/8sCAcD974q5OzUGhpMEIHuY7XqV4JyEpTK3hvkMvniN9pMs1m5Ji8et9mD 05DkqoIZldhoO2Mv7AME8HvfgBby1Ki7Lkcep7wozoSx1i1Q8E8ME85TAMyWwTK7jSTt8x1Mqv0 jpHT3hWw0Sg7AIe848R5K2yqUBlmwPMj1nj5uid/HJhVgcfrFLB1e4hDoj/hC53CGPVvQ/OnaH4 97rT3iXXvyeTjDZab9olF2zyZBUhvFJArMhJL7IUOv5AJIgbXFArAP5QxEG+QB10xtxQ2UQpnQg vIuF4iHr7rKkw/3nYHp79bO7SAqopOXLJHQ+KlkHY9HphnnChNzqRhE0xVK4wWEHhgKoE7GfNl1 km1AMaUXI5aIIomA== X-Received: by 2002:a17:90b:3e86:b0:359:83a3:584d with SMTP id 98e67ed59e1d1-359b1ba5209mr3908844a91.6.1772762708740; Thu, 05 Mar 2026 18:05:08 -0800 (PST) Received: from localhost ([27.122.242.71]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-359c003bd46sm22440a91.4.2026.03.05.18.05.07 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 05 Mar 2026 18:05:08 -0800 (PST) Date: Fri, 6 Mar 2026 11:05:05 +0900 From: Hyunchul Lee To: Viacheslav Dubeyko Cc: "glaubitz@physik.fu-berlin.de" , "frank.li@vivo.com" , "slava@dubeyko.com" , "hch@infradead.org" , "linux-kernel@vger.kernel.org" , "linux-fsdevel@vger.kernel.org" , "cheol.lee@lge.com" Subject: Re: [PATCH] hfsplus: limit sb_maxbytes to partition size Message-ID: References: <20260303082807.750679-1-hyc.lee@gmail.com> <5c670210661f30038070616c65492fa2a96b028c.camel@ibm.com> <532c5cdf12ced8eee5e5a93efe592937b63b889d.camel@ibm.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: On Fri, Mar 06, 2026 at 01:23:16AM +0000, Viacheslav Dubeyko wrote: > On Fri, 2026-03-06 at 09:57 +0900, Hyunchul Lee wrote: > > On Thu, Mar 05, 2026 at 11:21:19PM +0000, Viacheslav Dubeyko wrote: > > > On Thu, 2026-03-05 at 10:52 +0900, Hyunchul Lee wrote: > > > > > > > > > > > > Sorry it's generic/285, not generic/268. > > > > > > in generic/285, there is a test that creates a hole exceeding the block > > > > > > size and appends small data to the file. hfsplus fails because it fills > > > > > > the block device and returns ENOSPC. However if it returns EFBIG > > > > > > instead, the test is skipped. > > > > > > > > > > > > For writes like xfs_io -c "pwrite 8t 512", should fops->write_iter > > > > > > returns ENOSPC, or would it be better to return EFBIG? > > > > > > > > > > > > > > > > > Current hfsplus_file_extend() implementation doesn't support holes. I assume you > > > > > mean this code [1]: > > > > > > > > > > len = hip->clump_blocks; > > > > > start = hfsplus_block_allocate(sb, sbi->total_blocks, goal, &len); > > > > > if (start >= sbi->total_blocks) { > > > > > start = hfsplus_block_allocate(sb, goal, 0, &len); > > > > > if (start >= goal) { > > > > > res = -ENOSPC; > > > > > goto out; > > > > > } > > > > > } > > > > > > > > > > Am I correct? > > > > > > > > > Yes, > > > > > > > > hfsplus_write_begin() > > > > cont_write_begin() > > > > cont_expand_zero() > > > > > > > > 1) xfs_io -c "pwrite 8t 512" > > > > 2) hfsplus_begin_write() is called with offset 2^43 and length 512 > > > > 3) cont_expand_zero() allocates and zeroes out one block repeatedly > > > > for the range > > > > 0 to 2^43 - 1. To achieve this, hfsplus_write_begin() is called repeatedly. > > > > 4) hfsplus_write_begin() allocates one block through hfsplus_get_block() => > > > > hfsplus_file_extend() > > > > > > I think we can consider these directions: > > > > > > (1) Currently, HFS+ code doesn't support holes. So, it means that > > > hfsplus_write_begin() can check pos variable and i_size_read(inode). If pos is > > > bigger than i_size_read(inode), then hfsplus_file_extend() will reject such > > > request. So, we can return error code (probably, -EFBIG) for this case without > > > calling hfsplus_file_extend(). But, from another point of view, maybe, > > > hfsplus_file_extend() could be one place for this check. Does it make sense? > > > > > > (2) I think that hfsplus_file_extend() could treat hole or absence of free > > > blocks like -ENOSPC. Probably, we can change the error code from -ENOSPC to - > > > EFBIG in hfsplus_write_begin(). What do you think? > > > > > Even if holes are not supported, shouldn't the following writes be > > supported? > > > > xfs_io -f -c "pwrite 4k 512" > > > > If so, since we need to support cases where pos > i_size_read(inode), > > The pos > i_size_read(inode) means that you create the hole. Because, That's correct. However I believe that not supporting writes like the one mentioned above is a significant limitation. Filesystems that don't support sparse files, such as exFAT, allocate blocks and fill them with zeros. > oppositely, when HFS+ logic tries to allocate new block, then it expects to have > pos == i_size_read(inode). And we need to take into account this code [1]: > > if (iblock >= hip->fs_blocks) { > if (!create) > return 0; > if (iblock > hip->fs_blocks) <-- This is the rejection of hole > return -EIO; > if (ablock >= hip->alloc_blocks) { > res = hfsplus_file_extend(inode, false); > if (res) > return res; > } > } > > The generic_write_end() changes the inode size: i_size_write(inode, pos + > copied). I think that it's not problem. hfsplus_write_begin() cont_write_begin() cont_expand_zero() cont_expand_zero() calls hfsplus_get_block() to allocate blocks between i_size_read(inode) and pos, if pos > i_size_read(inode). > > > wouldn't the condition "pos - i_size_read(inode) > free space" be better? > > Also instead of checking every time in hfsplus_write_begin() or > > hfsplus_file_extend(), how about implementing the check in the > > file_operations->write_iter callback function, and returing EFBIG? > > Which callback do you mean here? I am not sure that it's good idea. > Here is a simple code snippet. static const struct file_operations hfsplus_file_operations = { ... - .write_iter = generic_file_write_iter, + .write_iter = hfsplus_file_write_iter, ... +ssize_t hfsplus_file_write_iter(struct kiocb *iocb, struct iov_iter *iter) +{ ... + // check iocb->ki_pos is beyond i_size + + ret = generic_file_write_iter(iocb, iter); > Thanks, > Slava. > > > > > > > > > > > > > [1] https://elixir.bootlin.com/linux/v6.19/source/fs/hfsplus/extents.c#L239 -- Thanks, Hyunchul