On Tue, Aug 22, 2023 at 10:19:00PM +0200, Christian Brauner wrote: > > > my preferred solution. How do you feel about that? > > > > I'm happy to have you pull my xfs-linux tags into your vfs tree. :) > > Ah, sweet. I apppreciate that. I'll mention in the pr to Linus that if > he wants to reject other parts of the super work that he should then > still simply pull the freeze stuff from you without the rest. > > > > > Here's a tag with just the two vfs patches: > > https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git/tag/?h=vfs-6.6-merge-2 > > > > This second tag builds on that, by adding the first actual user of > > FREEZE_HOLDER_KERNEL: > > https://git.kernel.org/pub/scm/fs/xfs/xfs-linux.git/tag/?h=vfs-6.6-merge-3 > > Assuming I understood correctly I did just pull both tags and pushed > them out. Would you please take a look at > https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git/log/?h=vfs.super > and let me know if everything looks as expected? I'll be going afk in a > bit just waiting for the kernel build to finish to kick of some > xfstests. If you find anything I'll fix up any issues up tomorrow > morning. Hmm. Looking at the {up,down}_write -> super_{un,}lock_excl conversion, I think you missed wait_for_partially_frozen: static int wait_for_partially_frozen(struct super_block *sb) { int ret = 0; do { unsigned short old = sb->s_writers.frozen; up_write(&sb->s_umount); ret = wait_var_event_killable(&sb->s_writers.frozen, sb->s_writers.frozen != old); down_write(&sb->s_umount); } while (ret == 0 && sb->s_writers.frozen != SB_UNFROZEN && sb->s_writers.frozen != SB_FREEZE_COMPLETE); return ret; } That said, freeze_super() took an s_active refcount at the top, called super_lock_excl (which means the sb isn't DYING and has been BORN) and doesn't release it before calling wait_for_partially_frozen. AFAICT, the subsequent down_write -> super_lock_excl conversions in freeze_super do not gain us much since I don't think the sb can get to SB_DYING state without s_active reaching zero, right? According to "super: use higher-level helper for {freeze,thaw}", it sounds like the subsequent down_write calls in freeze_super were replaced for consistency, even though it "...isn't possible to observe a dying superblock". The missing conversion isn't strictly necessary, but it probably makese sense to do it anyway. (Aside from that, the conversion looks correct to me.) > > > > There will be more for 6.7(+?) if Luis manages to get back to his > > auto-fsfreeze during suspend, or if Shiyang finishes the series to > > handle pmem media error reporting in xfs. > > Ok, sounds good let me know/Cc me when ready/needed. Will do! --D > > Thanks for all the help!
