Received-SPF: pass (google.com: domain of linux-kernel+bounces-166161-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <d0f55fe1-0868-4797-8fa5-1f60f9a0058c@oracle.com>
Date: Thu, 2 May 2024 09:55:44 +0100
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v3 09/21] xfs: Do not free EOF blocks for forcealign
To: Dave Chinner <david@fromorbit.com>
Cc: djwong@kernel.org, hch@lst.de, viro@zeniv.linux.org.uk, brauner@kernel.org,
        jack@suse.cz, chandan.babu@oracle.com, willy@infradead.org,
        axboe@kernel.dk, martin.petersen@oracle.com,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        tytso@mit.edu, jbongio@google.com, ojaswin@linux.ibm.com,
        ritesh.list@gmail.com, mcgrof@kernel.org, p.raghav@samsung.com,
        linux-xfs@vger.kernel.org, catherine.hoang@oracle.com
References: <20240429174746.2132161-1-john.g.garry@oracle.com>
 <20240429174746.2132161-10-john.g.garry@oracle.com>
 <ZjF2jjtsA/C6ajtb@dread.disaster.area>
 <833f5821-a928-441f-848f-3a846111dcb7@oracle.com>
 <ZjLoJ4FeSbsb/hch@dread.disaster.area>
Content-Language: en-US
From: John Garry <john.g.garry@oracle.com>
Organization: Oracle Corporation
In-Reply-To: <ZjLoJ4FeSbsb/hch@dread.disaster.area>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?VVNORDdnR1VPeWZGVTR0QzgxMGhRYzhaWjRGamNnQTFlZ2VpMUhxUHdoNlk1?=
 =?utf-8?B?MkJIdlg3OXZNZlFNeVM4eGxsK1VQelV1WFMweHViL1hzcHZ4MnJ4bE9WcUIw?=
 =?utf-8?B?d1JkRHlGWFlYaDRaZFNwRnRGQ3ptSWJVUFJDNWN5VDdsY2xIVzc4RTFNTTZv?=
 =?utf-8?B?cU1ocUxFOFZuNnlMOVN1S2hBRmdsb3FDKzRxVWhjWURQNTdHSUQ5YUJ0S1ZB?=
 =?utf-8?B?WGN3MXFzTXRkQzc0UGlwb0ZoazFnZmJDVXlRM1hIdHhwTVlrbFlOYldjd01E?=
 =?utf-8?B?bm1yR0cwYkpyT0M0amJKWENVd0pSVTIxMnpLOEpKTFJDNllsbjhQYVE3bUZY?=
 =?utf-8?B?ZlpURDJOKy8vb1Evd1dVcWNrblpqRjdsT0N1bWVoN2Q5aHVnK216RWZQWFN2?=
 =?utf-8?B?NkF1QThhbEdjL2R1aURuUTBySUVUS3JsRk8veGk4TWJlUEV5VDJqaCs2Njc4?=
 =?utf-8?B?ZERyMDI5RDFGb2RsV0VRUTlTNjh5bkVvRmtyY21kZmJ1eEpINGl2ZDd4eGtu?=
 =?utf-8?B?eHd2SUk3ZDhVSlk1SlBMRFhpK0I3eGRtRVVPU3hDRW40R2h2d1lQNnl1ZW0y?=
 =?utf-8?B?aS9PamlkakQzYXJoU1d6djk4NzRkVVN2bGU1My9DZGpUTTE4aGM0NDY3YWli?=
 =?utf-8?B?ZVlWUU1XTm5iVTFJRWNTcGozRTd0bmxjNllwSGNZSDZ2c3dBOXd3Rk9HRWJT?=
 =?utf-8?B?RjlXclRVbnpBQnhsN2VBeXE1RVF1Sm5jMnRBZFZNNkE5RVBtZnZZUWlMcGNa?=
 =?utf-8?B?OEQvZXcyQ1hHb0FvcWppQVplbVdibFVMaXl1WWJFZ09SRXdFemFVa2Q0TUZ2?=
 =?utf-8?B?NHVyNTVMMjhYSUJtQjQ2cW92NHpER0cvWmM0YU5JY0l2ODhleG1XZkVOcW1H?=
 =?utf-8?B?QWZmTklmbUhGR2lpYi9XQXJvcXBnVkZLYUM5dDlxemVGY2hlQlJLOGtTdGpB?=
 =?utf-8?B?TmkyNW1JWkxXZ2VoTGhXZnZSQnhtNTQwUWgwSlBqMWl6UHR0bkErV0Z3ZnVQ?=
 =?utf-8?B?SXExUndqYldad0tjb2RwZi92NDI4ZVc1YksvZld1aXVWTUdHMThGdVprZEgy?=
 =?utf-8?B?NllWMHhzem1Md05nRnQ3SE1DY0tHdjRjdXdYelhZWnhOZDV5NEJRUHhpK3Jx?=
 =?utf-8?B?MmJIYWZkeEJ4K2tsREFQVkxjSjQ5bUJLdXZHMTBHUlJEV1p2eUwyU1F5d1FV?=
 =?utf-8?B?dG5YNEE3ZmMweFd3L0cyQXpkQWF0YkEvZTA4MEU4OHFrdmgyQnZnbm5FR0ZH?=
 =?utf-8?B?MjJGRm13c3JlaWFEbnhPTjNuK2pkeWg3bUMvMEFJbWUxUkIxMWluZzM2MzZ2?=
 =?utf-8?B?WGdOUC9mZnU2dGtIUU1IcExuVXF0em5hSndmQmJmYnZQYVJBQjJpaHlyand5?=
 =?utf-8?B?KzhQeGJCYmhCMkVEUkUxNnJkVUtLdTZaSWpwVWpmSm9paGM5MGlOSDVKTENu?=
 =?utf-8?B?bU9mMXpBVmdoYmtOWlExUkVwK1ZjNkwvSkV0alRHWnBRR1pJTnAzdS9DTUlo?=
 =?utf-8?B?dEIrVzlKZlJFT3hVN0w2eHROb01PNG92RFR5TVN6VUpHM0FBOXpvbGI0TC9h?=
 =?utf-8?B?SlRPT3VkTW0rSnlHbHZrVHBRRk1FRGZ1S2NrcE5RV29Kdi83U3dmZjYxWnlM?=
 =?utf-8?B?UzMvdnlBd0YyeHFnNXM5Y2NrSkdqU0d1VnZTUHYzZnRaUXZGdUVUY0xMdThH?=
 =?utf-8?B?WXhLRE13ZmxuYmc4UWg3UU1pTXQ5enhqenZGbmZOVnJMbytuK2xZR0swanIr?=
 =?utf-8?B?WC9YaFlZYkxlbnZWd2Y2LzRjTERQTDZ2c3g1VXlnOFpNYlNwbDVGMko0bFVL?=
 =?utf-8?B?VjFYTk93VjZXTFlabWhabWRZWjN0U2k1WS9QYS9zM3lPemkxWHF1R2VremlZ?=
 =?utf-8?B?VkxRdHNwQ0pLdmpqMUljQzljVWVBUHg0YmNURFhnNGNKWkYrOC91NmdwWk10?=
 =?utf-8?B?dU5VazdNUlMvQi8wemp4STJpSWR6N0lwWDdoNGZOZkxKVTAvQnVmV0VHcmdW?=
 =?utf-8?B?ZW1vY2x2Y0huWGE5UEcvNDRqOEdySVRzOTQzNm5tMDhtZG5vbnRHaDNBUWhJ?=
 =?utf-8?B?TVE3dzhEQkFsVGE4OVlTUGxUWklJOTIwbDBxT2JwaU5Bam9OUnV0MVc1eHcv?=
 =?utf-8?B?NjRxWDgyQWNQRlFmcXVub1pTVzlFakszd1BpbkNIQWFrMDVxdm9hWkxuNWFn?=
 =?utf-8?B?NEE9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	mEhNcOUnjqm+6jKfN0+/k/bMi2+urUE36dcDR2Ox1v4xPObcOiU8VF1TeW7+w1YtWxvXuPkXtsOZG759ztbPNW+K6oOZ52QPDYX5hxALExWZkLJXIIEXhpQ/U/NCVdBgrnBJRuy0hKDwX6CxFSi6Ge1eYctS3uTnyE15Jo16jtSvosFPE0SpPWrAH0LzhmXKwGj0DezerwL+qea4boWXfNpxjYevqJnWIXPLKuwlO4kBE0ZYrFse9aRFxUugurBs+ZEGxED+vT1Yqdr6iZZVEcw3mzDYv8JxasSHfH7BSbwrJWTwgA1Ot7xJDkO4P4V8zJJ7Oap0/i+8bP2dgYSB939QrfWE1U5Hc29XkPzTfxCYsrYG7zAQWXnQ51VTL87tCtisqEEFmO+ol4H113eDghMi7XKvkiGLNapG9yZP93za32D/Oj/nSsd11C6NzpzorRWl/opF7/lLqziLp4gvKp2aO/LfZ2Pd3WrIRelj0qfMIeaS1QK9bLNwetafB5pwrspZkB8F/n38gOLYbkvndCvsCJKzVGNZjrJ1xBLYrXw0ygy2vFrCS5ASDq5EOB7sJc3YGvq8XFa00rmBuUlsjgSncd5NLEKRY8aEfukUO08=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ffd152c4-cada-4878-a48e-08dc6a85a9ce
X-MS-Exchange-CrossTenant-AuthSource: DM6PR10MB4313.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 May 2024 08:55:49.4525
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: mnNr0AvKEWFE2EISwdZpPA+H05gVzlx1KpeA8yjEUwkrHlr7o36dab0677BWGpsgRm0jooTBTgLSetbI6kAPQQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR10MB5579
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1011,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-05-01_16,2024-05-02_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 bulkscore=0 adultscore=0
 phishscore=0 mlxscore=0 suspectscore=0 mlxlogscore=999 malwarescore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2404010000
 definitions=main-2405020053
X-Proofpoint-GUID: YvBDcHkov1WjIxCqQSin9NBxMJh6EFNt
X-Proofpoint-ORIG-GUID: YvBDcHkov1WjIxCqQSin9NBxMJh6EFNt

On 02/05/2024 02:11, Dave Chinner wrote:
>>> static inline bool
>>> xfs_inode_has_forcealign(struct xfs_inode *ip)
>>> {
>>> 	if (!(ip->di_flags & XFS_DIFLAG_EXTSIZE))
>>> 		return false;
>>> 	if (ip->i_extsize <= 1)
>>> 		return false;
>>>
>>> 	if (xfs_is_cow_inode(ip))
>>> 		return false;
>> Could we just include this in the forcealign validate checks? Currently we
>> just check CoW extsize is zero there.
> Checking COW extsize is zero doesn't tell us anything useful about
> whether the inode might have shared extents, or that the filesystem
> has had the sysfs "always cow" debug knob turned on. That changes
> filesystem behaviour at mount time and has nothing to do with the
> on-disk format constraints.
> 
> And now that I think about it, checking for COW extsize is
> completely the wrong thing to do because it doesn't get used until
> an extent is shared and a COW trigger is hit. So the presence of COW
> extsize has zero impact on whether we can use forced alignment or
> not.

ok

> 
> IOWs, we have to check for shared extents or always cow here,
> because even a file with correctly set up forced alignment needs to
> have forced alignment disabled when always_cow is enabled. Every
> write is going to use the COW path and AFAICT we don't support
> forced alignment through that path yet.

ok

> 
>>> 	if (ip->di_flags & XFS_DIFLAG_REALTIME)
>>> 		return false;
>> We check this in xfs_inode_validate_forcealign()
> That's kinda my point - we have a random smattering of different
> checks at different layers and in different contexts. i.e.  There's
> no one function that performs -all- the "can we do forced alignment"
> checks that allow forced alignment to be used. This simply adds all
> those checks in the one place and ensures that even if other code
> gets checks wrong, we won't use forcealign inappropriately.

Fine, I can do that if you think it is the best strategy.

Thanks,
John