From: Jan Kara <email@example.com> To: "Darrick J. Wong" <firstname.lastname@example.org> Cc: Dave Chinner <email@example.com>, Jan Kara <firstname.lastname@example.org>, email@example.com, Christoph Hellwig <firstname.lastname@example.org>, email@example.com, Chao Yu <firstname.lastname@example.org>, Damien Le Moal <email@example.com>, "Darrick J. Wong" <firstname.lastname@example.org>, Jaegeuk Kim <email@example.com>, Jeff Layton <firstname.lastname@example.org>, Johannes Thumshirn <email@example.com>, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, email@example.com, firstname.lastname@example.org, Miklos Szeredi <email@example.com>, Steve French <firstname.lastname@example.org>, Ted Tso <email@example.com>, Matthew Wilcox <firstname.lastname@example.org> Subject: Re: [PATCH 03/11] mm: Protect operations adding pages to page cache with invalidate_lock Date: Mon, 17 May 2021 13:21:15 +0200 [thread overview] Message-ID: <20210517112115.GC31755@quack2.suse.cz> (raw) In-Reply-To: <20210514161730.GL9675@magnolia> On Fri 14-05-21 09:17:30, Darrick J. Wong wrote: > On Fri, May 14, 2021 at 09:19:45AM +1000, Dave Chinner wrote: > > We've been down this path before more than a decade ago when the > > powers that be decreed that inode locking order is to be "by > > structure address" rather than inode number, because "inode number > > is not unique across multiple superblocks". > > > > I'm not sure that there is anywhere that locks multiple inodes > > across different superblocks, but here we are again.... > > Hm. Are there situations where one would want to lock multiple > /mappings/ across different superblocks? The remapping code doesn't > allow cross-super operations, so ... pipes and splice, maybe? I don't > remember that code well enough to say for sure. Splice and friends work one file at a time. I.e., first they fill a pipe from the file with ->read_iter, then they flush the pipe to the target file with ->write_iter. So file locking doesn't get coupled there. > I've been operating under the assumption that as long as one takes all > the same class of lock at the same time (e.g. all the IOLOCKs, then all > the MMAPLOCKs, then all the ILOCKs, like reflink does) that the > incongruency in locking order rules within a class shouldn't be a > problem. That's my understanding as well. > > > It might simply be time to convert all > > > three XFS inode locks to use the same ordering rules. > > > > Careful, there lie dragons along that path because of things like > > how the inode cluster buffer operations work - they all assume > > ascending inode number traversal within and across inode cluster > > buffers and hence we do have locking order constraints based on > > inode number... > > Fair enough, I'll leave the ILOCK alone. :) OK, so should I change the order for invalidate_lock or shall we just leave that alone as it is not a practical problem AFAICT. Honza -- Jan Kara <email@example.com> SUSE Labs, CR
next prev parent reply other threads:[~2021-05-17 11:21 UTC|newest] Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-12 13:46 [PATCH 0/11 v5] fs: Hole punch vs page cache filling races Jan Kara 2021-05-12 13:46 ` [PATCH 01/11] mm: Fix comments mentioning i_mutex Jan Kara 2021-05-12 13:46 ` [PATCH 02/11] documentation: Sync file_operations members with reality Jan Kara 2021-05-12 13:46 ` [PATCH 03/11] mm: Protect operations adding pages to page cache with invalidate_lock Jan Kara 2021-05-12 14:20 ` Matthew Wilcox 2021-05-13 17:49 ` Jan Kara 2021-05-12 14:40 ` Matthew Wilcox 2021-05-13 19:01 ` Jan Kara 2021-05-13 19:38 ` Matthew Wilcox 2021-05-14 11:07 ` Jan Kara 2021-05-12 15:23 ` Darrick J. Wong 2021-05-13 17:44 ` Jan Kara 2021-05-13 18:52 ` Darrick J. Wong 2021-05-13 23:19 ` Dave Chinner 2021-05-14 16:17 ` Darrick J. Wong 2021-05-17 11:21 ` Jan Kara [this message] 2021-05-18 22:36 ` Dave Chinner 2021-05-19 10:57 ` Jan Kara 2021-05-12 13:46 ` [PATCH 04/11] ext4: Convert to use mapping->invalidate_lock Jan Kara 2021-05-12 13:46 ` [PATCH 05/11] ext2: Convert to using invalidate_lock Jan Kara 2021-05-12 13:46 ` [PATCH 06/11] xfs: Convert to use invalidate_lock Jan Kara 2021-05-12 13:46 ` [PATCH 07/11] zonefs: Convert to using invalidate_lock Jan Kara 2021-05-13 0:34 ` Damien Le Moal 2021-05-12 13:46 ` [PATCH 08/11] f2fs: " Jan Kara 2021-05-12 13:46 ` [PATCH 09/11] fuse: " Jan Kara 2021-05-12 13:46 ` [PATCH 10/11] ceph: Fix race between hole punch and page fault Jan Kara 2021-05-12 15:19 ` Jeff Layton 2021-05-12 13:46 ` [PATCH 11/11] cifs: " Jan Kara
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20210517112115.GC31755@quack2.suse.cz \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH 03/11] mm: Protect operations adding pages to page cache with invalidate_lock' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).