From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pj1-f48.google.com (mail-pj1-f48.google.com [209.85.216.48]) (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 9DB181C3F34 for ; Thu, 23 Jan 2025 20:24:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.48 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737663892; cv=none; b=NgoHI0pCSLMWds8tNZLHRpi7PG8zdQjQ3o+DoCALHoxUHWVTJE5zjWAzR3k6sIFHg6ixJBBO4nzXgdJxsLCUmjtNmvQgN4BTw+kBFUgJXkSwBj7Wwy0mk6CZ0BZzu5EV8x2KhlNR4qj0w4NeuqFUvm2g6v3CjmdT/0SGXlDad0E= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737663892; c=relaxed/simple; bh=kLTU6pzyItby0Xy68IXUlVww8A5hyuVx1r9o6Jn2yyE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=ZaeJQKlFg57A9EYriQNVp6Cv+inkNjmxLLF69l437Z4iWvEwyJn08qvjkeHX21u9Cgc2t599YtQ8xXERVucZPmwk1vsP2mIAung1RGNOjt0Ra7opaPksytNlZEhE4U8on5PEF9le+7LG+rPbZqaNSsdgCbdVtVgYiaeGOpcWNY8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=fromorbit.com; spf=pass smtp.mailfrom=fromorbit.com; dkim=pass (2048-bit key) header.d=fromorbit-com.20230601.gappssmtp.com header.i=@fromorbit-com.20230601.gappssmtp.com header.b=j1aCEUK9; arc=none smtp.client-ip=209.85.216.48 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=fromorbit.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=fromorbit.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=fromorbit-com.20230601.gappssmtp.com header.i=@fromorbit-com.20230601.gappssmtp.com header.b="j1aCEUK9" Received: by mail-pj1-f48.google.com with SMTP id 98e67ed59e1d1-2ee76befe58so2526127a91.2 for ; Thu, 23 Jan 2025 12:24:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fromorbit-com.20230601.gappssmtp.com; s=20230601; t=1737663890; x=1738268690; 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=dwxbQH9kgdkSqlPoE+manNQA8Ci31FJ9FZBvSpHLAn4=; b=j1aCEUK9CeXYTu6E7K5BTbm808nRVGCB49cJv1QTWBtIqyvzcarBnWikBgwuFUQxPH /zsDcvxnKoYDXUv0OfFOVq2RzuQKgCMhxqADDWt1JLXu4SkwE6cjky/iqdMlpcbtslia QCc8iqZdLZoxAqcDIbuPRfeGY5YLP8c/e6LmlAAz0A+0VQ2XpiLIuNMBqBtJf/laWbUZ yT3R7poIgIPOAPFB6G0dnnVONec1i7aD6dLFfzUdwLWgjfdpEVi88+ag7I8kzwM/pSlf hlbou0XBkkaK/3x7GM75QGxrUFZsfEp1+Ee5+ImIhaA3KR/rZ5HKgs1nVwK+UUZbu52s q29g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1737663890; x=1738268690; 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=dwxbQH9kgdkSqlPoE+manNQA8Ci31FJ9FZBvSpHLAn4=; b=wZuJJxKno8DsZv5lbd9DrXJyXK4bbHrlb/LBJfyHM+ozXGDFD/gCCOqwu0pdnMvvOx cdRSzpBeFzoE85McL3CgmIpOj2/RjLrsmxVE7XJsny4IW0M9JWhlI+iRqZvzVhjGJtjo 0vpJ3pZmaoGH5kIzpfJsvb6q3AmYQwaPmkPCSjaSSl4Av+gM3u1UQSilWIo90vVxfukB k8rfoYEuwaUg5JGWW7/ps3WIE+cGRnRNeZg6Vdfts+gZseBBHU6CALGxFqTVDkfVaC/2 /7ml/ZLWjjvylsYO3o2CUGw4j419QYyCupUq0c0biiGRz0ntLDTFYzNspW78kghZkxb2 nTmQ== X-Forwarded-Encrypted: i=1; AJvYcCXGp1MoTdNxbA1bX7r4qURg0F/h3qG38ukx4/78rnuxS/jhFqcnxrN9Hvj8OYuh4i9A8wWyGW19XZqv/40=@vger.kernel.org X-Gm-Message-State: AOJu0YzgdtZREjfS6V69pYigVDoo6taaOs1k9gqXKXw1tfapoQXGHSaT LsmEESQIYzopsF+pQ3XPBP4TfVcmpxaPi1G9kLi0mmp7zKYteDgSBg6LgwGnmiI= X-Gm-Gg: ASbGncsh4gbqy9ZsCz2/rwJdwRLb+lKoqrxVBxI5MoVm87naOe5RTYoRPJcRInWTQcr zXJqdkhfrjoFBM5qcqYZKqPKuPRqwc5ufQ3AfJ3tti+9w75MRIQ55K7VkK6DyKOBZNQxqoJvxsR qeyzlYdy96j0fgrr24g0OpqGHcjQXiTKPZtlspW+apxaYE003OLcm5EvZHJAVH9h6F5KHtDh4+k HmAoUPxV11/xZznU3x9jfaaQ1SXb82SOBjS/KbFDwo1R7oXVqmHISxuUpbkWyesmb012U4liT77 gpxBPsqLVRR+G9BTGNhvNnGERA08mfGgu2oIuzQcPdC+6q7rgk56PCAd X-Google-Smtp-Source: AGHT+IEDWgzC0RvHUKB4IxqckjyA0ZkEijLZzCLsb3DH4LWINRo1G1cTO4KJdVOfn7eQaWHn0DCVVA== X-Received: by 2002:a17:90b:3a08:b0:2ee:c291:765a with SMTP id 98e67ed59e1d1-2f782c8f352mr40851352a91.8.1737663889811; Thu, 23 Jan 2025 12:24:49 -0800 (PST) Received: from dread.disaster.area (pa49-186-89-135.pa.vic.optusnet.com.au. [49.186.89.135]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2f7ffa6acd0sm151904a91.28.2025.01.23.12.24.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Jan 2025 12:24:49 -0800 (PST) Received: from dave by dread.disaster.area with local (Exim 4.98) (envelope-from ) id 1tb3kr-00000009dTo-2VTC; Fri, 24 Jan 2025 07:24:45 +1100 Date: Fri, 24 Jan 2025 07:24:45 +1100 From: Dave Chinner To: Konstantin Komarov Cc: ntfs3@lists.linux.dev, linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org, Jiaji Qin , Kun Hu Subject: Re: [PATCH] fs/ntfs3: Update inode->i_mapping->a_ops on compression state change Message-ID: References: <20250123135335.15060-1-almaz.alexandrovich@paragon-software.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=us-ascii Content-Disposition: inline In-Reply-To: <20250123135335.15060-1-almaz.alexandrovich@paragon-software.com> On Thu, Jan 23, 2025 at 04:53:35PM +0300, Konstantin Komarov wrote: > Update inode->i_mapping->a_ops when the compression state changes to > ensure correct address space operations. > Clear ATTR_FLAG_SPARSED/FILE_ATTRIBUTE_SPARSE_FILE when enabling > compression to prevent flag conflicts. > > Fixes: 6b39bfaeec44 ("fs/ntfs3: Add support for the compression attribute") > Reported-by: Kun Hu , Jiaji Qin > Signed-off-by: Konstantin Komarov > --- > fs/ntfs3/attrib.c | 1 + > fs/ntfs3/file.c | 2 ++ > fs/ntfs3/frecord.c | 6 ++++-- > 3 files changed, 7 insertions(+), 2 deletions(-) > > diff --git a/fs/ntfs3/attrib.c b/fs/ntfs3/attrib.c > index af94e3737470..d2410ab6c7bf 100644 > --- a/fs/ntfs3/attrib.c > +++ b/fs/ntfs3/attrib.c > @@ -2666,6 +2666,7 @@ int attr_set_compress(struct ntfs_inode *ni, bool compr) > > /* Update data attribute flags. */ > if (compr) { > + attr->flags &= ATTR_FLAG_SPARSED; > attr->flags |= ATTR_FLAG_COMPRESSED; > attr->nres.c_unit = NTFS_LZNT_CUNIT; > } else { > diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c > index 3f96a11804c9..e8f452f47cd5 100644 > --- a/fs/ntfs3/file.c > +++ b/fs/ntfs3/file.c > @@ -105,6 +105,8 @@ int ntfs_fileattr_set(struct mnt_idmap *idmap, struct dentry *dentry, > int err = ni_set_compress(inode, flags & FS_COMPR_FL); > if (err) > return err; > + inode->i_mapping->a_ops = > + (flags & FS_COMPR_FL) ? &ntfs_aops_cmpr : &ntfs_aops; It is not safe to change a_ops dynamically like this. There can be other operations running concurrently using the existing aops, and some functionality (e.g. the page fault paths) have dependencies between aops methods and swapping the aops between those operations can cause all sorts of problem (including crashing the kernel). This is the reason why we cannot turn functionality like FSDAX on and off via fileattr_set() methods setting/clearing inode flags. The inode has to transition out of cache and be re-instantiated to change the inode aops methods safely... -Dave. -- Dave Chinner david@fromorbit.com